Skip to content

Commit db50f0c

Browse files
DougGregorDougGregor
committed
[Macros] Resolve macro names using unqualified lookup that ignores expansions
The macro name resolution in the source lookup cache was only looking at macros in the current module, meaning that any names introduced by peer or declaration macros declared in one module but used in another would not be found by name lookup. Switch the source lookup cache over to using the same `forEachPotentialResolvedMacro` API that is used by lookup within types, so we have consistent name-lookup-level macro resolution in both places. ... except that would be horribly cyclic, of course, so introduce name lookup flags to ignore top-level declarations introduced by macro expansions. This is semantically correct because macro expansions are not allowed to introduce new macros anyway, because that would have been a terrible idea. Fixes rdar://107321469. Peer and declaration macros at module scope should work a whole lot better now.
1 parent 77d89e5 commit db50f0c

18 files changed

+116
-38
lines changed

‎include/swift/AST/FileUnit.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ class FileUnit : public DeclContext, public ASTAllocated<FileUnit> {
6565
///
6666
/// This does a simple local lookup, not recursively looking through imports.
6767
virtual void lookupValue(DeclName name, NLKind lookupKind,
68+
OptionSet<ModuleLookupFlags> Flags,
6869
SmallVectorImpl<ValueDecl*> &result) const = 0;
6970

7071
/// Look up a local type declaration by its mangled name.
@@ -381,6 +382,7 @@ class BuiltinUnit final : public FileUnit {
381382
explicit BuiltinUnit(ModuleDecl &M);
382383

383384
virtual void lookupValue(DeclName name, NLKind lookupKind,
385+
OptionSet<ModuleLookupFlags> Flags,
384386
SmallVectorImpl<ValueDecl*> &result) const override;
385387

386388
/// Find all Objective-C methods with the given selector.

‎include/swift/AST/LookupKinds.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,9 @@ enum NLOptions : unsigned {
5353
// Include @usableFromInline and @inlinable
5454
NL_IncludeUsableFromInline = 1 << 6,
5555

56+
/// Exclude names introduced by macro expansions in the top-level module.
57+
NL_ExcludeMacroExpansions = 1 << 7,
58+
5659
/// The default set of options used for qualified name lookup.
5760
///
5861
/// FIXME: Eventually, add NL_ProtocolMembers to this, once all of the
@@ -81,6 +84,13 @@ static inline NLOptions operator~(NLOptions value) {
8184

8285
void simple_display(llvm::raw_ostream &out, NLOptions options);
8386

87+
88+
/// Flags affecting module-level lookup.
89+
enum class ModuleLookupFlags : unsigned {
90+
/// Exclude names introduced by macro expansions in the top-level module.
91+
ExcludeMacroExpansions = 1 << 0,
92+
};
93+
8494
} // end namespace swift
8595

8696
#endif

‎include/swift/AST/Module.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -733,8 +733,19 @@ class ModuleDecl
733733
///
734734
/// This does a simple local lookup, not recursively looking through imports.
735735
void lookupValue(DeclName Name, NLKind LookupKind,
736+
OptionSet<ModuleLookupFlags> Flags,
736737
SmallVectorImpl<ValueDecl*> &Result) const;
737738

739+
/// Look up a (possibly overloaded) value set at top-level scope
740+
/// (but with the specified access path, which may come from an import decl)
741+
/// within the current module.
742+
///
743+
/// This does a simple local lookup, not recursively looking through imports.
744+
void lookupValue(DeclName Name, NLKind LookupKind,
745+
SmallVectorImpl<ValueDecl*> &Result) const {
746+
lookupValue(Name, LookupKind, {}, Result);
747+
}
748+
738749
/// Look up a local type declaration by its mangled name.
739750
///
740751
/// This does a simple local lookup, not recursively looking through imports.

‎include/swift/AST/NameLookup.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,8 @@ enum class UnqualifiedLookupFlags {
239239
// This lookup should include results that are @inlinable or
240240
// @usableFromInline.
241241
IncludeUsableFromInline = 1 << 5,
242+
/// This lookup should exclude any names introduced by macro expansions.
243+
ExcludeMacroExpansions = 1 << 6,
242244
};
243245

244246
using UnqualifiedLookupOptions = OptionSet<UnqualifiedLookupFlags>;
@@ -542,6 +544,17 @@ template <typename Result>
542544
void filterForDiscriminator(SmallVectorImpl<Result> &results,
543545
DebuggerClient *debugClient);
544546

547+
/// Call the given function body with each macro declaration and its associated
548+
/// role attribute for the given role.
549+
///
550+
/// This routine intentionally avoids calling `forEachAttachedMacro`, which
551+
/// triggers request cycles, and should only be used when resolving macro
552+
/// names for the purposes of (other) name lookup.
553+
void forEachPotentialResolvedMacro(
554+
DeclContext *moduleScopeCtx, DeclNameRef macroName, MacroRole role,
555+
llvm::function_ref<void(MacroDecl *, const MacroRoleAttr *)> body
556+
);
557+
545558
} // end namespace namelookup
546559

547560
/// Describes an inherited nominal entry.

‎include/swift/AST/SourceFile.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -442,6 +442,7 @@ class SourceFile final : public FileUnit {
442442
const SmallVectorImpl<ValueDecl *> &getCachedVisibleDecls() const;
443443

444444
virtual void lookupValue(DeclName name, NLKind lookupKind,
445+
OptionSet<ModuleLookupFlags> Flags,
445446
SmallVectorImpl<ValueDecl*> &result) const override;
446447

447448
virtual void lookupVisibleDecls(ImportPath::Access accessPath,

‎include/swift/AST/SynthesizedFileUnit.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ class SynthesizedFileUnit final : public FileUnit {
4444
void addTopLevelDecl(Decl *D) { TopLevelDecls.push_back(D); }
4545

4646
virtual void lookupValue(DeclName name, NLKind lookupKind,
47+
OptionSet<ModuleLookupFlags> Flags,
4748
SmallVectorImpl<ValueDecl *> &result) const override;
4849

4950
void lookupObjCMethods(

‎include/swift/ClangImporter/ClangModule.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ class ClangModuleUnit final : public LoadedFile {
6666
virtual bool isSystemModule() const override;
6767

6868
virtual void lookupValue(DeclName name, NLKind lookupKind,
69+
OptionSet<ModuleLookupFlags> Flags,
6970
SmallVectorImpl<ValueDecl*> &results) const override;
7071

7172
virtual TypeDecl *

‎include/swift/Serialization/SerializedModuleLoader.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -360,6 +360,7 @@ class SerializedASTFile final : public LoadedFile {
360360
virtual bool isSystemModule() const override;
361361

362362
virtual void lookupValue(DeclName name, NLKind lookupKind,
363+
OptionSet<ModuleLookupFlags> Flags,
363364
SmallVectorImpl<ValueDecl*> &results) const override;
364365

365366
virtual StringRef

‎lib/AST/ASTContext.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1152,7 +1152,9 @@ ProtocolDecl *ASTContext::getProtocol(KnownProtocolKind kind) const {
11521152
if (!M)
11531153
return nullptr;
11541154
M->lookupValue(getIdentifier(getProtocolName(kind)),
1155-
NLKind::UnqualifiedLookup, results);
1155+
NLKind::UnqualifiedLookup,
1156+
ModuleLookupFlags::ExcludeMacroExpansions,
1157+
results);
11561158

11571159
for (auto result : results) {
11581160
if (auto protocol = dyn_cast<ProtocolDecl>(result)) {
@@ -6308,7 +6310,7 @@ Type ASTContext::getNamedSwiftType(ModuleDecl *module, StringRef name) {
63086310
if (auto module = clangUnit->getOverlayModule())
63096311
module->lookupValue(identifier, NLKind::UnqualifiedLookup, results);
63106312
} else {
6311-
file->lookupValue(identifier, NLKind::UnqualifiedLookup, results);
6313+
file->lookupValue(identifier, NLKind::UnqualifiedLookup, { }, results);
63126314
}
63136315
}
63146316

‎lib/AST/Module.cpp

Lines changed: 33 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -172,9 +172,6 @@ class swift::SourceLookupCache {
172172
template<typename Range>
173173
void addToMemberCache(Range decls);
174174

175-
using MacroDeclMap = llvm::DenseMap<Identifier, TinyPtrVector<MacroDecl *>>;
176-
MacroDeclMap MacroDecls;
177-
178175
using AuxiliaryDeclMap = llvm::DenseMap<DeclName, TinyPtrVector<MissingDecl *>>;
179176
AuxiliaryDeclMap TopLevelAuxiliaryDecls;
180177
SmallVector<Decl *, 4> MayHaveAuxiliaryDecls;
@@ -190,6 +187,7 @@ class swift::SourceLookupCache {
190187
void invalidate();
191188

192189
void lookupValue(DeclName Name, NLKind LookupKind,
190+
OptionSet<ModuleLookupFlags> Flags,
193191
SmallVectorImpl<ValueDecl*> &Result);
194192

195193
/// Retrieves all the operator decls. The order of the results is not
@@ -263,6 +261,10 @@ void SourceLookupCache::addToUnqualifiedLookupCache(Range decls,
263261
// a malformed context.
264262
if (ED->isInvalid()) continue;
265263

264+
if (ED->getAttrs().hasAttribute<CustomAttr>()) {
265+
MayHaveAuxiliaryDecls.push_back(ED);
266+
}
267+
266268
if (!ED->hasUnparsedMembers() || ED->maybeHasOperatorDeclarations())
267269
addToUnqualifiedLookupCache(ED->getMembers(), true);
268270
}
@@ -273,9 +275,6 @@ void SourceLookupCache::addToUnqualifiedLookupCache(Range decls,
273275
else if (auto *PG = dyn_cast<PrecedenceGroupDecl>(D))
274276
PrecedenceGroups[PG->getName()].push_back(PG);
275277

276-
else if (auto *macro = dyn_cast<MacroDecl>(D))
277-
MacroDecls[macro->getBaseIdentifier()].push_back(macro);
278-
279278
else if (auto *MED = dyn_cast<MacroExpansionDecl>(D))
280279
MayHaveAuxiliaryDecls.push_back(MED);
281280
}
@@ -353,32 +352,26 @@ void SourceLookupCache::populateAuxiliaryDeclCache() {
353352
for (auto attrConst : decl->getAttrs().getAttributes<CustomAttr>()) {
354353
auto *attr = const_cast<CustomAttr *>(attrConst);
355354
UnresolvedMacroReference macroRef(attr);
356-
auto macroName = macroRef.getMacroName().getBaseIdentifier();
357-
358-
auto found = MacroDecls.find(macroName);
359-
if (found == MacroDecls.end())
360-
continue;
361-
362-
for (const auto *macro : found->second) {
363-
macro->getIntroducedNames(MacroRole::Peer,
364-
dyn_cast<ValueDecl>(decl),
365-
introducedNames[attr]);
366-
}
355+
namelookup::forEachPotentialResolvedMacro(
356+
decl->getDeclContext()->getModuleScopeContext(),
357+
macroRef.getMacroName(), MacroRole::Peer,
358+
[&](MacroDecl *macro, const MacroRoleAttr *roleAttr) {
359+
macro->getIntroducedNames(MacroRole::Peer,
360+
dyn_cast<ValueDecl>(decl),
361+
introducedNames[attr]);
362+
});
367363
}
368364

369365
if (auto *med = dyn_cast<MacroExpansionDecl>(decl)) {
370366
UnresolvedMacroReference macroRef(med);
371-
auto macroName = macroRef.getMacroName().getBaseIdentifier();
372-
373-
auto found = MacroDecls.find(macroName);
374-
if (found == MacroDecls.end())
375-
continue;
376-
377-
for (const auto *macro : found->second) {
378-
macro->getIntroducedNames(MacroRole::Declaration,
379-
/*attachedTo*/ nullptr,
380-
introducedNames[med]);
381-
}
367+
namelookup::forEachPotentialResolvedMacro(
368+
decl->getDeclContext()->getModuleScopeContext(),
369+
macroRef.getMacroName(), MacroRole::Declaration,
370+
[&](MacroDecl *macro, const MacroRoleAttr *roleAttr) {
371+
macro->getIntroducedNames(MacroRole::Declaration,
372+
/*attachedTo*/ nullptr,
373+
introducedNames[med]);
374+
});
382375
}
383376

384377
// Add macro-introduced names to the top-level auxiliary decl cache as
@@ -425,6 +418,7 @@ SourceLookupCache::SourceLookupCache(const ModuleDecl &M)
425418
}
426419

427420
void SourceLookupCache::lookupValue(DeclName Name, NLKind LookupKind,
421+
OptionSet<ModuleLookupFlags> Flags,
428422
SmallVectorImpl<ValueDecl*> &Result) {
429423
auto I = TopLevelValues.find(Name);
430424
if (I != TopLevelValues.end()) {
@@ -433,6 +427,10 @@ void SourceLookupCache::lookupValue(DeclName Name, NLKind LookupKind,
433427
Result.push_back(Elt);
434428
}
435429

430+
// If we aren't supposed to find names introduced by macros, we're done.
431+
if (Flags.contains(ModuleLookupFlags::ExcludeMacroExpansions))
432+
return;
433+
436434
// Add top-level auxiliary decls to the result.
437435
//
438436
// FIXME: We need to not consider auxiliary decls if we're doing lookup
@@ -906,17 +904,18 @@ static bool isParsedModule(const ModuleDecl *mod) {
906904
}
907905

908906
void ModuleDecl::lookupValue(DeclName Name, NLKind LookupKind,
907+
OptionSet<ModuleLookupFlags> Flags,
909908
SmallVectorImpl<ValueDecl*> &Result) const {
910909
auto *stats = getASTContext().Stats;
911910
if (stats)
912911
++stats->getFrontendCounters().NumModuleLookupValue;
913912

914913
if (isParsedModule(this)) {
915-
getSourceLookupCache().lookupValue(Name, LookupKind, Result);
914+
getSourceLookupCache().lookupValue(Name, LookupKind, Flags, Result);
916915
return;
917916
}
918917

919-
FORWARD(lookupValue, (Name, LookupKind, Result));
918+
FORWARD(lookupValue, (Name, LookupKind, Flags, Result));
920919
}
921920

922921
TypeDecl * ModuleDecl::lookupLocalType(StringRef MangledName) const {
@@ -1010,6 +1009,7 @@ void ModuleDecl::lookupImportedSPIGroups(
10101009
}
10111010

10121011
void BuiltinUnit::lookupValue(DeclName name, NLKind lookupKind,
1012+
OptionSet<ModuleLookupFlags> Flags,
10131013
SmallVectorImpl<ValueDecl*> &result) const {
10141014
getCache().lookupValue(name.getBaseIdentifier(), lookupKind, *this, result);
10151015
}
@@ -1021,8 +1021,9 @@ void BuiltinUnit::lookupObjCMethods(
10211021
}
10221022

10231023
void SourceFile::lookupValue(DeclName name, NLKind lookupKind,
1024+
OptionSet<ModuleLookupFlags> flags,
10241025
SmallVectorImpl<ValueDecl*> &result) const {
1025-
getCache().lookupValue(name, lookupKind, result);
1026+
getCache().lookupValue(name, lookupKind, flags, result);
10261027
}
10271028

10281029
void ModuleDecl::lookupVisibleDecls(ImportPath::Access AccessPath,
@@ -3980,6 +3981,7 @@ SynthesizedFileUnit::getDiscriminatorForPrivateDecl(const Decl *D) const {
39803981

39813982
void SynthesizedFileUnit::lookupValue(
39823983
DeclName name, NLKind lookupKind,
3984+
OptionSet<ModuleLookupFlags> Flags,
39833985
SmallVectorImpl<ValueDecl *> &result) const {
39843986
for (auto *decl : TopLevelDecls) {
39853987
if (auto VD = dyn_cast<ValueDecl>(decl)) {

‎lib/AST/ModuleNameLookup.cpp

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,10 @@ class ModuleNameLookup {
5757
NLOptions options);
5858
};
5959

60+
// Exclude names introduced by macro expansions.
61+
enum class LocalLookupFlags {
62+
ExcludeMacroExpansions,
63+
};
6064

6165
/// Encapsulates the work done for a recursive qualified lookup into a module
6266
/// by full name.
@@ -81,12 +85,13 @@ class LookupByName : public ModuleNameLookup<LookupByName> {
8185
}
8286

8387
void doLocalLookup(ModuleDecl *module, ImportPath::Access path,
88+
OptionSet<ModuleLookupFlags> flags,
8489
SmallVectorImpl<ValueDecl *> &localDecls) {
8590
// If this import is specific to some named decl ("import Swift.Int")
8691
// then filter out any lookups that don't match.
8792
if (!path.matches(name))
8893
return;
89-
module->lookupValue(name, lookupKind, localDecls);
94+
module->lookupValue(name, lookupKind, flags, localDecls);
9095
}
9196
};
9297

@@ -112,6 +117,7 @@ class LookupVisibleDecls : public ModuleNameLookup<LookupVisibleDecls> {
112117
}
113118

114119
void doLocalLookup(ModuleDecl *module, ImportPath::Access path,
120+
OptionSet<ModuleLookupFlags> flags,
115121
SmallVectorImpl<ValueDecl *> &localDecls) {
116122
VectorDeclConsumer consumer(localDecls);
117123
module->lookupVisibleDecls(path, consumer, lookupKind);
@@ -167,9 +173,14 @@ void ModuleNameLookup<LookupStrategy>::lookupInModule(
167173
currentCount = decls.size();
168174
};
169175

176+
OptionSet<ModuleLookupFlags> currentModuleLookupFlags = {};
177+
if (options & NL_ExcludeMacroExpansions)
178+
currentModuleLookupFlags |= ModuleLookupFlags::ExcludeMacroExpansions;
179+
170180
// Do the lookup into the current module.
171181
auto *module = moduleOrFile->getParentModule();
172-
getDerived()->doLocalLookup(module, accessPath, decls);
182+
getDerived()->doLocalLookup(
183+
module, accessPath, currentModuleLookupFlags, decls);
173184
updateNewDecls(moduleScopeContext);
174185

175186
bool canReturnEarly = (initialCount != decls.size() &&
@@ -198,7 +209,7 @@ void ModuleNameLookup<LookupStrategy>::lookupInModule(
198209
return;
199210

200211
getDerived()->doLocalLookup(import.importedModule, import.accessPath,
201-
decls);
212+
{ }, decls);
202213
updateNewDecls(moduleScopeContext);
203214
};
204215

‎lib/AST/NameLookup.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1539,12 +1539,16 @@ static DeclName adjustLazyMacroExpansionNameKey(
15391539
///
15401540
/// This routine intentionally avoids calling `forEachAttachedMacro`, which
15411541
/// triggers request cycles.
1542-
static void forEachPotentialResolvedMacro(
1542+
void namelookup::forEachPotentialResolvedMacro(
15431543
DeclContext *moduleScopeCtx, DeclNameRef macroName, MacroRole role,
15441544
llvm::function_ref<void(MacroDecl *, const MacroRoleAttr *)> body
15451545
) {
15461546
ASTContext &ctx = moduleScopeCtx->getASTContext();
1547-
UnqualifiedLookupDescriptor lookupDesc{macroName, moduleScopeCtx};
1547+
UnqualifiedLookupDescriptor lookupDesc{
1548+
macroName, moduleScopeCtx, SourceLoc(),
1549+
UnqualifiedLookupFlags::ExcludeMacroExpansions
1550+
};
1551+
15481552
auto lookup = evaluateOrDefault(
15491553
ctx.evaluator, UnqualifiedLookupRequest{lookupDesc}, {});
15501554
for (auto result : lookup.allResults()) {

‎lib/AST/UnqualifiedLookup.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -520,6 +520,8 @@ void UnqualifiedLookupFactory::addImportedResults(const DeclContext *const dc) {
520520
auto nlOptions = NL_UnqualifiedDefault;
521521
if (options.contains(Flags::IncludeUsableFromInline))
522522
nlOptions |= NL_IncludeUsableFromInline;
523+
if (options.contains(Flags::ExcludeMacroExpansions))
524+
nlOptions |= NL_ExcludeMacroExpansions;
523525
lookupInModule(dc, Name.getFullName(), CurModuleResults,
524526
NLKind::UnqualifiedLookup, resolutionKind, dc, nlOptions);
525527

‎lib/ClangImporter/ClangImporter.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3344,6 +3344,7 @@ void ClangModuleUnit::getDisplayDecls(SmallVectorImpl<Decl*> &results, bool recu
33443344
}
33453345

33463346
void ClangModuleUnit::lookupValue(DeclName name, NLKind lookupKind,
3347+
OptionSet<ModuleLookupFlags> flags,
33473348
SmallVectorImpl<ValueDecl*> &results) const {
33483349
// FIXME: Ignore submodules, which are empty for now.
33493350
if (clangModule && clangModule->isSubModule())

‎lib/ClangImporter/DWARFImporter.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ class DWARFModuleUnit final : public LoadedFile {
3333
/// DWARFimporterDelegate.
3434
virtual void
3535
lookupValue(DeclName name, NLKind lookupKind,
36+
OptionSet<ModuleLookupFlags> Flags,
3637
SmallVectorImpl<ValueDecl *> &results) const override {
3738
Owner.lookupValueDWARF(name, lookupKind,
3839
getParentModule()->getName(), results);

0 commit comments

Comments
 (0)