Merge pull request swiftlang#39664 from zoecarver/lazy-pt9-use-lookup-table

[cxx-interop] Add members to the LookupTable where possible.

Author: zoecarver

include/swift/AST/Decl.h

@@ -3078,6 +3078,10 @@ class NominalTypeDecl : public GenericTypeDecl, public IterableDeclContext {
   /// Prepare to traverse the list of extensions.
   void prepareExtensions();
 
+  /// Add loaded members from all extensions. Eagerly load any members that we
+  /// can't lazily load.
+  void addLoadedExtensions();
+
   /// Retrieve the conformance loader (if any), and removing it in the
   /// same operation. The caller is responsible for loading the
   /// conformances.
@@ -3200,6 +3204,11 @@ class NominalTypeDecl : public GenericTypeDecl, public IterableDeclContext {
   /// Add a new extension to this nominal type.
   void addExtension(ExtensionDecl *extension);
 
+  /// Add a member to this decl's lookup table.
+  ///
+  /// Calls "prepareLookupTable" as a side effect.
+  void addMemberToLookupTable(Decl *member);
+
   /// Retrieve the set of extensions of this type.
   ExtensionRange getExtensions();
 

lib/AST/DeclContext.cpp

@@ -867,6 +867,10 @@ void IterableDeclContext::addMemberSilently(Decl *member, Decl *hint,
       if (d->isImplicit())
         return true;
 
+      // Imported decls won't have complete location info.
+      if (d->hasClangNode())
+        return true;
+
       return false;
     };
 

lib/AST/NameLookup.cpp

@@ -1220,6 +1220,12 @@ void ExtensionDecl::addedMember(Decl *member) {
   }
 }
 
+void NominalTypeDecl::addMemberToLookupTable(Decl *member) {
+  prepareLookupTable();
+
+  LookupTable->addMember(member);
+}
+
 // For lack of anywhere more sensible to put it, here's a diagram of the pieces
 // involved in finding members and extensions of a NominalTypeDecl.
 //
@@ -1332,7 +1338,9 @@ void NominalTypeDecl::prepareLookupTable() {
   } else {
     LookupTable->addMembers(getMembers());
   }
+}
 
+void NominalTypeDecl::addLoadedExtensions() {
   for (auto e : getExtensions()) {
     // If we can lazy-load this extension, only take the members we've loaded
     // so far.
@@ -1424,10 +1432,10 @@ DirectLookupRequest::evaluate(Evaluator &evaluator,
 
   decl->prepareLookupTable();
 
-  // If we're allowed to load extensions, call prepareExtensions to ensure we
+  // If we're allowed to load extensions, call addLoadedExtensions to ensure we
   // properly invalidate the lazily-complete cache for any extensions brought in
   // by modules loaded after-the-fact. This can happen with the LLDB REPL.
-  decl->prepareExtensions();
+  decl->addLoadedExtensions();
 
   auto &Table = *decl->LookupTable;
   if (!useNamedLazyMemberLoading) {

lib/ClangImporter/ClangImporter.cpp

@@ -4051,7 +4051,8 @@ ClangImporter::Implementation::loadNamedMembers(
   auto table = findLookupTable(*CMO);
   assert(table && "clang module without lookup table");
 
-  assert(isa<clang::ObjCContainerDecl>(CD) || isa<clang::NamespaceDecl>(CD));
+  assert(isa<clang::ObjCContainerDecl>(CD) || isa<clang::NamespaceDecl>(CD) ||
+         isa<clang::RecordDecl>(CD));
 
   // Force the members of the entire inheritance hierarchy to be loaded and
   // deserialized before loading the named member of a class. This warms up

lib/ClangImporter/ImportDecl.cpp

@@ -2401,48 +2401,6 @@ namespace {
         if (auto *typedefForAnon = decl->getTypedefNameForAnonDecl())
           return importFullName(typedefForAnon);
       }
-
-      if (!decl->isRecord())
-        return {ImportedName(), None};
-
-      // If the type has no name and no structure name, but is not anonymous,
-      // generate a name for it. Specifically this is for cases like:
-      //   struct a {
-      //     struct {} z;
-      //   }
-      // Where the member z is an unnamed struct, but does have a member-name
-      // and is accessible as a member of struct a.
-      if (auto recordDecl = dyn_cast<clang::RecordDecl>(
-                              decl->getLexicalDeclContext())) {
-        for (auto field : recordDecl->fields()) {
-          if (field->getType()->getAsTagDecl() == decl) {
-            // Create a name for the declaration from the field name.
-            std::string Id;
-            llvm::raw_string_ostream IdStream(Id);
-
-            const char *kind;
-            if (decl->isStruct())
-              kind = "struct";
-            else if (decl->isUnion())
-              kind = "union";
-            else
-              llvm_unreachable("unknown decl kind");
-
-            IdStream << "__Unnamed_" << kind << "_";
-            if (field->isAnonymousStructOrUnion()) {
-              IdStream << "__Anonymous_field" << field->getFieldIndex();
-            } else {
-              assert(!field->getDeclName().isEmpty() &&
-                     "Microsoft anonymous struct extension?");
-              IdStream << field->getName();
-            }
-            ImportedName Result;
-            Result.setDeclName(Impl.SwiftContext.getIdentifier(IdStream.str()));
-            Result.setEffectiveContext(decl->getDeclContext());
-            return {Result, None};
-          }
-        }
-      }
 
       return {ImportedName(), None};
     }
@@ -3634,7 +3592,7 @@ namespace {
         if (auto nominalDecl =
                 dyn_cast<NominalTypeDecl>(nestedType->getDeclContext())) {
           assert(nominalDecl == result && "interesting nesting of C types?");
-          nominalDecl->addMember(nestedType);
+          nominalDecl->addMemberToLookupTable(nestedType);
         }
       }
 
@@ -3675,7 +3633,7 @@ namespace {
                                                   /*wantBody=*/true);
           ctors.push_back(valueCtor);
         }
-        result->addMember(member);
+        result->addMemberToLookupTable(member);
       }
 
       const clang::CXXRecordDecl *cxxRecordDecl =
@@ -3710,12 +3668,14 @@ namespace {
         ctors.push_back(valueCtor);
       }
 
+      // Add ctors directly as they cannot always be looked up from the clang
+      // decl (some are synthesized by Swift).
       for (auto ctor : ctors) {
         result->addMember(ctor);
       }
 
       for (auto method : methods) {
-        result->addMember(method);
+        result->addMemberToLookupTable(method);
       }
 
       result->setHasUnreferenceableStorage(hasUnreferenceableStorage);
@@ -3731,10 +3691,13 @@ namespace {
           auto getterAndSetter = subscriptInfo.second;
           auto subscript = makeSubscript(getterAndSetter.first,
                                          getterAndSetter.second);
+          // Also add subscripts directly because they won't be found from the
+          // clang decl.
           result->addMember(subscript);
         }
       }
 
+      result->setMemberLoader(&Impl, 0);
       return result;
     }
 
