Clean up handling of external declarations, NFC

This is the first in a series of patches that fixes some resilience-related
issues with synthesized accessors and materializeForSet.

Previously we maintained two lists of external declarations encountered while
type checking:

- ASTContext::ExternalDefinitions
- TypeChecker::implicitlyDefinedFunctions

The former contained the following:

- Imported nominal types from Clang, so that SILGen can emit witness tables
- Functions and variables with Clang decls, so that IRGen can instruct Clang
  to emit them
- Synthesized accessors

The latter contained synthesized functions for derived conformances.

Since the second list was not visible outside Sema, we relied on the Clang
importer to add the type that contained the declaration to the
ExternalDefinitions list. In practice, we only synthesized members of enums
in this manner.

Because of this, SILGenModule::emitExternalDefinitions() had special logic to
skip members of enums, since it would visit them when visiting the enum itself.

Instead, it appears that we can remove implicitlyDefinedFunctions completely,
changing usage sites to add the decl to ExternalDefinitions instead, and
simplify SILGenModule::emitExternalDefinition() a bit in the process.

Also, it looks like we never had Modules appear in ExternalDefinitions, so
assert if those come up instead of skipping them.

lib/ClangImporter/ImportDecl.cpp

@@ -2073,7 +2073,7 @@ namespace {
       }
 
       // Add the type decl to ExternalDefinitions so that we can type-check
-      // raw values and IRGen can emit metadata for it.
+      // raw values and SILGen can emit witness tables for derived conformances.
       // FIXME: There might be better ways to do this.
       Impl.registerExternalDecl(result);
       return result;

lib/IRGen/GenDecl.cpp

@@ -1376,6 +1376,7 @@ void IRGenModule::emitExternalDefinition(Decl *D) {
   case DeclKind::PostfixOperator:
   case DeclKind::IfConfig:
   case DeclKind::Param:
+  case DeclKind::Module:
     llvm_unreachable("Not a valid external definition for IRgen");
 
   case DeclKind::Var:
@@ -1397,9 +1398,6 @@ void IRGenModule::emitExternalDefinition(Decl *D) {
   case DeclKind::Class:
   case DeclKind::Protocol:
     break;
-
-  case DeclKind::Module:
-    break;
   }
 }
 

lib/SILGen/SILGenDecl.cpp

