From 12d89a97ea201a888f77d96542dfe277cc5619ff Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Wed, 5 Apr 2023 21:31:17 -0700 Subject: [PATCH 1/8] [Module-scope lookup] Only add macro-expanded names that match. When we look into expanded macros to find declarations for module-scope name lookup, make sure that we only return declarations whose name matches the requested name. We were effectively spamming the results with unrelated names, and most clients trust the results of this lookup (vs. filtering them further), so they would get confused. The particular failure for this test is that type reconstruction would fail because we were looking for one unique name, but we found many non-matching names, and type reconstruction bails out rather than try to filter further. (cherry picked from commit 8e63bf1bc47f83a1113bf51341986ff370af79a8) --- lib/AST/Module.cpp | 6 ++++-- test/Macros/macro_expand_peers.swift | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/AST/Module.cpp b/lib/AST/Module.cpp index d066745f753ea..36f65f2c7ec42 100644 --- a/lib/AST/Module.cpp +++ b/lib/AST/Module.cpp @@ -444,10 +444,12 @@ void SourceLookupCache::lookupValue(DeclName Name, NLKind LookupKind, return; for (auto *unexpandedDecl : auxDecls->second) { - // Add expanded peers to the result. + // Add expanded peers and freestanding declarations to the result. unexpandedDecl->forEachMacroExpandedDecl( [&](ValueDecl *decl) { - Result.push_back(decl); + if (decl->getName().matchesRef(Name)) { + Result.push_back(decl); + } }); } } diff --git a/test/Macros/macro_expand_peers.swift b/test/Macros/macro_expand_peers.swift index 8647561b7923d..273e414f5903e 100644 --- a/test/Macros/macro_expand_peers.swift +++ b/test/Macros/macro_expand_peers.swift @@ -13,7 +13,7 @@ // RUN: %target-swift-frontend -swift-version 5 -typecheck -load-plugin-library %t/%target-library-name(MacroDefinition) -parse-as-library %s -disable-availability-checking -dump-macro-expansions > %t/expansions-dump.txt 2>&1 // RUN: %FileCheck -check-prefix=CHECK-DUMP %s < %t/expansions-dump.txt -// RUN: %target-build-swift -swift-version 5 -Xfrontend -disable-availability-checking -load-plugin-library %t/%target-library-name(MacroDefinition) -parse-as-library %s -o %t/main -module-name MacroUser +// RUN: %target-build-swift -swift-version 5 -Xfrontend -disable-availability-checking -load-plugin-library %t/%target-library-name(MacroDefinition) -parse-as-library %s -o %t/main -module-name MacroUser -g // RUN: %target-codesign %t/main // RUN: %target-run %t/main | %FileCheck %s -check-prefix=CHECK-EXEC From fec82897c1d7ce1fbc975af211d795cda2a9effc Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Wed, 5 Apr 2023 23:54:05 -0700 Subject: [PATCH 2/8] [Macros] Do not look into macro expansions while resolving macro arguments. 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. This addresses a class of cyclic dependencies that we've been seeing with macros, and aligns the lookup behavior for module-level lookups with that specified in the macros proposals. It is not fully complete because lookup until nominal types does not yet support excluding results from macro expansions. (cherry picked from commit f96240d2d8433543c21699633c3701c8c106263b) --- include/swift/AST/Evaluator.h | 29 +++++++- include/swift/AST/NameLookupRequests.h | 17 +++-- lib/AST/Evaluator.cpp | 14 +++- lib/AST/NameLookupRequests.cpp | 93 ++++++++++++++++++++++++++ 4 files changed, 145 insertions(+), 8 deletions(-) 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/NameLookupRequests.cpp b/lib/AST/NameLookupRequests.cpp index 902665eacc09d..6a68b8fc71fb0 100644 --- a/lib/AST/NameLookupRequests.cpp +++ b/lib/AST/NameLookupRequests.cpp @@ -508,6 +508,99 @@ 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) { + // FIXME: Direct lookups don't currently have an "exclude macro expansions" + // flag. + 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" From 494dee982c2eebe449560068354ebb5c4e25224c Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Wed, 5 Apr 2023 23:38:55 -0700 Subject: [PATCH 3/8] [Module-scope lookup] Find arbitrary names in macro expansions. Whenever we perform a name lookup, we need to make sure to expand all macros that use `names: arbitrary`, because of course they can produce... wait for it... arbitrary names, and we don't know what they are until we instatiate them. (cherry picked from commit bbe4ff1e56a4f168b6b31404856a00d20b83bf79) --- lib/AST/Module.cpp | 47 ++++++++++++++++++++++-- lib/Sema/TypeCheckExpr.cpp | 8 ++-- lib/Sema/TypeCheckNameLookup.cpp | 2 + lib/Sema/TypeChecker.h | 2 + test/Macros/top_level_freestanding.swift | 10 +++++ 5 files changed, 62 insertions(+), 7 deletions(-) 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/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/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() +} From 888a5437e955d630c544cd6b10e6bc16b581cdee Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Thu, 6 Apr 2023 14:33:48 -0700 Subject: [PATCH 4/8] [Macros] Suppress lookups into macro expansions in direct lookup in types/extensions Eliminate a source of cyclic dependencies by not expanding macros when we are resolving macro arguments within a type or extension context. This extends the scheme introduced for module-scope lookup to also apply to lookup within types. (cherry picked from commit 527a4cdd42a9ad23e52f76b7998357e5a4ccaec5) --- include/swift/AST/Decl.h | 2 ++ lib/AST/NameLookup.cpp | 38 ++++++++++++++++++++++++++-------- lib/AST/NameLookupRequests.cpp | 13 ++++++++++-- test/Macros/macro_expand.swift | 7 ++++++- 4 files changed, 48 insertions(+), 12 deletions(-) diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h index 2e560c36c5c39..b0378bc770865 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/lib/AST/NameLookup.cpp b/lib/AST/NameLookup.cpp index bd1c746d09aa7..e44345a4db876 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 6a68b8fc71fb0..07d90315795e2 100644 --- a/lib/AST/NameLookupRequests.cpp +++ b/lib/AST/NameLookupRequests.cpp @@ -555,8 +555,17 @@ static UnqualifiedLookupDescriptor excludeMacrosIfNeeded( /// Exclude macros in the direct lookup descriptor if we need to. static DirectLookupDescriptor excludeMacrosIfNeeded( DirectLookupDescriptor descriptor) { - // FIXME: Direct lookups don't currently have an "exclude macro expansions" - // flag. + 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; } diff --git a/test/Macros/macro_expand.swift b/test/Macros/macro_expand.swift index 04b7908520edf..9a975d493f286 100644 --- a/test/Macros/macro_expand.swift +++ b/test/Macros/macro_expand.swift @@ -274,6 +274,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") } @@ -333,9 +337,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. From 0c088c2cacea90da36c66ee914e0787f32d0ada6 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Sat, 8 Apr 2023 13:03:35 -0700 Subject: [PATCH 5/8] [Macros] Mangle attached macro expansions based only on syntactic information The mangling of attached macro expansions based on the declaration to which they are attached requires semantic information (specifically, the interface type of that declaration) that caused cyclic dependencies during type checking. Replace the mangling with a less-complete mangling that only requires syntactic information from the declaration, i.e., the name of the declaration to which the macro was attached. This eliminates reference cycles that occur with attached macros that produce arbitrary names. (cherry picked from commit a23d39bdfb619f5c308893970de28230ef6a97f1) --- docs/ABI/Mangling.rst | 12 +++---- include/swift/AST/ASTMangler.h | 3 +- lib/AST/ASTMangler.cpp | 34 ++++++++++++++------ lib/Demangling/Demangler.cpp | 35 ++++++++++++++++++--- lib/Demangling/NodePrinter.cpp | 30 ++++++++++++------ lib/Demangling/Remangler.cpp | 25 ++++++--------- test/Demangle/Inputs/manglings.txt | 4 +-- test/Macros/accessor_macros.swift | 4 +-- test/Macros/macro_expand.swift | 2 +- test/Macros/macro_expand_conformances.swift | 4 +-- test/Macros/macro_expand_peers.swift | 32 ++++++++++++++----- test/SourceKit/Macros/macro_basic.swift | 20 ++++++------ 12 files changed, 135 insertions(+), 70 deletions(-) diff --git a/docs/ABI/Mangling.rst b/docs/ABI/Mangling.rst index f31ef21d54ab8..8d86f9c0565f7 100644 --- a/docs/ABI/Mangling.rst +++ b/docs/ABI/Mangling.rst @@ -396,13 +396,13 @@ Entities macro-discriminator-list ::= macro-discriminator-list? file-discriminator? macro-expansion-operator INDEX - macro-expansion-operator ::= identifier 'fMa' // attached accessor macro - macro-expansion-operator ::= identifier 'fMA' // attached member-attribute macro + macro-expansion-operator ::= decl-name identifier 'fMa' // attached accessor macro + macro-expansion-operator ::= decl-name identifier 'fMA' // attached member-attribute macro macro-expansion-operator ::= identifier 'fMf' // freestanding macro - macro-expansion-operator ::= identifier 'fMm' // attached member macro - macro-expansion-operator ::= identifier 'fMp' // attached peer macro - macro-expansion-operator ::= identifier 'fMc' // attached conformance macro - macro-expansion-operator ::= identifier 'fMu' // uniquely-named entity + macro-expansion-operator ::= decl-name identifier 'fMm' // attached member macro + macro-expansion-operator ::= decl-name identifier 'fMp' // attached peer macro + macro-expansion-operator ::= decl-name identifier 'fMc' // attached conformance macro + macro-expansion-operator ::= decl-name identifier 'fMu' // uniquely-named entity file-discriminator ::= identifier 'Ll' // anonymous file-discriminated declaration diff --git a/include/swift/AST/ASTMangler.h b/include/swift/AST/ASTMangler.h index de07d203299d3..141aa27e4e0eb 100644 --- a/include/swift/AST/ASTMangler.h +++ b/include/swift/AST/ASTMangler.h @@ -401,7 +401,8 @@ class ASTMangler : public Mangler { void appendType(Type type, GenericSignature sig, const ValueDecl *forDecl = nullptr); - void appendDeclName(const ValueDecl *decl); + void appendDeclName( + const ValueDecl *decl, DeclBaseName name = DeclBaseName()); GenericTypeParamType *appendAssocType(DependentMemberType *DepTy, GenericSignature sig, diff --git a/lib/AST/ASTMangler.cpp b/lib/AST/ASTMangler.cpp index 793595a9789d7..fd22e49d7bffa 100644 --- a/lib/AST/ASTMangler.cpp +++ b/lib/AST/ASTMangler.cpp @@ -1006,8 +1006,9 @@ static Optional getOverriddenSwiftProtocolObjCName( return None; } -void ASTMangler::appendDeclName(const ValueDecl *decl) { - DeclBaseName name = decl->getBaseName(); +void ASTMangler::appendDeclName(const ValueDecl *decl, DeclBaseName name) { + if (name.empty()) + name = decl->getBaseName(); assert(!name.isSpecial() && "Cannot print special names"); auto *synthesizedTypeAttr = @@ -4022,21 +4023,34 @@ std::string ASTMangler::mangleAttachedMacroExpansion( const Decl *decl, CustomAttr *attr, MacroRole role) { beginMangling(); + // Append the context and name of the declaration. + // We don't mangle the declaration itself because doing so requires semantic + // information (e.g., its interface type), which introduces cyclic + // dependencies. const Decl *attachedTo = decl; + DeclBaseName attachedToName; if (auto valueDecl = dyn_cast(decl)) { - if (role != MacroRole::MemberAttribute) { - appendAnyDecl(valueDecl); - } else { - // Appending the member would result in a cycle since `VarDecl` appends - // its type, which would then loop back around to getting the attributes - // again. We'll instead add a discriminator for each member. - appendContextOf(valueDecl); + appendContextOf(valueDecl); + + // Mangle the name, replacing special names with their user-facing names. + attachedToName = valueDecl->getName().getBaseName(); + if (attachedToName.isSpecial()) { + attachedToName = + decl->getASTContext().getIdentifier(attachedToName.userFacingName()); + } + appendDeclName(valueDecl, attachedToName); + + // For member attribute macros, the attribute is attached to the enclosing + // declaration. + if (role == MacroRole::MemberAttribute) { attachedTo = decl->getDeclContext()->getAsDecl(); } } else { appendContext(decl->getDeclContext(), ""); + appendIdentifier("_"); } + // Determine the name of the macro. DeclBaseName macroName; if (auto *macroDecl = attachedTo->getResolvedMacro(attr)) { macroName = macroDecl->getName().getBaseName(); @@ -4044,6 +4058,8 @@ std::string ASTMangler::mangleAttachedMacroExpansion( macroName = decl->getASTContext().getIdentifier("__unknown_macro__"); } + // FIXME: attached macro discriminators should take attachedToName into + // account. appendMacroExpansionOperator( macroName.userFacingName(), role, decl->getAttachedMacroDiscriminator(macroName, role, attr)); diff --git a/lib/Demangling/Demangler.cpp b/lib/Demangling/Demangler.cpp index c8dfa0d7f2ebc..69dd9c86ade2c 100644 --- a/lib/Demangling/Demangler.cpp +++ b/lib/Demangling/Demangler.cpp @@ -4061,47 +4061,74 @@ static bool isMacroExpansionNodeKind(Node::Kind kind) { NodePointer Demangler::demangleMacroExpansion() { Node::Kind kind; + bool isAttached; + bool isFreestanding; switch (nextChar()) { case 'a': kind = Node::Kind::AccessorAttachedMacroExpansion; + isAttached = true; + isFreestanding = false; break; case 'A': kind = Node::Kind::MemberAttributeAttachedMacroExpansion; + isAttached = true; + isFreestanding = false; break; case 'f': kind = Node::Kind::FreestandingMacroExpansion; + isAttached = false; + isFreestanding = true; break; case 'm': kind = Node::Kind::MemberAttachedMacroExpansion; + isAttached = true; + isFreestanding = false; break; case 'p': kind = Node::Kind::PeerAttachedMacroExpansion; + isAttached = true; + isFreestanding = false; break; case 'c': kind = Node::Kind::ConformanceAttachedMacroExpansion; + isAttached = true; + isFreestanding = false; break; case 'u': kind = Node::Kind::MacroExpansionUniqueName; + isAttached = false; + isFreestanding = false; break; default: return nullptr; } - NodePointer name = popNode(Node::Kind::Identifier); - NodePointer privateDiscriminator = popNode(Node::Kind::PrivateDeclName); + NodePointer macroName = popNode(Node::Kind::Identifier); + NodePointer privateDiscriminator = nullptr; + if (isFreestanding) + privateDiscriminator = popNode(Node::Kind::PrivateDeclName); + NodePointer attachedName = nullptr; + if (isAttached) + attachedName = popNode(isDeclName); + NodePointer context = popNode(isMacroExpansionNodeKind); if (!context) context = popContext(); NodePointer discriminator = demangleIndexAsNode(); - auto result = createWithChildren( - kind, context, name, discriminator); + NodePointer result; + if (isAttached) { + result = createWithChildren( + kind, context, attachedName, macroName, discriminator); + } else { + result = createWithChildren(kind, context, macroName, discriminator); + } if (privateDiscriminator) result->addChild(privateDiscriminator, *this); return result; diff --git a/lib/Demangling/NodePrinter.cpp b/lib/Demangling/NodePrinter.cpp index 9ca114bcff9ce..959b15cb9cf64 100644 --- a/lib/Demangling/NodePrinter.cpp +++ b/lib/Demangling/NodePrinter.cpp @@ -1434,28 +1434,38 @@ NodePointer NodePrinter::print(NodePointer Node, unsigned depth, /*hasName*/ true); case Node::Kind::AccessorAttachedMacroExpansion: return printEntity(Node, depth, asPrefixContext, TypePrinting::NoType, - /*hasName*/true, "accessor macro expansion #", - (int)Node->getChild(2)->getIndex() + 1); + /*hasName*/true, + ("accessor macro @" + + nodeToString(Node->getChild(2)) + " expansion #"), + (int)Node->getChild(3)->getIndex() + 1); case Node::Kind::FreestandingMacroExpansion: return printEntity(Node, depth, asPrefixContext, TypePrinting::NoType, /*hasName*/true, "freestanding macro expansion #", (int)Node->getChild(2)->getIndex() + 1); case Node::Kind::MemberAttributeAttachedMacroExpansion: return printEntity(Node, depth, asPrefixContext, TypePrinting::NoType, - /*hasName*/true, "member attribute macro expansion #", - (int)Node->getChild(2)->getIndex() + 1); + /*hasName*/true, + ("member attribute macro @" + + nodeToString(Node->getChild(2)) + " expansion #"), + (int)Node->getChild(3)->getIndex() + 1); case Node::Kind::MemberAttachedMacroExpansion: return printEntity(Node, depth, asPrefixContext, TypePrinting::NoType, - /*hasName*/true, "member macro expansion #", - (int)Node->getChild(2)->getIndex() + 1); + /*hasName*/true, + ("member macro @" + nodeToString(Node->getChild(2)) + + " expansion #"), + (int)Node->getChild(3)->getIndex() + 1); case Node::Kind::PeerAttachedMacroExpansion: return printEntity(Node, depth, asPrefixContext, TypePrinting::NoType, - /*hasName*/true, "peer macro expansion #", - (int)Node->getChild(2)->getIndex() + 1); + /*hasName*/true, + ("peer macro @" + nodeToString(Node->getChild(2)) + + " expansion #"), + (int)Node->getChild(3)->getIndex() + 1); case Node::Kind::ConformanceAttachedMacroExpansion: return printEntity(Node, depth, asPrefixContext, TypePrinting::NoType, - /*hasName*/true, "conformance macro expansion #", - (int)Node->getChild(2)->getIndex() + 1); + /*hasName*/true, + ("conformance macro @" + nodeToString(Node->getChild(2)) + + " expansion #"), + (int)Node->getChild(3)->getIndex() + 1); case Node::Kind::MacroExpansionUniqueName: return printEntity(Node, depth, asPrefixContext, TypePrinting::NoType, /*hasName*/true, "unique name #", diff --git a/lib/Demangling/Remangler.cpp b/lib/Demangling/Remangler.cpp index bec02c27fb30e..bf2e80636571a 100644 --- a/lib/Demangling/Remangler.cpp +++ b/lib/Demangling/Remangler.cpp @@ -2908,51 +2908,46 @@ ManglingError Remangler::mangleFreestandingMacroExpansion( ManglingError Remangler::mangleAccessorAttachedMacroExpansion( Node *node, unsigned depth) { RETURN_IF_ERROR(mangleChildNode(node, 0, depth + 1)); - if (auto privateDiscriminator = node->getChild(3)) - RETURN_IF_ERROR(mangle(privateDiscriminator, depth + 1)); RETURN_IF_ERROR(mangleChildNode(node, 1, depth + 1)); + RETURN_IF_ERROR(mangleChildNode(node, 2, depth + 1)); Buffer << "fMa"; - return mangleChildNode(node, 2, depth + 1); + return mangleChildNode(node, 3, depth + 1); } ManglingError Remangler::mangleMemberAttributeAttachedMacroExpansion( Node *node, unsigned depth) { RETURN_IF_ERROR(mangleChildNode(node, 0, depth + 1)); - if (auto privateDiscriminator = node->getChild(3)) - RETURN_IF_ERROR(mangle(privateDiscriminator, depth + 1)); RETURN_IF_ERROR(mangleChildNode(node, 1, depth + 1)); + RETURN_IF_ERROR(mangleChildNode(node, 2, depth + 1)); Buffer << "fMA"; - return mangleChildNode(node, 2, depth + 1); + return mangleChildNode(node, 3, depth + 1); } ManglingError Remangler::mangleMemberAttachedMacroExpansion( Node *node, unsigned depth) { RETURN_IF_ERROR(mangleChildNode(node, 0, depth + 1)); - if (auto privateDiscriminator = node->getChild(3)) - RETURN_IF_ERROR(mangle(privateDiscriminator, depth + 1)); RETURN_IF_ERROR(mangleChildNode(node, 1, depth + 1)); + RETURN_IF_ERROR(mangleChildNode(node, 2, depth + 1)); Buffer << "fMm"; - return mangleChildNode(node, 2, depth + 1); + return mangleChildNode(node, 3, depth + 1); } ManglingError Remangler::manglePeerAttachedMacroExpansion( Node *node, unsigned depth) { RETURN_IF_ERROR(mangleChildNode(node, 0, depth + 1)); - if (auto privateDiscriminator = node->getChild(3)) - RETURN_IF_ERROR(mangle(privateDiscriminator, depth + 1)); RETURN_IF_ERROR(mangleChildNode(node, 1, depth + 1)); + RETURN_IF_ERROR(mangleChildNode(node, 2, depth + 1)); Buffer << "fMp"; - return mangleChildNode(node, 2, depth + 1); + return mangleChildNode(node, 3, depth + 1); } ManglingError Remangler::mangleConformanceAttachedMacroExpansion( Node *node, unsigned depth) { RETURN_IF_ERROR(mangleChildNode(node, 0, depth + 1)); - if (auto privateDiscriminator = node->getChild(3)) - RETURN_IF_ERROR(mangle(privateDiscriminator, depth + 1)); RETURN_IF_ERROR(mangleChildNode(node, 1, depth + 1)); + RETURN_IF_ERROR(mangleChildNode(node, 2, depth + 1)); Buffer << "fMc"; - return mangleChildNode(node, 2, depth + 1); + return mangleChildNode(node, 3, depth + 1); } ManglingError Remangler::mangleMacroExpansionUniqueName( diff --git a/test/Demangle/Inputs/manglings.txt b/test/Demangle/Inputs/manglings.txt index 844f2ba526d45..ba5f391d61d35 100644 --- a/test/Demangle/Inputs/manglings.txt +++ b/test/Demangle/Inputs/manglings.txt @@ -454,7 +454,5 @@ $s4main4TestVAA3ABCV4hereyySiFfa ---> runtime attribute generator of main.ABC.he $s9MacroUser13testStringify1a1bySi_SitF9stringifyfMf1_ ---> freestanding macro expansion #3 of stringify in MacroUser.testStringify(a: Swift.Int, b: Swift.Int) -> () $s9MacroUser016testFreestandingA9ExpansionyyF4Foo3L_V23bitwidthNumberedStructsfMf_6methodfMu0_ ---> unique name #2 of method in freestanding macro expansion #1 of bitwidthNumberedStructs in Foo3 #1 in MacroUser.testFreestandingMacroExpansion() -> () @__swiftmacro_1a13testStringifyAA1bySi_SitF9stringifyfMf_ ---> freestanding macro expansion #1 of stringify in a.testStringify(a: Swift.Int, b: Swift.Int) -> () -@__swiftmacro_15accessor_macros8MyStructV4nameSSvp17myPropertyWrapperfMa_ ---> accessor macro expansion #1 of myPropertyWrapper in accessor_macros.MyStruct.name : Swift.String -@__swiftmacro_18macro_expand_peers1SV1f1a3for_SSSi_SSSdtYaF20addCompletionHandlerfMp_ ---> peer macro expansion #1 of addCompletionHandler in macro_expand_peers.S.f(a: Swift.Int, for: Swift.String, _: Swift.Double) async -> Swift.String -@__swiftmacro_25macro_expand_conformances7GenericV20DelegatedConformancefMc_ ---> conformance macro expansion #1 of DelegatedConformance in macro_expand_conformances.Generic +@__swiftmacro_18macro_expand_peers1SV1f20addCompletionHandlerfMp_ ---> peer macro @addCompletionHandler expansion #1 of f in macro_expand_peers.S @__swiftmacro_9MacroUser16MemberNotCoveredV33_4361AD9339943F52AE6186DD51E04E91Ll0dE0fMf0_ ---> freestanding macro expansion #2 of NotCovered(in _4361AD9339943F52AE6186DD51E04E91) in MacroUser.MemberNotCovered diff --git a/test/Macros/accessor_macros.swift b/test/Macros/accessor_macros.swift index d00e710969d72..3b9df438b34b5 100644 --- a/test/Macros/accessor_macros.swift +++ b/test/Macros/accessor_macros.swift @@ -43,7 +43,7 @@ struct MyStruct { @myPropertyWrapper var name: String - // CHECK-DUMP: @__swiftmacro_15accessor_macros8MyStructV4nameSSvp17myPropertyWrapperfMa_.swift + // CHECK-DUMP: @__swiftmacro_15accessor_macros8MyStructV4name17myPropertyWrapperfMa_.swift // CHECK-DUMP: get { // CHECK-DUMP: _name.wrappedValue // CHECK-DUMP: } @@ -53,7 +53,7 @@ struct MyStruct { @myPropertyWrapper var birthDate: Date? - // CHECK-DUMP: @__swiftmacro_15accessor_macros8MyStructV9birthDateAA0F0VSgvp17myPropertyWrapperfMa_.swift + // CHECK-DUMP: @__swiftmacro_15accessor_macros8MyStructV9birthDate17myPropertyWrapperfMa_.swift // CHECK-DUMP: get { // CHECK-DUMP: _birthDate.wrappedValue // CHECK-DUMP: } diff --git a/test/Macros/macro_expand.swift b/test/Macros/macro_expand.swift index 9a975d493f286..908f202b9ba41 100644 --- a/test/Macros/macro_expand.swift +++ b/test/Macros/macro_expand.swift @@ -67,7 +67,7 @@ struct Bad {} // CHECK-DIAGS: error: macro expansion cannot introduce default literal type '_ImageLiteralType' // CHECK-DIAGS: error: macro expansion cannot introduce default literal type '_FileReferenceLiteralType' -// CHECK-DIAGS: CONTENTS OF FILE @__swiftmacro_9MacroUser3BadV7InvalidfMp_.swift +// CHECK-DIAGS: CONTENTS OF FILE @__swiftmacro_9MacroUser3Bad7InvalidfMp_.swift // CHECK-DIAGS: import Swift // CHECK-DIAGS: precedencegroup MyPrecedence {} // CHECK-DIAGS: @attached(member) macro myMacro() diff --git a/test/Macros/macro_expand_conformances.swift b/test/Macros/macro_expand_conformances.swift index 39840a9f9bc90..6c9116b06e0ed 100644 --- a/test/Macros/macro_expand_conformances.swift +++ b/test/Macros/macro_expand_conformances.swift @@ -29,7 +29,7 @@ struct S {} @Hashable struct S2 {} -// CHECK-DUMP: @__swiftmacro_25macro_expand_conformances1SV9EquatablefMc_.swift +// CHECK-DUMP: @__swiftmacro_25macro_expand_conformances1S9EquatablefMc_.swift // CHECK-DUMP: extension S : Equatable {} // CHECK: true @@ -55,7 +55,7 @@ struct Wrapped: P { @DelegatedConformance struct Generic {} -// CHECK-DUMP: @__swiftmacro_25macro_expand_conformances7GenericV20DelegatedConformancefMc_.swift +// CHECK-DUMP: @__swiftmacro_25macro_expand_conformances7Generic20DelegatedConformancefMc_.swift // CHECK-DUMP: extension Generic : P where Element: P {} func requiresP(_ value: (some P).Type) { diff --git a/test/Macros/macro_expand_peers.swift b/test/Macros/macro_expand_peers.swift index 273e414f5903e..b10d564495b00 100644 --- a/test/Macros/macro_expand_peers.swift +++ b/test/Macros/macro_expand_peers.swift @@ -29,13 +29,16 @@ macro addCompletionHandler() = #externalMacro(module: "MacroDefinition", type: " macro AddClassReferencingSelf() = #externalMacro(module: "MacroDefinition", type: "AddClassReferencingSelfMacro") #endif +@attached(peer, names: arbitrary) +macro addCompletionHandlerArbitrarily(_: Int) = #externalMacro(module: "MacroDefinition", type: "AddCompletionHandler") + struct S { @addCompletionHandler func f(a: Int, for b: String, _ value: Double) async -> String { return b } - // CHECK-DUMP: @__swiftmacro_18macro_expand_peers1SV1f1a3for_SSSi_SSSdtYaF20addCompletionHandlerfMp_.swift + // CHECK-DUMP: @__swiftmacro_18macro_expand_peers1SV1f20addCompletionHandlerfMp_.swift // CHECK-DUMP: func f(a: Int, for b: String, _ value: Double, completionHandler: @escaping (String) -> Void) { // CHECK-DUMP: Task { // CHECK-DUMP: completionHandler(await f(a: a, for: b, value)) @@ -55,10 +58,10 @@ extension S { return b } - // CHECK-DUMP: @__swiftmacro_18macro_expand_peers1SV1g1a3for_SSSi_SSSdtYaF20addCompletionHandlerfMp_.swift - // CHECK-DUMP: func f(a: Int, for b: String, _ value: Double, completionHandler: @escaping (String) -> Void) { + // CHECK-DUMP: @__swiftmacro_18macro_expand_peers1SV1g20addCompletionHandlerfMp_.swift + // CHECK-DUMP: func g(a: Int, for b: String, _ value: Double, completionHandler: @escaping (String) -> Void) { // CHECK-DUMP: Task { - // CHECK-DUMP: completionHandler(await f(a: a, for: b, value)) + // CHECK-DUMP: completionHandler(await g(a: a, for: b, value)) // CHECK-DUMP: } // CHECK-DUMP: } @@ -97,9 +100,9 @@ func global(a: Int, b: String) { print(a, b) } -// CHECK-DUMP: @__swiftmacro_18macro_expand_peers6global1a1bySi_SStF10wrapInTypefMp_.swift -// CHECK-DUMP: struct $s18macro_expand_peers6global1a1bySi_SStF10wrapInTypefMp_6globalfMu0_ { -// CHECK-DUMP: func $s18macro_expand_peers6global1a1bySi_SStF10wrapInTypefMp_6globalfMu_(a: Int, b: String) { +// CHECK-DUMP: @__swiftmacro_18macro_expand_peers6global10wrapInTypefMp_.swift +// CHECK-DUMP: struct $s18macro_expand_peers6global10wrapInTypefMp_6globalfMu0_ { +// CHECK-DUMP: func $s18macro_expand_peers6global10wrapInTypefMp_6globalfMu_(a: Int, b: String) { // CHECK-DUMP: global(a: a, b: b) // CHECK-DUMP: } // CHECK-DUMP: } @@ -126,3 +129,18 @@ struct Main { @AddClassReferencingSelf protocol MyProto { } + +// Reference cycles amongst arbitrary peer macros and macro arguments. +let x = 10 +let y = 10 +struct S2 { + @addCompletionHandlerArbitrarily(x) + func f(a: Int, for b: String, _ value: Double) async -> String { + return b + } + + @addCompletionHandlerArbitrarily(y) + func g(a: Int, for b: String, _ value: Double) async -> String { + return b + } +} diff --git a/test/SourceKit/Macros/macro_basic.swift b/test/SourceKit/Macros/macro_basic.swift index 05f909356cdcf..1b83d297bc8d8 100644 --- a/test/SourceKit/Macros/macro_basic.swift +++ b/test/SourceKit/Macros/macro_basic.swift @@ -165,11 +165,11 @@ macro anonymousTypes(_: () -> String) = #externalMacro(module: "MacroDefinition" // RUN: %sourcekitd-test -req=refactoring.expand.macro -pos=21:1 %s -- ${COMPILER_ARGS[@]} | %FileCheck -check-prefix=ATTACHED_EXPAND %s // RUN: %sourcekitd-test -req=refactoring.expand.macro -pos=21:2 %s -- ${COMPILER_ARGS[@]} | %FileCheck -check-prefix=ATTACHED_EXPAND %s // ATTACHED_EXPAND: source.edit.kind.active: -// ATTACHED_EXPAND-NEXT: 23:3-23:3 (@__swiftmacro_9MacroUser1SV13myTypeWrapperfMA_.swift) "@accessViaStorage " +// ATTACHED_EXPAND-NEXT: 23:3-23:3 (@__swiftmacro_9MacroUser1SV1x13myTypeWrapperfMA_.swift) "@accessViaStorage " // ATTACHED_EXPAND-NEXT: source.edit.kind.active: -// ATTACHED_EXPAND-NEXT: 24:3-24:3 (@__swiftmacro_9MacroUser1SV13myTypeWrapperfMA0_.swift) "@accessViaStorage " +// ATTACHED_EXPAND-NEXT: 24:3-24:3 (@__swiftmacro_9MacroUser1SV1y13myTypeWrapperfMA0_.swift) "@accessViaStorage " // ATTACHED_EXPAND-NEXT: source.edit.kind.active: -// ATTACHED_EXPAND-NEXT: 25:1-25:1 (@__swiftmacro_9MacroUser1SV13myTypeWrapperfMm_.swift) " +// ATTACHED_EXPAND-NEXT: 25:1-25:1 (@__swiftmacro_9MacroUser1S13myTypeWrapperfMm_.swift) " // ATTACHED_EXPAND-EMPTY: // ATTACHED_EXPAND-NEXT: private var _storage = _Storage() // ATTACHED_EXPAND-NEXT: " @@ -177,7 +177,7 @@ macro anonymousTypes(_: () -> String) = #externalMacro(module: "MacroDefinition" // ATTACHED_EXPAND-NEXT: 21:1-21:15 "" //##-- Cursor info on the attribute expanded by @myTypeWrapper -// RUN: %sourcekitd-test -req=cursor -cursor-action -req-opts=retrieve_symbol_graph=1 -offset=2 @__swiftmacro_9MacroUser1SV13myTypeWrapperfMA_.swift -primary-file %s -- ${COMPILER_ARGS[@]} | %FileCheck -check-prefix=NESTED_ATTACHED_CURSOR %s +// RUN: %sourcekitd-test -req=cursor -cursor-action -req-opts=retrieve_symbol_graph=1 -offset=2 @__swiftmacro_9MacroUser1SV1x13myTypeWrapperfMA_.swift -primary-file %s -- ${COMPILER_ARGS[@]} | %FileCheck -check-prefix=NESTED_ATTACHED_CURSOR %s // NESTED_ATTACHED_CURSOR: source.lang.swift.ref.macro // NESTED_ATTACHED_CURSOR-SAME: macro_basic.swift:10:27-10:43 // NESTED_ATTACHED_CURSOR-LABEL: SYMBOL GRAPH BEGIN @@ -196,9 +196,9 @@ macro anonymousTypes(_: () -> String) = #externalMacro(module: "MacroDefinition" // NESTED_ATTACHED_CURSOR-NEXT: ACTIONS END //##-- Expansion on the attribute expanded by @myTypeWrapper -// RUN: %sourcekitd-test -req=refactoring.expand.macro -pos=1:2 @__swiftmacro_9MacroUser1SV13myTypeWrapperfMA_.swift -primary-file %s -- ${COMPILER_ARGS[@]} | %FileCheck -check-prefix=NESTED_ATTACHED_EXPAND %s +// RUN: %sourcekitd-test -req=refactoring.expand.macro -pos=1:2 @__swiftmacro_9MacroUser1SV1x13myTypeWrapperfMA_.swift -primary-file %s -- ${COMPILER_ARGS[@]} | %FileCheck -check-prefix=NESTED_ATTACHED_EXPAND %s // NESTED_ATTACHED_EXPAND: source.edit.kind.active: -// NESTED_ATTACHED_EXPAND-NEXT: Macros/macro_basic.swift 23:13-23:13 (@__swiftmacro_9MacroUser1SV1xSivp16accessViaStoragefMa_.swift) "{ +// NESTED_ATTACHED_EXPAND-NEXT: Macros/macro_basic.swift 23:13-23:13 (@__swiftmacro_9MacroUser1SV1x16accessViaStoragefMa_.swift) "{ // NESTED_ATTACHED_EXPAND-NEXT: get { _storage.x } // NESTED_ATTACHED_EXPAND-EMPTY: // NESTED_ATTACHED_EXPAND-NEXT: set { _storage.x = newValue } @@ -209,7 +209,7 @@ macro anonymousTypes(_: () -> String) = #externalMacro(module: "MacroDefinition" //##-- Expansion on the first accessor macro // RUN: %sourcekitd-test -req=refactoring.expand.macro -pos=30:4 %s -- ${COMPILER_ARGS[@]} | %FileCheck -check-prefix=ACCESSOR1_EXPAND %s // ACCESSOR1_EXPAND: source.edit.kind.active: -// ACCESSOR1_EXPAND-NEXT: 31:13-31:13 (@__swiftmacro_9MacroUser2S2V1xSivp16accessViaStoragefMa_.swift) "{ +// ACCESSOR1_EXPAND-NEXT: 31:13-31:13 (@__swiftmacro_9MacroUser2S2V1x16accessViaStoragefMa_.swift) "{ // ACCESSOR1_EXPAND-NEXT: get { _storage.x } // ACCESSOR1_EXPAND-EMPTY: // ACCESSOR1_EXPAND-NEXT: set { _storage.x = newValue } @@ -220,7 +220,7 @@ macro anonymousTypes(_: () -> String) = #externalMacro(module: "MacroDefinition" //##-- Expansion on the second accessor macro // RUN: %sourcekitd-test -req=refactoring.expand.macro -pos=33:13 %s -- ${COMPILER_ARGS[@]} | %FileCheck -check-prefix=ACCESSOR2_EXPAND %s // ACCESSOR2_EXPAND: source.edit.kind.active: -// ACCESSOR2_EXPAND-NEXT: 34:14-34:18 (@__swiftmacro_9MacroUser2S2V1ySivp16accessViaStoragefMa_.swift) "{ +// ACCESSOR2_EXPAND-NEXT: 34:14-34:18 (@__swiftmacro_9MacroUser2S2V1y16accessViaStoragefMa_.swift) "{ // ACCESSOR2_EXPAND-NEXT: get { _storage.y } // ACCESSOR2_EXPAND-EMPTY: // ACCESSOR2_EXPAND-NEXT: set { _storage.y = newValue } @@ -231,7 +231,7 @@ macro anonymousTypes(_: () -> String) = #externalMacro(module: "MacroDefinition" //##-- Expansion on the addCompletionHandler macro. // RUN: %sourcekitd-test -req=refactoring.expand.macro -pos=42:5 %s -- ${COMPILER_ARGS[@]} | %FileCheck -check-prefix=PEER_EXPAND %s // PEER_EXPAND: source.edit.kind.active: -// PEER_EXPAND-NEXT: 45:4-45:4 (@__swiftmacro_9MacroUser2S3V1f1a3for_SSSi_SSSdtYaF20addCompletionHandlerfMp_.swift) " +// PEER_EXPAND-NEXT: 45:4-45:4 (@__swiftmacro_9MacroUser2S3V1f20addCompletionHandlerfMp_.swift) " // PEER_EXPAND-EMPTY: // PEER_EXPAND-NEXT: func f(a: Int, for b: String, _ value: Double, completionHandler: @escaping (String) -> Void) { // PEER_EXPAND-NEXT: Task { @@ -245,7 +245,7 @@ macro anonymousTypes(_: () -> String) = #externalMacro(module: "MacroDefinition" //##-- Expansion on a conformance macro. // RUN: %sourcekitd-test -req=refactoring.expand.macro -pos=51:5 %s -- ${COMPILER_ARGS[@]} | %FileCheck -check-prefix=CONFORMANCE_EXPAND %s // CONFORMANCE_EXPAND: source.edit.kind.active: -// CONFORMANCE_EXPAND-NEXT: 52:14-52:14 (@__swiftmacro_9MacroUser2S4V8HashablefMc_.swift) " +// CONFORMANCE_EXPAND-NEXT: 52:14-52:14 (@__swiftmacro_9MacroUser2S48HashablefMc_.swift) " // CONFORMANCE_EXPAND-EMPTY: // CONFORMANCE_EXPAND-NEXT: extension S4 : Hashable {} // CONFORMANCE_EXPAND-NEXT: " From b5110ee8f5f36ded062f9d70db253521b269edb0 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Sun, 9 Apr 2023 22:12:37 -0400 Subject: [PATCH 6/8] [Macros] Ensure that we compute the interface type before we add accessors Adding accessors to a stored property, which removes its initializer. Force computation of the interface type first. There are better ways to model this in the AST, but it requires reworking how we handle initializers to break them into requests. (cherry picked from commit ef7970bad3a3c3dc71dab87ec064c9f719c43106) --- lib/Sema/TypeCheckMacros.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/Sema/TypeCheckMacros.cpp b/lib/Sema/TypeCheckMacros.cpp index fc115c774c911..5fc3efecc2a8c 100644 --- a/lib/Sema/TypeCheckMacros.cpp +++ b/lib/Sema/TypeCheckMacros.cpp @@ -1330,6 +1330,7 @@ evaluateAttachedMacro(MacroDecl *macro, Decl *attachedTo, CustomAttr *attr, Optional swift::expandAccessors( AbstractStorageDecl *storage, CustomAttr *attr, MacroDecl *macro ) { + (void)storage->getInterfaceType(); // Evaluate the macro. auto macroSourceFile = evaluateAttachedMacro(macro, storage, attr, /*passParentContext*/false, From c1e66198550578cd38db686c2dcc9cb59a204b18 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Tue, 11 Apr 2023 07:41:12 -0400 Subject: [PATCH 7/8] Fix fragile test (cherry picked from commit 097ecfd545b7b7880838c6e52821a5ceeec8eef2) --- test/stdlib/symbol-visibility-linux.test-sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/stdlib/symbol-visibility-linux.test-sh b/test/stdlib/symbol-visibility-linux.test-sh index a8c59bd1a7af5..044c4dd6f7c98 100644 --- a/test/stdlib/symbol-visibility-linux.test-sh +++ b/test/stdlib/symbol-visibility-linux.test-sh @@ -27,6 +27,8 @@ // RUN: -e _ZNSt3_V28__rotateIPcEET_S2_S2_S2_St26random_access_iterator_tag \ // RUN: -e _ZN9__gnu_cxx12__to_xstringINSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEcEET_PFiPT0_mPKS8_P13__va_list_tagEmSB_z \ // RUN: -e _ZN9__gnu_cxx12__to_xstringINSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEcEET_PFiPT0_mPKS8_St9__va_listEmSB_z \ +// RUN: -e _ZStplIcSt11char_traitsIcESaIcEENSt7__cxx1112basic_stringIT_T0_T1_EEOS8_PKS5_ \ +// RUN: -e _ZStplIcSt11char_traitsIcESaIcEENSt7__cxx1112basic_stringIT_T0_T1_EEPKS5_OS8_ \ // RUN: -e _ZZNSt19_Sp_make_shared_tag5_S_tiEvE5__tag \ // RUN: -e _ZSt16__once_call_implISt12_Bind_simpleIFPFvPvEDnEEEvv \ // RUN: -e '_ZSt16__once_call_implISt12_Bind_simpleIFPFvPvEPA[0-9]\+_cEEEvv' \ From d1fe5f492fd44f3563b247230d7bb54e8b358f5e Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Tue, 11 Apr 2023 22:36:41 -0400 Subject: [PATCH 8/8] Fix Linux flaky test completely (cherry picked from commit fb82b8f7110716cf0c1f6713847d7eb883fac35a) --- test/stdlib/symbol-visibility-linux.test-sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/stdlib/symbol-visibility-linux.test-sh b/test/stdlib/symbol-visibility-linux.test-sh index 044c4dd6f7c98..bfcb2343543fa 100644 --- a/test/stdlib/symbol-visibility-linux.test-sh +++ b/test/stdlib/symbol-visibility-linux.test-sh @@ -286,6 +286,8 @@ // RUN: -e _ZSteqIcSt11char_traitsIcESaIcEEbRKNSt7__cxx1112basic_stringIT_T0_T1_EEPKS5_ \ // RUN: -e _ZStplIcSt11char_traitsIcESaIcEENSt7__cxx1112basic_stringIT_T0_T1_EEPKS5_RKS8_ \ // RUN: -e _ZN9__gnu_cxx32__throw_concurrence_unlock_errorEv \ +// RUN: -e _ZStplIcSt11char_traitsIcESaIcEENSt7__cxx1112basic_stringIT_T0_T1_EEOS8_PKS5_ \ +// RUN: -e _ZStplIcSt11char_traitsIcESaIcEENSt7__cxx1112basic_stringIT_T0_T1_EEPKS5_OS8_ \ // RUN: -e _ZNKSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEE11_M_disjunctEPKc \ // RUN: -e _ZNKSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEE11_M_is_localEv \ // RUN: -e _ZNKSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEE12find_last_ofEPKcm \