Skip to content

Commit 66c2e2c

Browse files
authored
[cxx-interop] Import non-public inherited members (#79348)
This patch is follow-up work from #78942 and imports non-public members, which were previously not being imported. Those members can be accessed in a Swift file blessed by the SWIFT_PRIVATE_FILEID annotation. As a consequence of this patch, we are also now importing inherited members that are inaccessible from the derived classes, because they were declared private, or because they were inherited via nested private inheritance. We import them anyway but mark them unavailable, for better diagnostics and to (somewhat) simplify the import logic for inheritance. Because non-public base class members are now imported too, this patch inflames an existing issue where a 'using' declaration on an inherited member with a synthesized name (e.g., operators) produces duplicate members, leading to miscompilation (resulting in a runtime crash). This was not previously noticed because a 'using' declaration on a public inherited member is not usually necessary, but is a common way to expose otherwise non-public members. This patch puts in a workaround to prevent this from affecting the behavior of MSVC's std::optional implementation, which uses this pattern of 'using' a private inherited member. That will be fixed in a follow-up patch. Follow-up work is also needed to correctly diagnose ambiguous overloads in cases of multiple inheritance, and to account for virtual inheritance. rdar://137764620
1 parent b352d17 commit 66c2e2c

35 files changed

+1476
-201
lines changed

include/swift/AST/ClangModuleLoader.h

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ class SourceLocation;
3232

3333
namespace swift {
3434

35+
class ClangInheritanceInfo;
3536
class ConcreteDeclRef;
3637
class Decl;
3738
class FuncDecl;
@@ -202,10 +203,17 @@ class ClangModuleLoader : public ModuleLoader {
202203
/// Imports a clang decl directly, rather than looking up its name.
203204
virtual Decl *importDeclDirectly(const clang::NamedDecl *decl) = 0;
204205

205-
/// Imports a clang decl from a base class, cloning it for \param newContext
206-
/// if it wasn't cloned for this specific context before.
206+
/// Clones an imported \param decl from its base class to its derived class
207+
/// \param newContext where it is inherited. Its access level is determined
208+
/// with respect to \param inheritance, which signifies whether \param decl
209+
/// was inherited via C++ public/protected/private inheritance.
210+
///
211+
/// This function uses a cache so that it is idempotent; successive
212+
/// invocations will only generate one cloned ValueDecl (and all return
213+
/// a pointer to it). Returns a NULL pointer upon failure.
207214
virtual ValueDecl *importBaseMemberDecl(ValueDecl *decl,
208-
DeclContext *newContext) = 0;
215+
DeclContext *newContext,
216+
ClangInheritanceInfo inheritance) = 0;
209217

210218
/// Emits diagnostics for any declarations named name
211219
/// whose direct declaration context is a TU.

include/swift/ClangImporter/ClangImporter.h

Lines changed: 115 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#ifndef SWIFT_CLANG_IMPORTER_H
1717
#define SWIFT_CLANG_IMPORTER_H
1818

19+
#include "swift/AST/Attr.h"
1920
#include "swift/AST/AttrKind.h"
2021
#include "swift/AST/ClangModuleLoader.h"
2122
#include "clang/Basic/Specifiers.h"
@@ -72,6 +73,7 @@ namespace swift {
7273
class ASTContext;
7374
class CompilerInvocation;
7475
class ClangImporterOptions;
76+
class ClangInheritanceInfo;
7577
class ClangModuleUnit;
7678
class ClangNode;
7779
class ConcreteDeclRef;
@@ -660,8 +662,8 @@ class ClangImporter final : public ClangModuleLoader {
660662
/// Imports a clang decl directly, rather than looking up it's name.
661663
Decl *importDeclDirectly(const clang::NamedDecl *decl) override;
662664

663-
ValueDecl *importBaseMemberDecl(ValueDecl *decl,
664-
DeclContext *newContext) override;
665+
ValueDecl *importBaseMemberDecl(ValueDecl *decl, DeclContext *newContext,
666+
ClangInheritanceInfo inheritance) override;
665667

666668
/// Emits diagnostics for any declarations named name
667669
/// whose direct declaration context is a TU.
@@ -779,6 +781,117 @@ ClangInvocationFileMapping getClangInvocationFileMapping(
779781
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> vfs = nullptr,
780782
bool suppressDiagnostic = false);
781783

784+
/// Information used to compute the access level of inherited C++ members.
785+
class ClangInheritanceInfo {
786+
/// The cumulative inheritance access specifier, that is used to compute the
787+
/// effective access level of a particular inherited member.
788+
///
789+
/// When constructing ClangInheritanceInfo for nested inheritance, this field
790+
/// gets clamped to the least permissive level between its current value and
791+
/// the inheritance access specifier.
792+
///
793+
/// If we encounter \e nested private inheritance in the class hierarchy
794+
/// (i.e., private inheritance beyond the first level of inheritance), we set
795+
/// the access level to nullopt to indicate that none of the members from
796+
/// classes beyond that point in the hierarchy should be accessible. This case
797+
/// must be treated separately from non-nested private inheritance, where
798+
/// inherited members are private but accessible from extensions.
799+
///
800+
/// See ClangInheritanceInfo::cumulativeInheritedAccess() for an example.
801+
std::optional<clang::AccessSpecifier> access;
802+
803+
public:
804+
/// Default constructor for this class that is used as the base case when
805+
/// recursively walking up a class inheritance hierarchy.
806+
ClangInheritanceInfo() : access(clang::AS_none) {}
807+
808+
/// Inductive case for this class that is used to accumulate inheritance
809+
/// metadata for cases of (nested) inheritance.
810+
ClangInheritanceInfo(ClangInheritanceInfo prev, clang::CXXBaseSpecifier base)
811+
: access(computeAccess(prev, base)) {}
812+
813+
/// Whether this info represents a case of nested private inheritance.
814+
bool isNestedPrivate() const { return !access.has_value(); }
815+
816+
/// Whether this info represents a case of C++ inheritance.
817+
///
818+
/// Returns \c false for the default instance of this class.
819+
bool isInheriting() const {
820+
return isNestedPrivate() || *access != clang::AS_none;
821+
}
822+
823+
/// Whether this is info represents a case of C++ inheritance.
824+
operator bool() const { return isInheriting(); }
825+
826+
/// Compute the (Swift) access level for inherited base member \param decl,
827+
/// for when its inherited (cloned) member in the derived class.
828+
///
829+
/// This access level is determined by whichever is more restrictive: what the
830+
/// \param decl was declared with (in its base class), or what it is being
831+
/// inherited with (ClangInheritanceInfo::access).
832+
///
833+
/// Always returns swift::AccessLevel::Public (i.e., corresponding to
834+
/// clang::AS_none) if this ClangInheritanceInfo::isInheriting() is \c false.
835+
AccessLevel accessForBaseDecl(const ValueDecl *baseDecl) const;
836+
837+
/// Marks \param clonedDecl as unavailable (using \c @available) if it
838+
/// cannot be accessed from the derived class, either because \param baseDecl
839+
/// was declared as private in the base class, or because \param clonedDecl
840+
/// was inherited with private inheritance.
841+
///
842+
/// Does nothing if this ClangInheritanceInfo::isInheriting() is \c false.
843+
void setUnavailableIfNecessary(const ValueDecl *baseDecl,
844+
ValueDecl *clonedDecl) const;
845+
846+
friend llvm::hash_code hash_value(const ClangInheritanceInfo &info) {
847+
return llvm::hash_combine(info.access);
848+
}
849+
850+
private:
851+
/// An example of how ClangInheritanceInfo:access is accumulated while
852+
/// recursively traversing the class hierarchy starting from \c E:
853+
///
854+
/// \code{.cpp}
855+
/// struct A { ... }; // access = nullopt (nested private)
856+
/// struct B : private A { ... }; // access = protected
857+
/// struct C : public B { ... }; // access = protected
858+
/// struct D : protected C { ... }; // access = public
859+
/// struct E : public D { ... }; // access = none [base case]
860+
/// \endcode
861+
///
862+
/// Another example, this time with non-nested private inheritance:
863+
///
864+
/// \code{.cpp}
865+
/// struct A { ... }; // access = nullopt
866+
/// struct B : public A { ... }; // access = nullopt
867+
/// struct C : private B { ... }; // access = private
868+
/// struct D : public C { ... }; // access = private
869+
/// struct E : private D { ... }; // access = none [base case]
870+
/// \endcode
871+
static std::optional<clang::AccessSpecifier>
872+
computeAccess(ClangInheritanceInfo prev, clang::CXXBaseSpecifier base) {
873+
auto baseAccess = base.getAccessSpecifier();
874+
assert(baseAccess != clang::AS_none &&
875+
"this should always be public, protected, or private");
876+
877+
if (!prev.isInheriting())
878+
// This is the first level of inheritance, so we just take the access
879+
// specifier from CXXBaseSpecifier. Note that this is the only scenario
880+
// where we can have access = private.
881+
return {baseAccess};
882+
883+
if (prev.isNestedPrivate() || baseAccess == clang::AS_private)
884+
// This is a case of nested inheritance, and either we encountered nested
885+
// private inheritance before, or this is our first time encountering it.
886+
return std::nullopt;
887+
888+
static_assert(clang::AS_private > clang::AS_protected &&
889+
clang::AS_protected > clang::AS_public &&
890+
"using std::max() relies on this ordering");
891+
return {std::max(*prev.access, baseAccess)};
892+
}
893+
};
894+
782895
} // end namespace swift
783896

784897
#endif

include/swift/ClangImporter/ClangImporterRequests.h

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
#include "swift/Basic/Statistic.h"
2525
#include "swift/ClangImporter/ClangImporter.h"
2626
#include "clang/AST/Type.h"
27+
#include "clang/Basic/Specifiers.h"
2728
#include "llvm/ADT/Hashing.h"
2829
#include "llvm/ADT/TinyPtrVector.h"
2930

@@ -133,25 +134,33 @@ class CXXNamespaceMemberLookup
133134
};
134135

135136
/// The input type for a record member lookup request.
137+
///
138+
/// These lookups may be requested recursively in the case of inheritance,
139+
/// for which we separately keep track of the derived class where we started
140+
/// looking (startDecl) and the access level for the current inheritance.
136141
struct ClangRecordMemberLookupDescriptor final {
137-
NominalTypeDecl *recordDecl;
138-
NominalTypeDecl *inheritingDecl;
139-
DeclName name;
142+
NominalTypeDecl *recordDecl; // Where we are currently looking
143+
NominalTypeDecl *inheritingDecl; // Where we started looking from
144+
DeclName name; // What we are looking for
145+
ClangInheritanceInfo inheritance;
140146

141147
ClangRecordMemberLookupDescriptor(NominalTypeDecl *recordDecl, DeclName name)
142-
: recordDecl(recordDecl), inheritingDecl(recordDecl), name(name) {
148+
: recordDecl(recordDecl), inheritingDecl(recordDecl), name(name),
149+
inheritance() {
143150
assert(isa<clang::RecordDecl>(recordDecl->getClangDecl()));
144151
}
145152

146153
friend llvm::hash_code
147154
hash_value(const ClangRecordMemberLookupDescriptor &desc) {
148-
return llvm::hash_combine(desc.name, desc.recordDecl, desc.inheritingDecl);
155+
return llvm::hash_combine(desc.name, desc.recordDecl, desc.inheritingDecl,
156+
desc.inheritance);
149157
}
150158

151159
friend bool operator==(const ClangRecordMemberLookupDescriptor &lhs,
152160
const ClangRecordMemberLookupDescriptor &rhs) {
153161
return lhs.name == rhs.name && lhs.recordDecl == rhs.recordDecl &&
154-
lhs.inheritingDecl == rhs.inheritingDecl;
162+
lhs.inheritingDecl == rhs.inheritingDecl &&
163+
lhs.inheritance == rhs.inheritance;
155164
}
156165

157166
friend bool operator!=(const ClangRecordMemberLookupDescriptor &lhs,
@@ -165,10 +174,14 @@ struct ClangRecordMemberLookupDescriptor final {
165174
// This private constructor should only be used in ClangRecordMemberLookup,
166175
// for recursively traversing base classes that inheritingDecl inherites from.
167176
ClangRecordMemberLookupDescriptor(NominalTypeDecl *recordDecl, DeclName name,
168-
NominalTypeDecl *inheritingDecl)
169-
: recordDecl(recordDecl), inheritingDecl(inheritingDecl), name(name) {
177+
NominalTypeDecl *inheritingDecl,
178+
ClangInheritanceInfo inheritance)
179+
: recordDecl(recordDecl), inheritingDecl(inheritingDecl), name(name),
180+
inheritance(inheritance) {
170181
assert(isa<clang::RecordDecl>(recordDecl->getClangDecl()));
171182
assert(isa<clang::CXXRecordDecl>(inheritingDecl->getClangDecl()));
183+
assert(inheritance.isInheriting() &&
184+
"recursive calls should indicate inheritance");
172185
assert(recordDecl != inheritingDecl &&
173186
"recursive calls should lookup elsewhere");
174187
}

0 commit comments

Comments
 (0)