@@ -1135,17 +1135,11 @@ void SILGenModule::emitExternalWitnessTable(ProtocolConformance *c) {
 void SILGenModule::emitExternalDefinition(Decl *d) {
   switch (d->getKind()) {
   case DeclKind::Func: {
-    // We'll emit all the members of an enum when we visit the enum.
-    if (isa<EnumDecl>(d->getDeclContext()))
-      break;
     emitFunction(cast<FuncDecl>(d));
     break;
   }
   case DeclKind::Constructor: {
     auto C = cast<ConstructorDecl>(d);
-    // We'll emit all the members of an enum when we visit the enum.
-    if (isa<EnumDecl>(d->getDeclContext()))
-      break;
     // For factories, we don't need to emit a special thunk; the normal
     // foreign-to-native thunk is sufficient.
     if (C->isFactoryInit())
@@ -1154,21 +1148,7 @@ void SILGenModule::emitExternalDefinition(Decl *d) {
     emitConstructor(C);
     break;
   }
-  case DeclKind::Enum: {
-    auto ed = cast<EnumDecl>(d);
-    // Emit derived conformance methods for the type.
-    for (auto member : ed->getMembers()) {
-      if (auto func = dyn_cast<FuncDecl>(member))
-        emitFunction(func);
-      else if (auto ctor = dyn_cast<ConstructorDecl>(member))
-        emitConstructor(ctor);
-    }
-    // Emit derived global decls.
-    for (auto derived : ed->getDerivedGlobalDecls()) {
-      emitFunction(cast<FuncDecl>(derived));
-    }
-    SWIFT_FALLTHROUGH;
-  }
+  case DeclKind::Enum:
   case DeclKind::Struct:
   case DeclKind::Class: {
     // Emit witness tables.
@@ -1190,9 +1170,6 @@ void SILGenModule::emitExternalDefinition(Decl *d) {
     // Imported static vars are handled solely in IRGen.
     break;
 
-  case DeclKind::Module:
-    break;
-
   case DeclKind::IfConfig:
   case DeclKind::Extension:
   case DeclKind::PatternBinding:
@@ -1209,6 +1186,7 @@ void SILGenModule::emitExternalDefinition(Decl *d) {
   case DeclKind::InfixOperator:
   case DeclKind::PrefixOperator:
   case DeclKind::PostfixOperator:
+  case DeclKind::Module:
     llvm_unreachable("Not a valid external definition for SILGen");
   }
 }

lib/Sema/DerivedConformanceEquatableHashable.cpp

@@ -266,10 +266,12 @@ deriveEquatable_enum_eq(TypeChecker &tc, Decl *parentDecl, EnumDecl *enumDecl) {
                                     Accessibility::Internal));
 
   if (enumDecl->hasClangNode())
-    tc.implicitlyDefinedFunctions.push_back(eqDecl);
+    tc.Context.addedExternalDecl(eqDecl);
 
   // Since it's an operator we insert the decl after the type at global scope.
-  return insertOperatorDecl(C, cast<IterableDeclContext>(parentDecl), eqDecl);
+  insertOperatorDecl(C, cast<IterableDeclContext>(parentDecl), eqDecl);
+
+  return eqDecl;
 }
 
 ValueDecl *DerivedConformance::deriveEquatable(TypeChecker &tc,
@@ -397,7 +399,7 @@ deriveHashable_enum_hashValue(TypeChecker &tc, Decl *parentDecl,
   getterDecl->setAccessibility(enumDecl->getFormalAccess());
 
   if (enumDecl->hasClangNode())
-    tc.implicitlyDefinedFunctions.push_back(getterDecl);
+    tc.Context.addedExternalDecl(getterDecl);
 
   // Create the property.
   VarDecl *hashValueDecl = new (C) VarDecl(/*static*/ false,

lib/Sema/DerivedConformanceRawRepresentable.cpp

@@ -337,7 +337,7 @@ static ConstructorDecl *deriveRawRepresentable_init(TypeChecker &tc,
   initDecl->setAccessibility(enumDecl->getFormalAccess());
 
   if (enumDecl->hasClangNode())
-    tc.implicitlyDefinedFunctions.push_back(initDecl);
+    tc.Context.addedExternalDecl(initDecl);
 
   cast<IterableDeclContext>(parentDecl)->addMember(initDecl);
   return initDecl;

lib/Sema/DerivedConformances.cpp

@@ -93,9 +93,9 @@ ValueDecl *DerivedConformance::getDerivableRequirement(NominalTypeDecl *nominal,
   return nullptr;
 }
 
-void DerivedConformance::_insertOperatorDecl(ASTContext &C,
-                                             IterableDeclContext *scope,
-                                             Decl *member) {
+void DerivedConformance::insertOperatorDecl(ASTContext &C,
+                                            IterableDeclContext *scope,
+                                            Decl *member) {
   // Find the module.
   auto mod = member->getModuleContext();
 
@@ -171,7 +171,7 @@ FuncDecl *DerivedConformance::declareDerivedPropertyGetter(TypeChecker &tc,
   getterDecl->setAccessibility(typeDecl->getFormalAccess());
 
   if (typeDecl->hasClangNode())
-    tc.implicitlyDefinedFunctions.push_back(getterDecl);
+    tc.Context.addedExternalDecl(getterDecl);
 
   return getterDecl;
 }

lib/Sema/DerivedConformances.h

@@ -109,19 +109,9 @@ ValueDecl *deriveBridgedNSError(TypeChecker &tc,
 
 /// Insert an operator declaration associated with a declaration
 /// context. The operator declaration is added at global scope.
-void _insertOperatorDecl(ASTContext &C,
-                         IterableDeclContext *scope,
-                         Decl *member);
-
-/// Insert an operator declaration associated with a declaration
-/// context. The operator declaration is added at global scope.
-template<typename SomeDecl>
-inline SomeDecl *insertOperatorDecl(ASTContext &C,
-                                    IterableDeclContext *scope,
-                                    SomeDecl *member) {
-  ::swift::DerivedConformance::_insertOperatorDecl(C, scope, member);
-  return member;
-}
+void insertOperatorDecl(ASTContext &C,
+                        IterableDeclContext *scope,
+                        Decl *member);
 
 /// Declare a getter for a derived property.
 FuncDecl *declareDerivedPropertyGetter(TypeChecker &tc,

lib/Sema/TypeChecker.cpp

@@ -388,6 +388,11 @@ static void typeCheckFunctionsAndExternalDecls(TypeChecker &TC) {
       auto decl = TC.Context.ExternalDefinitions[currentExternalDef];
 
       if (auto *AFD = dyn_cast<AbstractFunctionDecl>(decl)) {
+        // HACK: don't type-check the same function body twice.  This is
+        // supposed to be handled by just not enqueuing things twice,
+        // but that gets tricky with synthesized function bodies.
+        if (AFD->isBodyTypeChecked()) continue;
+
         PrettyStackTraceDecl StackEntry("type-checking", AFD);
         TC.typeCheckAbstractFunctionBody(AFD);
         continue;
@@ -464,11 +469,6 @@ static void typeCheckFunctionsAndExternalDecls(TypeChecker &TC) {
     }
     TC.UsedConformances.clear();
 
-    TC.definedFunctions.insert(TC.definedFunctions.end(),
-                               TC.implicitlyDefinedFunctions.begin(),
-                               TC.implicitlyDefinedFunctions.end());
-    TC.implicitlyDefinedFunctions.clear();
-
   } while (currentFunctionIdx < TC.definedFunctions.size() ||
            currentExternalDef < TC.Context.ExternalDefinitions.size() ||
            !TC.UsedConformances.empty());
@@ -524,7 +524,6 @@ void swift::performTypeChecking(SourceFile &SF, TopLevelContext &TLC,
     TypeChecker TC(Ctx);
     SharedTimer timer("Type checking / Semantic analysis");
 
-    auto &DefinedFunctions = TC.definedFunctions;
     if (Options.contains(TypeCheckingFlags::DebugTimeFunctionBodies))
       TC.enableDebugTimeFunctionBodies();
 
@@ -593,11 +592,6 @@ void swift::performTypeChecking(SourceFile &SF, TopLevelContext &TLC,
       TC.contextualizeTopLevelCode(TLC,
                              llvm::makeArrayRef(SF.Decls).slice(StartElem));
     }
-
-    DefinedFunctions.insert(DefinedFunctions.end(),
-                            TC.implicitlyDefinedFunctions.begin(),
-                            TC.implicitlyDefinedFunctions.end());
-    TC.implicitlyDefinedFunctions.clear();
 
     // If we're in REPL mode, inject temporary result variables and other stuff
     // that the REPL needs to synthesize.

lib/Sema/TypeChecker.h

@@ -432,10 +432,6 @@ class TypeChecker final : public LazyResolver {
   ASTContext &Context;
   DiagnosticEngine &Diags;
 
-  /// \brief The list of implicitly-defined functions created by the
-  /// type checker.
-  std::vector<AbstractFunctionDecl *> implicitlyDefinedFunctions;
-
   /// \brief The list of function definitions we've encountered.
   std::vector<AbstractFunctionDecl *> definedFunctions;