[Macros] Fix handling of extension macro conformances and witnesses

DougGregor · DougGregor · commit 2aaa9831cf90 · 2023-08-28T14:07:20.000-07:00
Fix two inter-related issues with extension macros that provide conformances to a protocol, the combined effect of which is that one cannot meaningfully provide extension macros that implement conformances to a protocol like Equatable or Hashable that also supports auto-synthesis. The first issue involves name lookup of operators provided by macro expansions. The logic for performing qualified lookup in addition to unqualified lookup (for operators) did not account for extension macros in the same manner as it did for member macros, so we would not find a macro-produced operator (such as operator==) in witness matching. The second issue is more fundamental, which is that the conformance lookup table would create `NormalProtocolConformance` instances for pre-macro-expansion conformance entries, even though these should always have been superseded by explicit conformances within the macro expansion buffers. The end result is that we could end up with two `NormalProtocolConformance` records for the same conformance. Some code was taught to ignore the pre-expansion placeholder conformances, other code was not. Instead, we now refuse to create a `NormalProtocolConformance` for the pre-expansion entries, and remove all of the special-case checks for this, so we always using the superseding explicit conformances produced by the macro expansions (or error if the macros don't produce them). Fixes rdar://113994346 / #66348 (cherry picked from commit fa48076)
diff --git a/include/swift/AST/DeclContext.h b/include/swift/AST/DeclContext.h
@@ -172,9 +172,6 @@ enum class ConformanceLookupKind : unsigned {
   /// All conformances except structurally-derived conformances, of which
   /// Sendable is the only one.
   NonStructural,
-  /// Exclude conformances added by a macro that has not been expanded
-  /// yet.
-  ExcludeUnexpandedMacros,
 };
 
 /// Describes a diagnostic for a conflict between two protocol
diff --git a/include/swift/AST/ProtocolConformance.h b/include/swift/AST/ProtocolConformance.h
@@ -552,6 +552,8 @@ class NormalProtocolConformance : public RootProtocolConformance,
     assert((sourceKind == ConformanceEntryKind::Implied) ==
                (bool)implyingConformance &&
            "an implied conformance needs something that implies it");
+    assert(sourceKind != ConformanceEntryKind::PreMacroExpansion &&
+           "cannot create conformance pre-macro-expansion");
     SourceKindAndImplyingConformance = {implyingConformance, sourceKind};
   }
 
