diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h index e35dbb48e6928..2a304e351649d 100644 --- a/include/swift/AST/Decl.h +++ b/include/swift/AST/Decl.h @@ -3835,6 +3835,8 @@ class NominalTypeDecl : public GenericTypeDecl, public IterableDeclContext { /// Whether to include @_implements members. /// Used by conformance-checking to find special @_implements members. IncludeAttrImplements = 1 << 0, + /// Whether to exclude members of macro expansions. + ExcludeMacroExpansions = 1 << 1, }; /// Find all of the declarations with the given name within this nominal type diff --git a/include/swift/AST/Evaluator.h b/include/swift/AST/Evaluator.h index 2ce84d54c746c..70b89958befe5 100644 --- a/include/swift/AST/Evaluator.h +++ b/include/swift/AST/Evaluator.h @@ -208,6 +208,18 @@ class Evaluator { /// is treated as a stack and is used to detect cycles. llvm::SetVector activeRequests; + /// How many `ResolveMacroRequest` requests are active. + /// + /// This allows us to quickly determine whether there is any + /// `ResolveMacroRequest` active in the active request stack. + /// It saves us from a linear scan through `activeRequests` when + /// we need to determine this information. + /// + /// Why on earth would we need to determine this information? + /// Please see the extended comment that goes with the constructor + /// of `UnqualifiedLookupRequest`. + unsigned numActiveResolveMacroRequests = 0; + /// A cache that stores the results of requests. evaluator::RequestCache cache; @@ -324,6 +336,16 @@ class Evaluator { return activeRequests.count(ActiveRequest(request)); } + /// Determine whether there is any active "resolve macro" request + /// on the request stack. + /// + /// Why on earth would we need to determine this information? + /// Please see the extended comment that goes with the constructor + /// of `UnqualifiedLookupRequest`. + bool hasActiveResolveMacroRequest() const { + return numActiveResolveMacroRequests > 0; + } + private: /// Diagnose a cycle detected in the evaluation of the given /// request. @@ -337,6 +359,10 @@ class Evaluator { /// request to the \c activeRequests stack. bool checkDependency(const ActiveRequest &request); + /// Note that we have finished this request, popping it from the + /// \c activeRequests stack. + void finishedRequest(const ActiveRequest &request); + /// Produce the result of the request without caching. template llvm::Expected @@ -366,8 +392,7 @@ class Evaluator { // Make sure we remove this from the set of active requests once we're // done. - assert(activeRequests.back() == activeReq); - activeRequests.pop_back(); + finishedRequest(activeReq); return std::move(result); } diff --git a/include/swift/AST/NameLookupRequests.h b/include/swift/AST/NameLookupRequests.h index b87567f00ab5a..21b93da06ec48 100644 --- a/include/swift/AST/NameLookupRequests.h +++ b/include/swift/AST/NameLookupRequests.h @@ -430,7 +430,7 @@ class UnqualifiedLookupRequest RequestFlags::Uncached | RequestFlags::DependencySink> { public: - using SimpleRequest::SimpleRequest; + UnqualifiedLookupRequest(UnqualifiedLookupDescriptor); private: friend SimpleRequest; @@ -456,7 +456,10 @@ class LookupInModuleRequest NLOptions), RequestFlags::Uncached | RequestFlags::DependencySink> { public: - using SimpleRequest::SimpleRequest; + LookupInModuleRequest( + const DeclContext *, DeclName, NLKind, + namelookup::ResolutionKind, const DeclContext *, + NLOptions); private: friend SimpleRequest; @@ -504,7 +507,9 @@ class ModuleQualifiedLookupRequest RequestFlags::Uncached | RequestFlags::DependencySink> { public: - using SimpleRequest::SimpleRequest; + ModuleQualifiedLookupRequest(const DeclContext *, + ModuleDecl *, DeclNameRef, + NLOptions); private: friend SimpleRequest; @@ -528,7 +533,9 @@ class QualifiedLookupRequest DeclNameRef, NLOptions), RequestFlags::Uncached> { public: - using SimpleRequest::SimpleRequest; + QualifiedLookupRequest(const DeclContext *, + SmallVector, + DeclNameRef, NLOptions); private: friend SimpleRequest; @@ -579,7 +586,7 @@ class DirectLookupRequest TinyPtrVector(DirectLookupDescriptor), RequestFlags::Uncached|RequestFlags::DependencySink> { public: - using SimpleRequest::SimpleRequest; + DirectLookupRequest(DirectLookupDescriptor); private: friend SimpleRequest; diff --git a/lib/AST/Evaluator.cpp b/lib/AST/Evaluator.cpp index 774cc60c69b79..49c1fab72b53c 100644 --- a/lib/AST/Evaluator.cpp +++ b/lib/AST/Evaluator.cpp @@ -17,6 +17,7 @@ #include "swift/AST/Evaluator.h" #include "swift/AST/DeclContext.h" #include "swift/AST/DiagnosticEngine.h" +#include "swift/AST/TypeCheckRequests.h" // for ResolveMacroRequest #include "swift/Basic/LangOptions.h" #include "swift/Basic/Range.h" #include "swift/Basic/SourceManager.h" @@ -61,14 +62,25 @@ Evaluator::Evaluator(DiagnosticEngine &diags, const LangOptions &opts) bool Evaluator::checkDependency(const ActiveRequest &request) { // Record this as an active request. - if (activeRequests.insert(request)) + if (activeRequests.insert(request)) { + if (request.getAs()) + ++numActiveResolveMacroRequests; return false; + } // Diagnose cycle. diagnoseCycle(request); return true; } +void Evaluator::finishedRequest(const ActiveRequest &request) { + if (request.getAs()) + --numActiveResolveMacroRequests; + + assert(activeRequests.back() == request); + activeRequests.pop_back(); +} + void Evaluator::diagnoseCycle(const ActiveRequest &request) { if (debugDumpCycles) { const auto printIndent = [](llvm::raw_ostream &OS, unsigned indent) { diff --git a/lib/AST/Module.cpp b/lib/AST/Module.cpp index 36f65f2c7ec42..e3c5b3618724d 100644 --- a/lib/AST/Module.cpp +++ b/lib/AST/Module.cpp @@ -174,6 +174,10 @@ class swift::SourceLookupCache { using AuxiliaryDeclMap = llvm::DenseMap>; AuxiliaryDeclMap TopLevelAuxiliaryDecls; + + /// Top-level macros that produce arbitrary names. + SmallVector TopLevelArbitraryMacros; + SmallVector MayHaveAuxiliaryDecls; void populateAuxiliaryDeclCache(); @@ -352,26 +356,46 @@ void SourceLookupCache::populateAuxiliaryDeclCache() { for (auto attrConst : decl->getAttrs().getAttributes()) { auto *attr = const_cast(attrConst); UnresolvedMacroReference macroRef(attr); + bool introducesArbitraryNames = false; namelookup::forEachPotentialResolvedMacro( decl->getDeclContext()->getModuleScopeContext(), macroRef.getMacroName(), MacroRole::Peer, [&](MacroDecl *macro, const MacroRoleAttr *roleAttr) { + // First check for arbitrary names. + if (roleAttr->hasNameKind(MacroIntroducedDeclNameKind::Arbitrary)) { + introducesArbitraryNames = true; + } + macro->getIntroducedNames(MacroRole::Peer, dyn_cast(decl), introducedNames[attr]); }); + + // Record this macro where appropriate. + if (introducesArbitraryNames) + TopLevelArbitraryMacros.push_back(MissingDecl::forUnexpandedMacro(attr, decl)); } if (auto *med = dyn_cast(decl)) { UnresolvedMacroReference macroRef(med); + bool introducesArbitraryNames = false; namelookup::forEachPotentialResolvedMacro( decl->getDeclContext()->getModuleScopeContext(), macroRef.getMacroName(), MacroRole::Declaration, [&](MacroDecl *macro, const MacroRoleAttr *roleAttr) { + // First check for arbitrary names. + if (roleAttr->hasNameKind(MacroIntroducedDeclNameKind::Arbitrary)) { + introducesArbitraryNames = true; + } + macro->getIntroducedNames(MacroRole::Declaration, /*attachedTo*/ nullptr, introducedNames[med]); }); + + // Record this macro where appropriate. + if (introducesArbitraryNames) + TopLevelArbitraryMacros.push_back(MissingDecl::forUnexpandedMacro(med, decl)); } // Add macro-introduced names to the top-level auxiliary decl cache as @@ -440,15 +464,30 @@ void SourceLookupCache::lookupValue(DeclName Name, NLKind LookupKind, ? UniqueMacroNamePlaceholder : Name; auto auxDecls = TopLevelAuxiliaryDecls.find(keyName); - if (auxDecls == TopLevelAuxiliaryDecls.end()) + + // Check macro expansions that could produce this name. + SmallVector unexpandedDecls; + if (auxDecls != TopLevelAuxiliaryDecls.end()) { + unexpandedDecls.insert( + unexpandedDecls.end(), auxDecls->second.begin(), auxDecls->second.end()); + } + + // Check macro expansions that can produce arbitrary names. + unexpandedDecls.insert( + unexpandedDecls.end(), + TopLevelArbitraryMacros.begin(), TopLevelArbitraryMacros.end()); + + if (unexpandedDecls.empty()) return; - for (auto *unexpandedDecl : auxDecls->second) { - // Add expanded peers and freestanding declarations to the result. + // Add matching expanded peers and freestanding declarations to the results. + SmallPtrSet macroExpandedDecls; + for (auto *unexpandedDecl : unexpandedDecls) { unexpandedDecl->forEachMacroExpandedDecl( [&](ValueDecl *decl) { if (decl->getName().matchesRef(Name)) { - Result.push_back(decl); + if (macroExpandedDecls.insert(decl).second) + Result.push_back(decl); } }); } diff --git a/lib/AST/NameLookup.cpp b/lib/AST/NameLookup.cpp index a5785e27719fc..14d86cdfd2db0 100644 --- a/lib/AST/NameLookup.cpp +++ b/lib/AST/NameLookup.cpp @@ -1713,13 +1713,24 @@ void NominalTypeDecl::prepareLookupTable() { } static TinyPtrVector -maybeFilterOutAttrImplements(TinyPtrVector decls, - DeclName name, - bool includeAttrImplements) { - if (includeAttrImplements) +maybeFilterOutUnwantedDecls(TinyPtrVector decls, + DeclName name, + bool includeAttrImplements, + bool excludeMacroExpansions) { + if (includeAttrImplements && !excludeMacroExpansions) return decls; TinyPtrVector result; for (auto V : decls) { + // If we're supposed to exclude anything that comes from a macro expansion, + // check whether the source location of the declaration is in a macro + // expansion, and skip this declaration if it does. + if (excludeMacroExpansions) { + auto sourceFile = + V->getModuleContext()->getSourceFileContainingLocation(V->getLoc()); + if (sourceFile && sourceFile->Kind == SourceFileKind::MacroExpansion) + continue; + } + // Filter-out any decl that doesn't have the name we're looking for // (asserting as a consistency-check that such entries all have // @_implements attrs for the name!) @@ -1755,12 +1766,16 @@ DirectLookupRequest::evaluate(Evaluator &evaluator, decl->hasLazyMembers()); const bool includeAttrImplements = flags.contains(NominalTypeDecl::LookupDirectFlags::IncludeAttrImplements); + const bool excludeMacroExpansions = + flags.contains(NominalTypeDecl::LookupDirectFlags::ExcludeMacroExpansions); LLVM_DEBUG(llvm::dbgs() << decl->getNameStr() << ".lookupDirect(" << name << ")" << ", hasLazyMembers()=" << decl->hasLazyMembers() << ", useNamedLazyMemberLoading=" << useNamedLazyMemberLoading + << ", excludeMacroExpansions=" + << excludeMacroExpansions << "\n"); decl->prepareLookupTable(); @@ -1797,8 +1812,9 @@ DirectLookupRequest::evaluate(Evaluator &evaluator, if (!allFound.empty()) { auto known = Table.find(name); if (known != Table.end()) { - auto swiftLookupResult = maybeFilterOutAttrImplements( - known->second, name, includeAttrImplements); + auto swiftLookupResult = maybeFilterOutUnwantedDecls( + known->second, name, includeAttrImplements, + excludeMacroExpansions); for (auto foundSwiftDecl : swiftLookupResult) { allFound.push_back(foundSwiftDecl); } @@ -1828,7 +1844,8 @@ DirectLookupRequest::evaluate(Evaluator &evaluator, } DeclName macroExpansionKey = adjustLazyMacroExpansionNameKey(ctx, name); - if (!Table.isLazilyCompleteForMacroExpansion(macroExpansionKey)) { + if (!excludeMacroExpansions && + !Table.isLazilyCompleteForMacroExpansion(macroExpansionKey)) { populateLookupTableEntryFromMacroExpansions( ctx, Table, macroExpansionKey, decl); for (auto ext : decl->getExtensions()) { @@ -1845,8 +1862,9 @@ DirectLookupRequest::evaluate(Evaluator &evaluator, } // We found something; return it. - return maybeFilterOutAttrImplements(known->second, name, - includeAttrImplements); + return maybeFilterOutUnwantedDecls(known->second, name, + includeAttrImplements, + excludeMacroExpansions); } bool NominalTypeDecl::createObjCMethodLookup() { @@ -2201,6 +2219,8 @@ QualifiedLookupRequest::evaluate(Evaluator &eval, const DeclContext *DC, auto flags = OptionSet(); if (options & NL_IncludeAttributeImplements) flags |= NominalTypeDecl::LookupDirectFlags::IncludeAttrImplements; + if (options & NL_ExcludeMacroExpansions) + flags |= NominalTypeDecl::LookupDirectFlags::ExcludeMacroExpansions; for (auto decl : current->lookupDirect(member.getFullName(), flags)) { // If we're performing a type lookup, don't even attempt to validate // the decl if its not a type. diff --git a/lib/AST/NameLookupRequests.cpp b/lib/AST/NameLookupRequests.cpp index 902665eacc09d..07d90315795e2 100644 --- a/lib/AST/NameLookupRequests.cpp +++ b/lib/AST/NameLookupRequests.cpp @@ -508,6 +508,108 @@ swift::extractNearestSourceLoc(CustomRefCountingOperationDescriptor desc) { return SourceLoc(); } +//----------------------------------------------------------------------------// +// Macro-related adjustments to name lookup requests. +//----------------------------------------------------------------------------// +// +// Macros introduced a significant wrinkle into Swift's name lookup mechanism. +// Specifically, when resolving names (and, really, anything else) within the +// arguments to a macro expansion, name lookup must not try to expand any +// macros, because doing so trivially creates a cyclic dependency amongst the +// macro expansions that will be detected by the request-evaluator. +// +// Our lookup requests don't always have enough information to answer the +// question "is this part of an argument to a macro?", so we do a much simpler, +// more efficient, and not-entirely-sound hack based on the request-evaluator. +// Specifically, if we are in the process of resolving a macro (which is +// determined by checking for the presence of a `ResolveMacroRequest` in the +// request-evaluator stack), then we adjust the options used for the name +// lookup request we are forming to exclude macro expansions. The evaluation +// of that request will then avoid expanding any macros, and not produce any +// results that involve entries in already-expanded macros. By adjusting the +// request itself, we still distinguish between requests that can and cannot +// look into macro expansions, so it doesn't break caching for those immediate +// requests. +// +// Over time, we should seek to replace this heuristic with a location-based +// check, where we use ASTScope to determine whether we are inside a macro +// argument. This existing check might still be useful because it's going to +// be faster than a location-based query, but the location-based query can be +// fully correct. + +/// Exclude macros in the unqualified lookup descriptor if we need to. +static UnqualifiedLookupDescriptor excludeMacrosIfNeeded( + UnqualifiedLookupDescriptor descriptor) { + if (descriptor.Options.contains( + UnqualifiedLookupFlags::ExcludeMacroExpansions)) + return descriptor; + + auto &evaluator = descriptor.DC->getASTContext().evaluator; + if (!evaluator.hasActiveResolveMacroRequest()) + return descriptor; + + descriptor.Options |= UnqualifiedLookupFlags::ExcludeMacroExpansions; + return descriptor; +} + +/// Exclude macros in the direct lookup descriptor if we need to. +static DirectLookupDescriptor excludeMacrosIfNeeded( + DirectLookupDescriptor descriptor) { + if (descriptor.Options.contains( + NominalTypeDecl::LookupDirectFlags::ExcludeMacroExpansions)) + return descriptor; + + auto &evaluator = descriptor.DC->getASTContext().evaluator; + if (!evaluator.hasActiveResolveMacroRequest()) + return descriptor; + + descriptor.Options |= + NominalTypeDecl::LookupDirectFlags::ExcludeMacroExpansions; + + return descriptor; +} + +/// Exclude macros in the name lookup options if we need to. +static NLOptions +excludeMacrosIfNeeded(const DeclContext *dc, NLOptions options) { + if (options & NL_ExcludeMacroExpansions) + return options; + + auto &evaluator = dc->getASTContext().evaluator; + if (!evaluator.hasActiveResolveMacroRequest()) + return options; + + return options | NL_ExcludeMacroExpansions; +} + +UnqualifiedLookupRequest::UnqualifiedLookupRequest( + UnqualifiedLookupDescriptor descriptor +) : SimpleRequest(excludeMacrosIfNeeded(descriptor)) { } + +LookupInModuleRequest::LookupInModuleRequest( + const DeclContext *moduleOrFile, DeclName name, NLKind lookupKind, + namelookup::ResolutionKind resolutionKind, + const DeclContext *moduleScopeContext, + NLOptions options + ) : SimpleRequest(moduleOrFile, name, lookupKind, resolutionKind, + moduleScopeContext, + excludeMacrosIfNeeded(moduleOrFile, options)) { } + +ModuleQualifiedLookupRequest::ModuleQualifiedLookupRequest( + const DeclContext *dc, ModuleDecl *module, DeclNameRef name, + NLOptions options + ) : SimpleRequest(dc, module, name, excludeMacrosIfNeeded(dc, options)) { } + +QualifiedLookupRequest::QualifiedLookupRequest( + const DeclContext *dc, + SmallVector decls, + DeclNameRef name, NLOptions options +) : SimpleRequest(dc, std::move(decls), name, + excludeMacrosIfNeeded(dc, options)) { } + +DirectLookupRequest::DirectLookupRequest(DirectLookupDescriptor descriptor) + : SimpleRequest(excludeMacrosIfNeeded(descriptor)) { } + // Implement the clang importer type zone. #define SWIFT_TYPEID_ZONE ClangImporter #define SWIFT_TYPEID_HEADER "swift/ClangImporter/ClangImporterTypeIDZone.def" diff --git a/lib/Sema/TypeCheckExpr.cpp b/lib/Sema/TypeCheckExpr.cpp index b7f36b810923f..d4101051f8038 100644 --- a/lib/Sema/TypeCheckExpr.cpp +++ b/lib/Sema/TypeCheckExpr.cpp @@ -615,9 +615,11 @@ static Type lookupDefaultLiteralType(const DeclContext *dc, StringRef name) { auto &ctx = dc->getASTContext(); DeclNameRef nameRef(ctx.getIdentifier(name)); - auto lookup = TypeChecker::lookupUnqualified(dc->getModuleScopeContext(), - nameRef, SourceLoc(), - defaultUnqualifiedLookupOptions); + auto lookup = TypeChecker::lookupUnqualified( + dc->getModuleScopeContext(), + nameRef, SourceLoc(), + defaultUnqualifiedLookupOptions | NameLookupFlags::ExcludeMacroExpansions + ); TypeDecl *TD = lookup.getSingleTypeResult(); if (!TD) return Type(); diff --git a/lib/Sema/TypeCheckNameLookup.cpp b/lib/Sema/TypeCheckNameLookup.cpp index 77a0248024e3b..51b8f416467b3 100644 --- a/lib/Sema/TypeCheckNameLookup.cpp +++ b/lib/Sema/TypeCheckNameLookup.cpp @@ -217,6 +217,8 @@ convertToUnqualifiedLookupOptions(NameLookupOptions options) { newOptions |= UnqualifiedLookupFlags::IncludeOuterResults; if (options.contains(NameLookupFlags::IncludeUsableFromInline)) newOptions |= UnqualifiedLookupFlags::IncludeUsableFromInline; + if (options.contains(NameLookupFlags::ExcludeMacroExpansions)) + newOptions |= UnqualifiedLookupFlags::ExcludeMacroExpansions; return newOptions; } diff --git a/lib/Sema/TypeChecker.h b/lib/Sema/TypeChecker.h index 17a117665be24..9c78a8b80020f 100644 --- a/lib/Sema/TypeChecker.h +++ b/lib/Sema/TypeChecker.h @@ -160,6 +160,8 @@ enum class NameLookupFlags { IncludeOuterResults = 1 << 1, // Whether to include results that are marked @inlinable or @usableFromInline. IncludeUsableFromInline = 1 << 2, + /// This lookup should exclude any names introduced by macro expansions. + ExcludeMacroExpansions = 1 << 3, }; /// A set of options that control name lookup. diff --git a/test/Macros/macro_expand.swift b/test/Macros/macro_expand.swift index fd4b998fc6e74..238cd7ec24573 100644 --- a/test/Macros/macro_expand.swift +++ b/test/Macros/macro_expand.swift @@ -282,6 +282,10 @@ _ = StructWithUnqualifiedLookup().foo() func testFreestandingMacroExpansion() { // Explicit structs to force macros to be parsed as decl. struct Foo { + static let singleton = Foo() + + static let s2 = Foo.singleton + #bitwidthNumberedStructs("MyIntOne") } @@ -341,9 +345,10 @@ func testFreestandingMacroExpansion() { testFreestandingMacroExpansion() // Explicit structs to force macros to be parsed as decl. +var globalBool = true struct ContainerOfNumberedStructs { #bitwidthNumberedStructs("MyIntOne") - #bitwidthNumberedStructs("MyIntTwo") + #bitwidthNumberedStructs("MyIntTwo", blah: globalBool) } // Avoid re-type-checking declaration macro arguments. diff --git a/test/Macros/top_level_freestanding.swift b/test/Macros/top_level_freestanding.swift index 6e1add0863a14..22dac94ec926e 100644 --- a/test/Macros/top_level_freestanding.swift +++ b/test/Macros/top_level_freestanding.swift @@ -54,3 +54,13 @@ struct HasInnerClosure { #freestandingWithClosure(0) { x in x } #freestandingWithClosure(1) { x in x } } + +// Arbitrary names at global scope + +@freestanding(declaration, names: arbitrary) macro bitwidthNumberedStructs(_ baseName: String) = #externalMacro(module: "MacroDefinition", type: "DefineBitwidthNumberedStructsMacro") + +#bitwidthNumberedStructs("MyIntGlobal") + +func testArbitraryAtGlobal() { + _ = MyIntGlobal16() +}