Skip to content

Commit 83030a9

Browse files
committed
[Macros] Don't visit macro-generated extensions in 'visitAuxiliaryDecls'.
Macro-generated extensions are hoisted to file scope, because extensions are not valid in nested scopes. Callers of 'visitAuxiliaryDecls' assume that the auxiliary decls are in the same decl context as the original, which is clearly not the case for extensions, and it leads to issues like visiting extension at the wrong time during SILGen. The extensions are already added to the top-level decls, so we don't need to visit them as auxiliary decls, and we can type-check macro-expanded decls at the end of visitation in TypeCheckDeclPrimary.
1 parent 3676379 commit 83030a9

File tree

11 files changed

+67
-43
lines changed

11 files changed

+67
-43
lines changed

include/swift/AST/Decl.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -924,6 +924,12 @@ class alignas(1 << DeclAlignInBits) Decl : public ASTAllocated<Decl> {
924924
bool visitFreestandingExpanded = true
925925
) const;
926926

927+
using ExtensionCallback = llvm::function_ref<void(ExtensionDecl *)>;
928+
929+
/// Iterate over each macro-expanded extension for this declaration,
930+
/// invoking the given callback with each extension.
931+
void forEachExpandedExtension(ExtensionCallback callback) const;
932+
927933
using MacroCallback = llvm::function_ref<void(CustomAttr *, MacroDecl *)>;
928934

929935
/// Iterate over each attached macro with the given role, invoking the

lib/AST/Decl.cpp

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -407,20 +407,6 @@ void Decl::visitAuxiliaryDecls(
407407
}
408408
}
409409

