Skip to content

Commit b7bfaf3

Browse files
committed
[Macros] Fix handling of extension macro conformances and witnesses
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
1 parent 5edfc26 commit b7bfaf3

12 files changed

+108
-49
lines changed

include/swift/AST/DeclContext.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -175,9 +175,6 @@ enum class ConformanceLookupKind : unsigned {
175175
/// All conformances except structurally-derived conformances, of which
176176
/// Sendable is the only one.
177177
NonStructural,
178-
/// Exclude conformances added by a macro that has not been expanded
179-
/// yet.
180-
ExcludeUnexpandedMacros,
181178
};
182179

183180
/// Describes a diagnostic for a conflict between two protocol

include/swift/AST/ProtocolConformance.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -552,6 +552,8 @@ class NormalProtocolConformance : public RootProtocolConformance,
552552
assert((sourceKind == ConformanceEntryKind::Implied) ==
553553
(bool)implyingConformance &&
554554
"an implied conformance needs something that implies it");
555+
assert(sourceKind != ConformanceEntryKind::PreMacroExpansion &&
556+
"cannot create conformance pre-macro-expansion");
555557
SourceKindAndImplyingConformance = {implyingConformance, sourceKind};
556558
}
557559

lib/AST/ConformanceLookupTable.cpp

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -892,6 +892,16 @@ ConformanceLookupTable::getConformance(NominalTypeDecl *nominal,
892892
if (!conformingDC)
893893
return nullptr;
894894

895+
// Never produce a conformance for a pre-macro-expansion conformance. They
896+
// are placeholders that will be superseded.
897+
if (entry->getKind() == ConformanceEntryKind::PreMacroExpansion) {
898+
if (auto supersedingEntry = entry->SupersededBy) {
899+
return getConformance(nominal, supersedingEntry);
900+
}
901+
902+
return nullptr;
903+
}
904+
895905
auto *conformingNominal = conformingDC->getSelfNominalTypeDecl();
896906

897907
// Form the conformance.
@@ -929,6 +939,16 @@ ConformanceLookupTable::getConformance(NominalTypeDecl *nominal,
929939
? conformingNominal->getLoc()
930940
: cast<ExtensionDecl>(conformingDC)->getLoc();
931941

942+
NormalProtocolConformance *implyingConf = nullptr;
943+
if (entry->Source.getKind() == ConformanceEntryKind::Implied) {
944+
auto implyingEntry = entry->Source.getImpliedSource();
945+
auto origImplyingConf = getConformance(conformingNominal, implyingEntry);
946+
if (!origImplyingConf)
947+
return nullptr;
948+
949+
implyingConf = origImplyingConf->getRootNormalConformance();
950+
}
951+
932952
// Create or find the normal conformance.
933953
auto normalConf =
934954
ctx.getNormalConformance(conformingType, protocol, conformanceLoc,
@@ -939,12 +959,6 @@ ConformanceLookupTable::getConformance(NominalTypeDecl *nominal,
939959
// early return at the start of this function.
940960
entry->Conformance = normalConf;
941961

942-
NormalProtocolConformance *implyingConf = nullptr;
943-
if (entry->Source.getKind() == ConformanceEntryKind::Implied) {
944-
auto implyingEntry = entry->Source.getImpliedSource();
945-
implyingConf = getConformance(conformingNominal, implyingEntry)
946-
->getRootNormalConformance();
947-
}
948962
normalConf->setSourceKindAndImplyingConformance(entry->Source.getKind(),
949963
implyingConf);
950964

lib/AST/NameLookup.cpp

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1757,6 +1757,17 @@ namespace {
17571757
};
17581758
}
17591759

1760+
/// Given an extension declaration, return the extended nominal type if the
1761+
/// extension was produced by expanding an extension or conformance macro from
1762+
/// the nominal declaration itself.
1763+
static NominalTypeDecl *nominalForExpandedExtensionDecl(ExtensionDecl *ext) {
1764+
if (!ext->isInMacroExpansionInContext())
1765+
return nullptr;
1766+
1767+
1768+
return ext->getSelfNominalTypeDecl();
1769+
}
1770+
17601771
PotentialMacroExpansions PotentialMacroExpansionsInContextRequest::evaluate(
17611772
Evaluator &evaluator, TypeOrExtensionDecl container) const {
17621773
/// The implementation here needs to be kept in sync with
@@ -1767,6 +1778,15 @@ PotentialMacroExpansions PotentialMacroExpansionsInContextRequest::evaluate(
17671778
auto containerDecl = container.getAsDecl();
17681779
forEachPotentialAttachedMacro(containerDecl, MacroRole::Member, nameTracker);
17691780

1781+
// If the container is an extension that was created from an extension macro,
1782+
// look at the nominal declaration to find any extension macros.
1783+
if (auto ext = dyn_cast<ExtensionDecl>(containerDecl)) {
1784+
if (auto nominal = nominalForExpandedExtensionDecl(ext)) {
1785+
forEachPotentialAttachedMacro(
1786+
nominal, MacroRole::Extension, nameTracker);
1787+
}
1788+
}
1789+
17701790
// Peer and freestanding declaration macros.
17711791
auto dc = container.getAsDeclContext();
17721792
auto idc = container.getAsIterableDeclContext();
@@ -1825,13 +1845,15 @@ populateLookupTableEntryFromMacroExpansions(ASTContext &ctx,
18251845
// names match.
18261846
{
18271847
MacroIntroducedNameTracker nameTracker;
1828-
if (auto nominal = dyn_cast<NominalTypeDecl>(container.getAsDecl())) {
1829-
forEachPotentialAttachedMacro(nominal, MacroRole::Extension, nameTracker);
1830-
if (nameTracker.shouldExpandForName(name)) {
1831-
(void)evaluateOrDefault(
1832-
ctx.evaluator,
1833-
ExpandExtensionMacros{nominal},
1834-
false);
1848+
if (auto ext = dyn_cast<ExtensionDecl>(container.getAsDecl())) {
1849+
if (auto nominal = nominalForExpandedExtensionDecl(ext)) {
1850+
forEachPotentialAttachedMacro(nominal, MacroRole::Extension, nameTracker);
1851+
if (nameTracker.shouldExpandForName(name)) {
1852+
(void)evaluateOrDefault(
1853+
ctx.evaluator,
1854+
ExpandExtensionMacros{nominal},
1855+
false);
1856+
}
18351857
}
18361858
}
18371859
}

lib/AST/ProtocolConformance.cpp

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1295,8 +1295,8 @@ IterableDeclContext::getLocalConformances(ConformanceLookupKind lookupKind)
12951295
switch (conformance->getSourceKind()) {
12961296
case ConformanceEntryKind::Explicit:
12971297
case ConformanceEntryKind::Synthesized:
1298+
return true;
12981299
case ConformanceEntryKind::PreMacroExpansion:
1299-
return true;
13001300
case ConformanceEntryKind::Implied:
13011301
case ConformanceEntryKind::Inherited:
13021302
return false;
@@ -1307,34 +1307,22 @@ IterableDeclContext::getLocalConformances(ConformanceLookupKind lookupKind)
13071307
case ConformanceEntryKind::Explicit:
13081308
case ConformanceEntryKind::Synthesized:
13091309
case ConformanceEntryKind::Implied:
1310-
case ConformanceEntryKind::PreMacroExpansion:
13111310
return true;
13121311
case ConformanceEntryKind::Inherited:
1312+
case ConformanceEntryKind::PreMacroExpansion:
13131313
return false;
13141314
}
13151315

13161316
case ConformanceLookupKind::All:
13171317
case ConformanceLookupKind::NonStructural:
13181318
return true;
1319-
1320-
case ConformanceLookupKind::ExcludeUnexpandedMacros:
1321-
switch (conformance->getSourceKind()) {
1322-
case ConformanceEntryKind::PreMacroExpansion:
1323-
return false;
1324-
case ConformanceEntryKind::Explicit:
1325-
case ConformanceEntryKind::Synthesized:
1326-
case ConformanceEntryKind::Implied:
1327-
case ConformanceEntryKind::Inherited:
1328-
return true;
1329-
}
13301319
}
13311320
});
13321321

13331322
// If we want to add structural conformances, do so now.
13341323
switch (lookupKind) {
13351324
case ConformanceLookupKind::All:
1336-
case ConformanceLookupKind::NonInherited:
1337-
case ConformanceLookupKind::ExcludeUnexpandedMacros: {
1325+
case ConformanceLookupKind::NonInherited: {
13381326
// Look for a Sendable conformance globally. If it is synthesized
13391327
// and matches this declaration context, use it.
13401328
auto dc = getAsGenericContext();

lib/SIL/IR/SILSymbolVisitor.cpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -238,9 +238,6 @@ class SILSymbolVisitorImpl : public ASTVisitor<SILSymbolVisitorImpl> {
238238
void addConformances(const IterableDeclContext *IDC) {
239239
for (auto conformance :
240240
IDC->getLocalConformances(ConformanceLookupKind::NonInherited)) {
241-
if (conformance->getSourceKind() == ConformanceEntryKind::PreMacroExpansion)
242-
continue;
243-
244241
auto protocol = conformance->getProtocol();
245242
if (Ctx.getOpts().PublicSymbolsOnly &&
246243
getDeclLinkage(protocol) != FormalLinkage::PublicUnique)

lib/SILGen/SILGenType.cpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1132,9 +1132,6 @@ class SILGenType : public TypeMemberVisitor<SILGenType> {
11321132
// are existential and do not have witness tables.
11331133
for (auto *conformance : theType->getLocalConformances(
11341134
ConformanceLookupKind::NonInherited)) {
1135-
if (conformance->getSourceKind() == ConformanceEntryKind::PreMacroExpansion)
1136-
continue;
1137-
11381135
assert(conformance->isComplete());
11391136
if (auto *normal = dyn_cast<NormalProtocolConformance>(conformance))
11401137
SGM.getWitnessTable(normal);

lib/Sema/TypeCheckMacros.cpp

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1592,15 +1592,7 @@ swift::expandExtensions(CustomAttr *attr, MacroDecl *macro,
15921592
for (auto protocol : potentialConformances) {
15931593
SmallVector<ProtocolConformance *, 2> existingConformances;
15941594
nominal->lookupConformance(protocol, existingConformances);
1595-
1596-
bool hasExistingConformance = llvm::any_of(
1597-
existingConformances,
1598-
[&](ProtocolConformance *conformance) {
1599-
return conformance->getSourceKind() !=
1600-
ConformanceEntryKind::PreMacroExpansion;
1601-
});
1602-
1603-
if (!hasExistingConformance) {
1595+
if (existingConformances.empty()) {
16041596
introducedConformances.push_back(protocol);
16051597
}
16061598
}

lib/Sema/TypeCheckProtocol.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6417,8 +6417,7 @@ void TypeChecker::checkConformancesInContext(IterableDeclContext *idc) {
64176417
const auto defaultAccess = nominal->getFormalAccess();
64186418

64196419
// Check each of the conformances associated with this context.
6420-
auto conformances = idc->getLocalConformances(
6421-
ConformanceLookupKind::ExcludeUnexpandedMacros);
6420+
auto conformances = idc->getLocalConformances();
64226421

64236422
// The conformance checker bundle that checks all conformances in the context.
64246423
auto &Context = dc->getASTContext();

test/IDE/complete_optionset.swift

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ public macro OptionSet<RawType>() =
2828
// MEMBER_STATIC: Decl[StaticVar]/CurrNominal: secondDay[#ShippingOptions#]; name=secondDay
2929
// MEMBER_STATIC: Decl[StaticVar]/CurrNominal: priority[#ShippingOptions#]; name=priority
3030
// MEMBER_STATIC: Decl[StaticVar]/CurrNominal: standard[#ShippingOptions#]; name=standard
31-
// MEMBER_STATIC: Decl[TypeAlias]/CurrNominal: ArrayLiteralElement[#ShippingOptions#]; name=ArrayLiteralElement
3231
// MEMBER_STATIC: Decl[TypeAlias]/CurrNominal: Element[#ShippingOptions#]; name=Element
3332
// MEMBER_STATIC: Decl[Constructor]/Super/IsSystem: init()[#ShippingOptions#]; name=init()
3433
// MEMBER_STATIC: Decl[Constructor]/Super/IsSystem: init({#(sequence): Sequence#})[#ShippingOptions#]; name=init(:)

test/Macros/Inputs/syntax_macro_definitions.swift

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1290,6 +1290,39 @@ public struct EquatableMacro: ExtensionMacro {
12901290
}
12911291
}
12921292

1293+
public struct EquatableViaMembersMacro: ExtensionMacro {
1294+
public static func expansion(
1295+
of node: AttributeSyntax,
1296+
attachedTo decl: some DeclGroupSyntax,
1297+
providingExtensionsOf type: some TypeSyntaxProtocol,
1298+
conformingTo protocols: [TypeSyntax],
1299+
in context: some MacroExpansionContext
1300+
) throws -> [ExtensionDeclSyntax] {
1301+
let comparisons: [String] = decl.storedProperties().map { property in
1302+
guard let binding = property.bindings.first,
1303+
let identifier = binding.pattern.as(IdentifierPatternSyntax.self)?.identifier else {
1304+
return "true"
1305+
}
1306+
1307+
return "lhs.\(identifier) == rhs.\(identifier)"
1308+
}
1309+
1310+
let condition = comparisons.joined(separator: " && ")
1311+
let equalOperator: DeclSyntax = """
1312+
static func ==(lhs: \(type.trimmed), rhs: \(type.trimmed)) -> Bool {
1313+
return \(raw: condition)
1314+
}
1315+
"""
1316+
1317+
let ext: DeclSyntax = """
1318+
extension \(type.trimmed): Equatable {
1319+
\(equalOperator)
1320+
}
1321+
"""
1322+
return [ext.cast(ExtensionDeclSyntax.self)]
1323+
}
1324+
}
1325+
12931326
public struct ConformanceViaExtensionMacro: ExtensionMacro {
12941327
public static func expansion(
12951328
of node: AttributeSyntax,

test/Macros/macro_expand_extensions.swift

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,3 +179,22 @@ struct MultipleConformances {}
179179
// CHECK-DUMP: }
180180
// CHECK-DUMP: extension MultipleConformances: P2 {
181181
// CHECK-DUMP: }
182+
183+
@attached(extension, conformances: Equatable, names: named(==))
184+
macro Equatable() = #externalMacro(module: "MacroDefinition", type: "EquatableViaMembersMacro")
185+
186+
@propertyWrapper
187+
struct NotEquatable<T> {
188+
var wrappedValue: T
189+
}
190+
191+
@Equatable
192+
struct HasPropertyWrappers {
193+
@NotEquatable
194+
var value: Int = 0
195+
}
196+
197+
func requiresEquatable<T: Equatable>(_: T) { }
198+
func testHasPropertyWrappers(hpw: HasPropertyWrappers) {
199+
requiresEquatable(hpw)
200+
}

0 commit comments

Comments
 (0)