@@ -4363,21 +4326,9 @@ namespace {
       Optional<ImportedName> correctSwiftName;
       ImportedName importedName;
 
-      if (!decl->isAnonymousStructOrUnion() && !decl->getDeclName().isEmpty()) {
-        std::tie(importedName, correctSwiftName) = importFullName(decl);
-        if (!importedName) {
-          return nullptr;
-        }
-      } else {
-        // Generate a field name for anonymous fields, this will be used in
-        // order to be able to expose the indirect fields injected from there
-        // as computed properties forwarding the access to the subfield.
-        std::string Id;
-        llvm::raw_string_ostream IdStream(Id);
-
-        IdStream << "__Anonymous_field" << decl->getFieldIndex();
-        importedName.setDeclName(Impl.SwiftContext.getIdentifier(IdStream.str()));
-        importedName.setEffectiveContext(decl->getDeclContext());
+      std::tie(importedName, correctSwiftName) = importFullName(decl);
+      if (!importedName) {
+        return nullptr;
       }
 
       auto name = importedName.getDeclName().getBaseIdentifier();
@@ -9874,6 +9825,38 @@ static void loadAllMembersOfSuperclassIfNeeded(ClassDecl *CD) {
     E->loadAllMembers();
 }
 
+static void loadAllMembersOfRecordDecl(StructDecl *recordDecl) {
+  auto &ctx = recordDecl->getASTContext();
+  auto clangRecord = cast<clang::RecordDecl>(recordDecl->getClangDecl());
+
+  // This is only to keep track of the members we've already seen.
+  llvm::SmallPtrSet<Decl *, 16> addedMembers;
+  for (auto member : recordDecl->getCurrentMembersWithoutLoading())
+    addedMembers.insert(member);
+
+  for (auto member : clangRecord->decls()) {
+    auto namedDecl = dyn_cast<clang::NamedDecl>(member);
+    if (!namedDecl)
+      continue;
+
+    // Don't try to load ourselves (this will create an infinite loop).
+    if (auto possibleSelf = dyn_cast<clang::RecordDecl>(member))
+      if (possibleSelf->getDefinition() == clangRecord)
+        continue;
+
+    auto name = ctx.getClangModuleLoader()->importName(namedDecl);
+    if (!name)
+      continue;
+
+    for (auto found : recordDecl->lookupDirect(name)) {
+      if (addedMembers.insert(found).second &&
+          // TODO: remove this check once PR#38675 lands.
+          found->getDeclContext() == recordDecl)
+        recordDecl->addMember(found);
+    }
+  }
+}
+
 void
 ClangImporter::Implementation::loadAllMembers(Decl *D, uint64_t extra) {
 
@@ -9892,26 +9875,8 @@ ClangImporter::Implementation::loadAllMembers(Decl *D, uint64_t extra) {
     return;
   }
 
-  auto namespaceDecl =
-      dyn_cast_or_null<clang::NamespaceDecl>(D->getClangDecl());
-  if (namespaceDecl) {
-    auto *enumDecl = cast<EnumDecl>(D);
-    // TODO: This redecls should only match redecls that are in the same
-    // module as namespaceDecl after we import one namespace per clang module.
-    for (auto ns : namespaceDecl->redecls()) {
-      for (auto m : ns->decls()) {
-        auto nd = dyn_cast<clang::NamedDecl>(m);
-        if (!nd)
-          continue;
-        auto member = importDecl(nd, CurrentVersion);
-        if (!member)
-          continue;
-
-        // TODO: remove this change when #34706 lands.
-        if (!member->NextDecl)
-          enumDecl->addMember(member);
-      }
-    }
+  if (D->getClangDecl() && isa<clang::RecordDecl>(D->getClangDecl())) {
+    loadAllMembersOfRecordDecl(cast<StructDecl>(D));
     return;
   }
 

lib/ClangImporter/ImportName.cpp

@@ -1670,9 +1670,67 @@ ImportedName NameImporter::importNameImpl(const clang::NamedDecl *D,
     }
   }
 
-  // For empty names, there is nothing to do.
-  if (D->getDeclName().isEmpty())
+  // Spcial case: unnamed/anonymous fields.
+  if (auto field = dyn_cast<clang::FieldDecl>(D)) {
+    if (field->isAnonymousStructOrUnion() || field->getDeclName().isEmpty()) {
+      // Generate a field name for anonymous fields, this will be used in
+      // order to be able to expose the indirect fields injected from there
+      // as computed properties forwarding the access to the subfield.
+      std::string name;
+      llvm::raw_string_ostream nameStream(name);
+
+      nameStream << "__Anonymous_field" << field->getFieldIndex();
+      result.setDeclName(swiftCtx.getIdentifier(nameStream.str()));
+      result.setEffectiveContext(field->getDeclContext());
+      return result;
+    }
+  }
+
+  if (D->getDeclName().isEmpty()) {
+    // If the type has no name and no structure name, but is not anonymous,
+    // generate a name for it. Specifically this is for cases like:
+    //   struct a {
+    //     struct {} z;
+    //   }
+    // Where the member z is an unnamed struct, but does have a member-name
+    // and is accessible as a member of struct a.
+    if (auto recordDecl = dyn_cast<clang::RecordDecl>(
+                            D->getLexicalDeclContext())) {
+      for (auto field : recordDecl->fields()) {
+        auto fieldTagDecl = field->getType()->getAsTagDecl();
+        if (fieldTagDecl == D) {
+          // Create a name for the declaration from the field name.
+          std::string name;
+          llvm::raw_string_ostream nameStream(name);
+
+          const char *kind;
+          if (fieldTagDecl->isStruct())
+            kind = "struct";
+          else if (fieldTagDecl->isUnion())
+            kind = "union";
+          else if  (fieldTagDecl->isEnum())
+            kind = "enum";
+          else
+            llvm_unreachable("unknown decl kind");
+
+          nameStream << "__Unnamed_" << kind << "_";
+          if (field->isAnonymousStructOrUnion()) {
+            nameStream << "__Anonymous_field" << field->getFieldIndex();
+          } else {
+            assert(!field->getDeclName().isEmpty() &&
+                   "Microsoft anonymous struct extension?");
+            nameStream << field->getName();
+          }
+          result.setDeclName(swiftCtx.getIdentifier(nameStream.str()));
+          result.setEffectiveContext(D->getDeclContext());
+          return result;
+        }
+      }
+    }
+
+    // Otherwise, for empty names, there is nothing to do.
     return result;
+  }
 
   /// Whether the result is a function name.
   bool isFunction = false;

test/ClangImporter/Inputs/SwiftPrivateAttr.txt

@@ -41,19 +41,19 @@ class __PrivFooSub : Foo {
 }
 func __privTest()
 struct S0 {
-  var __privValue: Int32
   init()
   init(__privValue: Int32)
+  var __privValue: Int32
 }
 struct __PrivS1 {
-  var value: Int32
   init()
   init(value: Int32)
+  var value: Int32
 }
 struct __PrivS2 {
-  var value: Int32
   init()
   init(value: Int32)
+  var value: Int32
 }
 var __PrivAnonymousA: Int { get }
 struct E0 : Equatable, RawRepresentable {

test/ClangImporter/ctypes_parse.swift

@@ -1,4 +1,4 @@
-// RUN: %target-typecheck-verify-swift %clang-importer-sdk
+// RUN: %target-typecheck-verify-swift %clang-importer-sdk -verify-ignore-unknown
 
 import ctypes
 

test/IDE/newtype.swift

@@ -128,9 +128,9 @@
 // PRINT-NEXT:  func takeMyABIOldTypeNonNullNS(_: String)
 //
 // PRINT-NEXT:  struct NSSomeContext {
-// PRINT-NEXT:    var i: Int32
 // PRINT-NEXT:    init()
 // PRINT-NEXT:    init(i: Int32)
+// PRINT-NEXT:    var i: Int32
 // PRINT-NEXT:  }
 // PRINT-NEXT:  extension NSSomeContext {
 // PRINT-NEXT:    struct Name : _ObjectiveCBridgeable, Hashable, Equatable, _SwiftNewtypeWrapper, RawRepresentable {