diff --git a/lib/AST/ConformanceLookupTable.cpp b/lib/AST/ConformanceLookupTable.cpp
@@ -889,6 +889,16 @@ ConformanceLookupTable::getConformance(NominalTypeDecl *nominal,
   if (!conformingDC)
     return nullptr;
 
+  // Never produce a conformance for a pre-macro-expansion conformance. They
+  // are placeholders that will be superseded.
+  if (entry->getKind() == ConformanceEntryKind::PreMacroExpansion) {
+    if (auto supersedingEntry = entry->SupersededBy) {
+      return getConformance(nominal, supersedingEntry);
+    }
+
+    return nullptr;
+  }
+
   auto *conformingNominal = conformingDC->getSelfNominalTypeDecl();
 
   // Form the conformance.
@@ -926,6 +936,16 @@ ConformanceLookupTable::getConformance(NominalTypeDecl *nominal,
           ? conformingNominal->getLoc()
           : cast<ExtensionDecl>(conformingDC)->getLoc();
 
+    NormalProtocolConformance *implyingConf = nullptr;
+    if (entry->Source.getKind() == ConformanceEntryKind::Implied) {
+      auto implyingEntry = entry->Source.getImpliedSource();
+      auto origImplyingConf = getConformance(conformingNominal, implyingEntry);
+      if (!origImplyingConf)
+        return nullptr;
+
+      implyingConf = origImplyingConf->getRootNormalConformance();
+    }
+
     // Create or find the normal conformance.
     auto normalConf =
         ctx.getConformance(conformingType, protocol, conformanceLoc,
@@ -936,12 +956,6 @@ ConformanceLookupTable::getConformance(NominalTypeDecl *nominal,
     // early return at the start of this function.
     entry->Conformance = normalConf;
 
-    NormalProtocolConformance *implyingConf = nullptr;
-    if (entry->Source.getKind() == ConformanceEntryKind::Implied) {
-      auto implyingEntry = entry->Source.getImpliedSource();
-      implyingConf = getConformance(conformingNominal, implyingEntry)
-                         ->getRootNormalConformance();
-    }
     normalConf->setSourceKindAndImplyingConformance(entry->Source.getKind(),
                                                     implyingConf);
 
diff --git a/lib/AST/NameLookup.cpp b/lib/AST/NameLookup.cpp
@@ -1757,6 +1757,17 @@ namespace {
   };
 }
 
+/// Given an extension declaration, return the extended nominal type if the
+/// extension was produced by expanding an extension or conformance macro from
+/// the nominal declaration itself.
+static NominalTypeDecl *nominalForExpandedExtensionDecl(ExtensionDecl *ext) {
+  if (!ext->isInMacroExpansionInContext())
+    return nullptr;
+
+
+  return ext->getSelfNominalTypeDecl();
+}
+
 PotentialMacroExpansions PotentialMacroExpansionsInContextRequest::evaluate(
     Evaluator &evaluator, TypeOrExtensionDecl container) const {
   /// The implementation here needs to be kept in sync with
@@ -1767,6 +1778,15 @@ PotentialMacroExpansions PotentialMacroExpansionsInContextRequest::evaluate(
   auto containerDecl = container.getAsDecl();
   forEachPotentialAttachedMacro(containerDecl, MacroRole::Member, nameTracker);
 
+  // If the container is an extension that was created from an extension macro,
+  // look at the nominal declaration to find any extension macros.
+  if (auto ext = dyn_cast<ExtensionDecl>(containerDecl)) {
+    if (auto nominal = nominalForExpandedExtensionDecl(ext)) {
+      forEachPotentialAttachedMacro(
+          nominal, MacroRole::Extension, nameTracker);
+    }
+  }
+
   // Peer and freestanding declaration macros.
   auto dc = container.getAsDeclContext();
   auto idc = container.getAsIterableDeclContext();
@@ -1825,13 +1845,15 @@ populateLookupTableEntryFromMacroExpansions(ASTContext &ctx,
   // names match.
   {
     MacroIntroducedNameTracker nameTracker;
-    if (auto nominal = dyn_cast<NominalTypeDecl>(container.getAsDecl())) {
-      forEachPotentialAttachedMacro(nominal, MacroRole::Extension, nameTracker);
-      if (nameTracker.shouldExpandForName(name)) {
-        (void)evaluateOrDefault(
-            ctx.evaluator,
-            ExpandExtensionMacros{nominal},
-            false);
+    if (auto ext = dyn_cast<ExtensionDecl>(container.getAsDecl())) {
+      if (auto nominal = nominalForExpandedExtensionDecl(ext)) {
+        forEachPotentialAttachedMacro(nominal, MacroRole::Extension, nameTracker);
+        if (nameTracker.shouldExpandForName(name)) {
+          (void)evaluateOrDefault(
+              ctx.evaluator,
+              ExpandExtensionMacros{nominal},
+              false);
+        }
       }
     }
   }
diff --git a/lib/AST/ProtocolConformance.cpp b/lib/AST/ProtocolConformance.cpp
@@ -1302,8 +1302,8 @@ IterableDeclContext::getLocalConformances(ConformanceLookupKind lookupKind)
            switch (conformance->getSourceKind()) {
            case ConformanceEntryKind::Explicit:
            case ConformanceEntryKind::Synthesized:
+               return true;
            case ConformanceEntryKind::PreMacroExpansion:
-             return true;
            case ConformanceEntryKind::Implied:
            case ConformanceEntryKind::Inherited:
              return false;
@@ -1314,34 +1314,22 @@ IterableDeclContext::getLocalConformances(ConformanceLookupKind lookupKind)
            case ConformanceEntryKind::Explicit:
            case ConformanceEntryKind::Synthesized:
            case ConformanceEntryKind::Implied:
-           case ConformanceEntryKind::PreMacroExpansion:
              return true;
            case ConformanceEntryKind::Inherited:
+           case ConformanceEntryKind::PreMacroExpansion:
              return false;
            }
 
          case ConformanceLookupKind::All:
          case ConformanceLookupKind::NonStructural:
            return true;
-
-          case ConformanceLookupKind::ExcludeUnexpandedMacros:
-           switch (conformance->getSourceKind()) {
-           case ConformanceEntryKind::PreMacroExpansion:
-             return false;
-           case ConformanceEntryKind::Explicit:
-           case ConformanceEntryKind::Synthesized:
-           case ConformanceEntryKind::Implied:
-           case ConformanceEntryKind::Inherited:
-             return true;
-           }
          }
       });
 
   // If we want to add structural conformances, do so now.
   switch (lookupKind) {
     case ConformanceLookupKind::All:
-    case ConformanceLookupKind::NonInherited:
-    case ConformanceLookupKind::ExcludeUnexpandedMacros: {
+    case ConformanceLookupKind::NonInherited: {
       // Look for a Sendable conformance globally. If it is synthesized
       // and matches this declaration context, use it.
       auto dc = getAsGenericContext();
diff --git a/lib/SIL/IR/SILSymbolVisitor.cpp b/lib/SIL/IR/SILSymbolVisitor.cpp
@@ -238,9 +238,6 @@ class SILSymbolVisitorImpl : public ASTVisitor<SILSymbolVisitorImpl> {
   void addConformances(const IterableDeclContext *IDC) {
     for (auto conformance :
          IDC->getLocalConformances(ConformanceLookupKind::NonInherited)) {
-      if (conformance->getSourceKind() == ConformanceEntryKind::PreMacroExpansion)
-        continue;
-
       auto protocol = conformance->getProtocol();
       if (Ctx.getOpts().PublicSymbolsOnly &&
           getDeclLinkage(protocol) != FormalLinkage::PublicUnique)
diff --git a/lib/SILGen/SILGenType.cpp b/lib/SILGen/SILGenType.cpp
@@ -1132,9 +1132,6 @@ class SILGenType : public TypeMemberVisitor<SILGenType> {
     // are existential and do not have witness tables.
     for (auto *conformance : theType->getLocalConformances(
                                ConformanceLookupKind::NonInherited)) {
-      if (conformance->getSourceKind() == ConformanceEntryKind::PreMacroExpansion)
-        continue;
-
       assert(conformance->isComplete());
       if (auto *normal = dyn_cast<NormalProtocolConformance>(conformance))
         SGM.getWitnessTable(normal);
diff --git a/lib/Sema/TypeCheckMacros.cpp b/lib/Sema/TypeCheckMacros.cpp
@@ -1593,15 +1593,7 @@ llvm::Optional<unsigned> swift::expandExtensions(CustomAttr *attr,
   for (auto protocol : potentialConformances) {
     SmallVector<ProtocolConformance *, 2> existingConformances;
     nominal->lookupConformance(protocol, existingConformances);
-
-    bool hasExistingConformance = llvm::any_of(
-        existingConformances,
-        [&](ProtocolConformance *conformance) {
-          return conformance->getSourceKind() !=
-              ConformanceEntryKind::PreMacroExpansion;
-        });
-
-    if (!hasExistingConformance) {
+    if (existingConformances.empty()) {
       introducedConformances.push_back(protocol);
     }
   }
diff --git a/lib/Sema/TypeCheckProtocol.cpp b/lib/Sema/TypeCheckProtocol.cpp
@@ -6443,8 +6443,7 @@ void TypeChecker::checkConformancesInContext(IterableDeclContext *idc) {
   const auto defaultAccess = nominal->getFormalAccess();
 
   // Check each of the conformances associated with this context.
-  auto conformances = idc->getLocalConformances(
-      ConformanceLookupKind::ExcludeUnexpandedMacros);
+  auto conformances = idc->getLocalConformances();
 
   // The conformance checker bundle that checks all conformances in the context.
   auto &Context = dc->getASTContext();
diff --git a/test/IDE/complete_optionset.swift b/test/IDE/complete_optionset.swift
@@ -28,7 +28,6 @@ public macro OptionSet<RawType>() =
 // MEMBER_STATIC: Decl[StaticVar]/CurrNominal:        secondDay[#ShippingOptions#]; name=secondDay
 // MEMBER_STATIC: Decl[StaticVar]/CurrNominal:        priority[#ShippingOptions#]; name=priority
 // MEMBER_STATIC: Decl[StaticVar]/CurrNominal:        standard[#ShippingOptions#]; name=standard
-// MEMBER_STATIC: Decl[TypeAlias]/CurrNominal:        ArrayLiteralElement[#ShippingOptions#]; name=ArrayLiteralElement
 // MEMBER_STATIC: Decl[TypeAlias]/CurrNominal:        Element[#ShippingOptions#]; name=Element
 // MEMBER_STATIC: Decl[Constructor]/Super/IsSystem:   init()[#ShippingOptions#]; name=init()
 // MEMBER_STATIC: Decl[Constructor]/Super/IsSystem:   init({#(sequence): Sequence#})[#ShippingOptions#]; name=init(:)
diff --git a/test/Macros/Inputs/syntax_macro_definitions.swift b/test/Macros/Inputs/syntax_macro_definitions.swift
@@ -1335,6 +1335,39 @@ public struct EquatableMacro: ConformanceMacro {
   }
 }
 
+public struct EquatableViaMembersMacro: ExtensionMacro {
+  public static func expansion(
+    of node: AttributeSyntax,
+    attachedTo decl: some DeclGroupSyntax,
+    providingExtensionsOf type: some TypeSyntaxProtocol,
+    conformingTo protocols: [TypeSyntax],
+    in context: some MacroExpansionContext
+  ) throws -> [ExtensionDeclSyntax] {
+    let comparisons: [String] = decl.storedProperties().map { property in
+      guard let binding = property.bindings.first,
+            let identifier = binding.pattern.as(IdentifierPatternSyntax.self)?.identifier else {
+        return "true"
+      }
+
+      return "lhs.\(identifier) == rhs.\(identifier)"
+    }
+
+    let condition = comparisons.joined(separator: " && ")
+    let equalOperator: DeclSyntax = """
+      static func ==(lhs: \(type.trimmed), rhs: \(type.trimmed)) -> Bool {
+        return \(raw: condition)
+      }
+      """
+
+    let ext: DeclSyntax = """
+      extension \(type.trimmed): Equatable {
+        \(equalOperator)
+      }
+      """
+    return [ext.cast(ExtensionDeclSyntax.self)]
+  }
+}
+
 public struct ConformanceViaExtensionMacro: ExtensionMacro {
   public static func expansion(
     of node: AttributeSyntax,
diff --git a/test/Macros/macro_expand_extensions.swift b/test/Macros/macro_expand_extensions.swift
@@ -165,3 +165,22 @@ macro AvailableEquatable() = #externalMacro(module: "MacroDefinition", type: "Co
 struct TestAvailability {
   static let x : any Equatable.Type = TestAvailability.self
 }
+
+@attached(extension, conformances: Equatable, names: named(==))
+macro Equatable() = #externalMacro(module: "MacroDefinition", type: "EquatableViaMembersMacro")
+
+@propertyWrapper
+struct NotEquatable<T> {
+  var wrappedValue: T
+}
+
+@Equatable
+struct HasPropertyWrappers {
+  @NotEquatable
+  var value: Int = 0
+}
+
+func requiresEquatable<T: Equatable>(_: T) { }
+func testHasPropertyWrappers(hpw: HasPropertyWrappers) {
+  requiresEquatable(hpw)
+}

Original file line number	Diff line number	Diff line change
`@@ -552,6 +552,8 @@ class NormalProtocolConformance : public RootProtocolConformance,`
`552`	`552`	`assert((sourceKind == ConformanceEntryKind::Implied) ==`
`553`	`553`	`(bool)implyingConformance &&`
`554`	`554`	`"an implied conformance needs something that implies it");`
	`555`	`+ assert(sourceKind != ConformanceEntryKind::PreMacroExpansion &&`
	`556`	`+ "cannot create conformance pre-macro-expansion");`
`555`	`557`	`SourceKindAndImplyingConformance = {implyingConformance, sourceKind};`
`556`	`558`	`}`
`557`	`559`