410-
if (auto *nominal = dyn_cast<NominalTypeDecl>(mutableThis)) {
411-
auto conformanceBuffers =
412-
evaluateOrDefault(ctx.evaluator,
413-
ExpandConformanceMacros{nominal},
414-
{});
415-
for (auto bufferID : conformanceBuffers) {
416-
auto startLoc = sourceMgr.getLocForBufferStart(bufferID);
417-
auto *sourceFile = moduleDecl->getSourceFileContainingLocation(startLoc);
418-
for (auto *extension : sourceFile->getTopLevelDecls()) {
419-
callback(extension);
420-
}
421-
}
422-
}
423-
424410
if (visitFreestandingExpanded) {
425411
if (auto *med = dyn_cast<MacroExpansionDecl>(mutableThis)) {
426412
if (auto bufferID = evaluateOrDefault(
@@ -436,6 +422,29 @@ void Decl::visitAuxiliaryDecls(
436422
// FIXME: fold VarDecl::visitAuxiliaryDecls into this.
437423
}
438424

425+
void
426+
Decl::forEachExpandedExtension(ExtensionCallback callback) const {
427+
auto &ctx = getASTContext();
428+
auto *mutableThis = const_cast<Decl *>(this);
429+
auto *nominal = dyn_cast<NominalTypeDecl>(mutableThis);
430+
if (!nominal)
431+
return;
432+
433+
SourceManager &sourceMgr = ctx.SourceMgr;
434+
auto *moduleDecl = getModuleContext();
435+
auto conformanceBuffers =
436+
evaluateOrDefault(ctx.evaluator,
437+
ExpandConformanceMacros{nominal},
438+
{});
439+
for (auto bufferID : conformanceBuffers) {
440+
auto startLoc = sourceMgr.getLocForBufferStart(bufferID);
441+
auto *sourceFile = moduleDecl->getSourceFileContainingLocation(startLoc);
442+
for (auto *extension : sourceFile->getTopLevelDecls()) {
443+
callback(cast<ExtensionDecl>(extension));
444+
}
445+
}
446+
}
447+
439448
void Decl::forEachAttachedMacro(MacroRole role,
440449
MacroCallback macroCallback) const {
441450
for (auto customAttrConst : getSemanticAttrs().getAttributes<CustomAttr>()) {

lib/AST/Module.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4104,8 +4104,7 @@ void FileUnit::getTopLevelDeclsWithAuxiliaryDecls(
41044104
getTopLevelDecls(nonExpandedDecls);
41054105
for (auto *decl : nonExpandedDecls) {
41064106
decl->visitAuxiliaryDecls([&](Decl *auxDecl) {
4107-
if (!isa<ExtensionDecl>(auxDecl))
4108-
results.push_back(auxDecl);
4107+
results.push_back(auxDecl);
41094108
});
41104109
results.push_back(decl);
41114110
}

lib/IRGen/GenDecl.cpp

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5578,11 +5578,6 @@ void IRGenModule::emitNestedTypeDecls(DeclRange members) {
55785578
continue;
55795579

55805580
member->visitAuxiliaryDecls([&](Decl *decl) {
5581-
// FIXME: Conformance macros can generate extension decls. These
5582-
// are visited as top-level decls; skip them here.
5583-
if (isa<ExtensionDecl>(decl))
5584-
return;
5585-
55865581
emitNestedTypeDecls({decl, nullptr});
55875582
});
55885583
switch (member->getKind()) {

lib/Index/Index.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1148,6 +1148,9 @@ void IndexSwiftASTWalker::visitModule(ModuleDecl &Mod) {
11481148
if (!handleSourceOrModuleFile(*SrcFile))
11491149
return;
11501150
walk(*SrcFile);
1151+
if (auto *synthesizedSF = SrcFile->getSynthesizedFile()) {
1152+
walk(synthesizedSF);
1153+
}
11511154
} else {
11521155
IsModuleFile = true;
11531156
isSystemModule = Mod.isNonUserModule();

lib/SILGen/SILGen.cpp

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2201,10 +2201,6 @@ class SILGenModuleRAII {
22012201
for (auto *D : sf->getTopLevelDecls()) {
22022202
// Emit auxiliary decls.
22032203
D->visitAuxiliaryDecls([&](Decl *auxiliaryDecl) {
2204-
// Skip extensions decls; they are visited below.
2205-
if (isa<ExtensionDecl>(auxiliaryDecl))
2206-
return;
2207-
22082204
FrontendStatsTracer StatsTracer(SGM.getASTContext().Stats,
22092205
"SILgen-decl", auxiliaryDecl);
22102206
SGM.visit(auxiliaryDecl);

lib/SILGen/SILGenType.cpp

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1101,11 +1101,6 @@ class SILGenType : public TypeMemberVisitor<SILGenType> {
11011101
SGM.emitLazyConformancesForType(theType);
11021102

11031103
forEachMemberToLower(theType, [&](Decl *member) {
1104-
// FIXME: Conformance macros can generate extension decls. These
1105-
// are visited as top-level decls; skip them here.
1106-
if (isa<ExtensionDecl>(member))
1107-
return;
1108-
11091104
visit(member);
11101105
});
11111106

lib/Sema/TypeCheckDeclPrimary.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1936,6 +1936,11 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
19361936
"`" + VD->getBaseName().userFacingName().str() + "`");
19371937
}
19381938
}
1939+
1940+
// Visit macro-generated extensions.
1941+
decl->forEachExpandedExtension([&](ExtensionDecl *extension) {
1942+
DeclVisitor<DeclChecker>::visit(extension);
1943+
});
19391944
}
19401945

19411946

lib/Sema/TypeCheckMacros.cpp

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1442,7 +1442,13 @@ swift::expandConformances(CustomAttr *attr, MacroDecl *macro,
14421442
extension->setExtendedNominal(nominal);
14431443
nominal->addExtension(extension);
14441444

1445-
// Make it accessible to getTopLevelDecls()
1445+
// Most other macro-generated declarations are visited through calling
1446+
// 'visitAuxiliaryDecls' on the original declaration the macro is attached
1447+
// to. We don't do this for macro-generated extensions, because the
1448+
// extension is not a peer of the original declaration. Instead of
1449+
// requiring all callers of 'visitAuxiliaryDecls' to understand the
1450+
// hoisting behavior of macro-generated extensions, we make the
1451+
// extension accessible through 'getTopLevelDecls()'.
14461452
if (auto file = dyn_cast<FileUnit>(
14471453
decl->getDeclContext()->getModuleScopeContext()))
14481454
file->getOrCreateSynthesizedFile().addTopLevelDecl(extension);

test/Index/index_macros.swift

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,12 @@ struct AddOne {
8181
// CHECK: [[@LINE-5]]:1 | function/Swift | freeLog() | [[FREE_LOG_USR]] | Ref,Call,Impl,RelCall,RelCont
8282
// CHECK-NEXT: RelCall,RelCont | instance-method/Swift | freeFunc() | [[FREE_FUNC_USR]]
8383

84+
func testExpr() {
85+
#freestandingExpr
86+
// CHECK: [[@LINE-1]]:3 | function/Swift | exprLog() | [[EXPR_LOG_USR]] | Ref,Call,Impl,RelCall,RelCont
87+
// CHECK-NEXT: RelCall,RelCont | function/Swift | testExpr()
88+
}
89+
8490
// CHECK: [[@LINE+4]]:40 | macro/Swift | Peer() | [[PEER_USR]] | Ref
8591
// CHECK: [[@LINE+3]]:23 | macro/Swift | MemberAttribute() | [[MEMBER_ATTRIBUTE_USR]] | Ref
8692
// CHECK: [[@LINE+2]]:15 | macro/Swift | Member() | [[MEMBER_USR]] | Ref
@@ -112,11 +118,6 @@ struct TestAttached {
112118
// CHECK: [[@LINE-24]]:39 | function/Swift | peerLog() | [[PEER_LOG_USR]] | Ref,Call,Impl,RelCall,RelCont
113119
// CHECK-NEXT: RelCall,RelCont | instance-method/Swift | peerFunc() | [[PEER_FUNC_USR]]
114120

115-
// `Conformance` adds `TestProto` as a conformance on an extension of `TestAttached`
116-
// CHECK: [[@LINE-28]]:1 | extension/ext-struct/Swift | TestAttached | {{.*}} | Def,Impl
117-
// CHECK: [[@LINE-29]]:1 | protocol/Swift | TestProto | [[PROTO_USR]] | Ref,Impl,RelBase
118-
// CHECK-NEXT: RelBase | extension/ext-struct/Swift | TestAttached
119-
120121
// CHECK: [[@LINE+1]]:8 | struct/Swift | Outer | [[OUTER_USR:.*]] | Def
121122
struct Outer {
122123
// CHECK: [[@LINE+1]]:4 | macro/Swift | PeerMember() | [[PEER_MEMBER_USR]] | Ref
@@ -137,16 +138,19 @@ struct Outer {
137138
// CHECK: [[@LINE-6]]:16 | function/Swift | memberLog() | [[MEMBER_LOG_USR]] | Ref,Call,Impl,RelCall,RelCont
138139
// CHECK-NEXT: RelCall,RelCont | instance-method/Swift | memberFunc() | [[INNER_FUNC_USR]]
139140

141+
142+
// Expanded extensions are visited last
143+
144+
// `Conformance` adds `TestProto` as a conformance on an extension of `TestAttached`
145+
// CHECK: [[@LINE-51]]:1 | extension/ext-struct/Swift | TestAttached | {{.*}} | Def,Impl
146+
// CHECK: [[@LINE-52]]:1 | protocol/Swift | TestProto | [[PROTO_USR]] | Ref,Impl,RelBase
147+
// CHECK-NEXT: RelBase | extension/ext-struct/Swift | TestAttached
148+
140149
// `Conformance` adds `TestProto` as a conformance on an extension of `TestInner`
141-
// CHECK: [[@LINE-10]]:3 | extension/ext-struct/Swift | TestInner | {{.*}} | Def,Impl
142-
// CHECK: [[@LINE-11]]:3 | protocol/Swift | TestProto | [[PROTO_USR]] | Ref,Impl,RelBase
150+
// CHECK: [[@LINE-18]]:3 | extension/ext-struct/Swift | TestInner | {{.*}} | Def,Impl
151+
// CHECK: [[@LINE-19]]:3 | protocol/Swift | TestProto | [[PROTO_USR]] | Ref,Impl,RelBase
143152
// CHECK-NEXT: RelBase | extension/ext-struct/Swift | TestInner
144153

145-
func testExpr() {
146-
#freestandingExpr
147-
// CHECK: [[@LINE-1]]:3 | function/Swift | exprLog() | [[EXPR_LOG_USR]] | Ref,Call,Impl,RelCall,RelCont
148-
// CHECK-NEXT: RelCall,RelCont | function/Swift | testExpr()
149-
}
150154

151155
//--- IndexMacros.swift
152156
import SwiftSyntax

test/Macros/macro_expand_conformances.swift

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,12 @@ requireHashable(S2())
6363

6464
requireEquatable(E.Nested())
6565

66+
extension E {
67+
@Equatable struct NestedInExtension {}
68+
}
69+
70+
requireEquatable(E.NestedInExtension())
71+
6672
#if TEST_DIAGNOSTICS
6773
requireEquatable(PublicEquatable())
6874

0 commit comments

Comments
 (0)