From b6e9c8e51c157df3fac29a39d1dc262d4dd4bdf8 Mon Sep 17 00:00:00 2001 From: Arnold Schwaighofer Date: Fri, 9 Jun 2023 09:12:37 -0700 Subject: [PATCH 01/10] IRGen: alloc_global and global_addr instructions need to agree on the storage If the storage is opaque we need to project to the underlying buffer. rdar://109636344 --- lib/IRGen/IRGenSIL.cpp | 15 +++++++++++---- test/IRGen/globals.swift | 15 +++++++++++++++ 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/lib/IRGen/IRGenSIL.cpp b/lib/IRGen/IRGenSIL.cpp index 4b1ffb0b71ea0..dad6ac28e28e0 100644 --- a/lib/IRGen/IRGenSIL.cpp +++ b/lib/IRGen/IRGenSIL.cpp @@ -2942,10 +2942,9 @@ void IRGenSILFunction::visitAllocGlobalInst(AllocGlobalInst *i) { void IRGenSILFunction::visitGlobalAddrInst(GlobalAddrInst *i) { SILGlobalVariable *var = i->getReferencedGlobal(); - SILType loweredTy = var->getLoweredTypeInContext(getExpansionContext()); - assert(loweredTy == i->getType().getObjectType()); + SILType loweredTy = var->getLoweredType(); auto &ti = getTypeInfo(loweredTy); - + auto expansion = IGM.getResilienceExpansionForLayout(var); // If the variable is empty in all resilience domains that can see it, @@ -2968,7 +2967,15 @@ void IRGenSILFunction::visitGlobalAddrInst(GlobalAddrInst *i) { // Otherwise, the static storage for the global consists of a fixed-size // buffer; project it. addr = emitProjectValueInBuffer(*this, loweredTy, addr); - + + + // Get the address of the type in context. + SILType loweredTyInContext = var->getLoweredTypeInContext(getExpansionContext()); + auto &tiInContext = getTypeInfo(loweredTyInContext); + auto ptr = Builder.CreateBitOrPointerCast(addr.getAddress(), + tiInContext.getStorageType()->getPointerTo()); + addr = Address(ptr, tiInContext.getStorageType(), + tiInContext.getBestKnownAlignment()); setLoweredAddress(i, addr); } diff --git a/test/IRGen/globals.swift b/test/IRGen/globals.swift index dce8d83ad2208..43384f563ef3e 100644 --- a/test/IRGen/globals.swift +++ b/test/IRGen/globals.swift @@ -54,3 +54,18 @@ extension A { // CHECK: define{{( dllexport)?}}{{( protected)?}} i32 @main(i32 %0, i8** %1) {{.*}} { // CHECK: store i64 {{.*}}, i64* getelementptr inbounds ([[INT]], [[INT]]* @"$s7globals2g0Sivp", i32 0, i32 0), align 8 +// CHECK: [[BUF_PROJ:%.*]] = call {{.*}} @__swift_project_value_buffer({{.*}}s7globals1gQrvp +// CHECK: [[CAST:%.*]] = bitcast {{.*}} [[BUF_PROJ]] +// CHECK: [[CAST2:%.*]] = bitcast {{.*}} [[CAST]] +// CHECK: call void @llvm.memcpy{{.*}}({{.*}}[[CAST2]] + + +public protocol Some {} + +public struct Implementer : Some { + var w = (0, 1, 2, 3, 4) + + public init() { } +} + +let g : some Some = Implementer() From 3b06c348fa9f6ef2cbe0260e883f8704257696bc Mon Sep 17 00:00:00 2001 From: Arnold Schwaighofer Date: Fri, 9 Jun 2023 15:02:39 -0700 Subject: [PATCH 02/10] Fix test --- test/IRGen/globals.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/IRGen/globals.swift b/test/IRGen/globals.swift index 43384f563ef3e..4c84f3270eec9 100644 --- a/test/IRGen/globals.swift +++ b/test/IRGen/globals.swift @@ -1,4 +1,4 @@ -// RUN: %target-swift-frontend -primary-file %s -emit-ir | %FileCheck %s +// RUN: %target-swift-frontend -primary-file %s -disable-availability-checking -emit-ir | %FileCheck %s // REQUIRES: swift_in_compiler // REQUIRES: PTRSIZE=64 From ba8d4d780161fcfe02304182ddabce714f2cb0a3 Mon Sep 17 00:00:00 2001 From: Alex Lorenz Date: Wed, 7 Jun 2023 15:06:06 -0700 Subject: [PATCH 03/10] [cxx-interop] compilations that do not enable C++ interoperability should not be able to import modules that do enable C++ interoperability by default A supplemental hidden frontend option allows advanced users to opt-out of this requirement. Fixes https://github.com/apple/swift/issues/65833 Fixes https://github.com/apple/swift/issues/65832 --- include/swift/AST/Decl.h | 7 +++++-- include/swift/AST/DiagnosticsSema.def | 6 ++++++ include/swift/AST/Module.h | 8 +++++++ include/swift/Basic/LangOptions.h | 4 ++++ include/swift/Option/FrontendOptions.td | 5 +++++ include/swift/Serialization/Validation.h | 5 +++++ lib/AST/Module.cpp | 1 + lib/Frontend/CompilerInvocation.cpp | 2 ++ lib/Frontend/Frontend.cpp | 3 +++ lib/Serialization/ModuleFile.h | 5 +++++ lib/Serialization/ModuleFileSharedCore.cpp | 4 ++++ lib/Serialization/ModuleFileSharedCore.h | 5 ++++- lib/Serialization/ModuleFormat.h | 7 ++++++- lib/Serialization/Serialization.cpp | 7 +++++++ lib/Serialization/SerializedModuleLoader.cpp | 15 +++++++++++++ ...y-cxx-interop-module-without-interop.swift | 4 ++-- ...tdlib-interop-module-without-interop.swift | 2 +- ...-import-into-non-interop-compilation.swift | 21 +++++++++++++++++++ 18 files changed, 104 insertions(+), 7 deletions(-) create mode 100644 test/Interop/Cxx/modules/prohibit-import-into-non-interop-compilation.swift diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h index 792568c22d518..c691a16bd3b17 100644 --- a/include/swift/AST/Decl.h +++ b/include/swift/AST/Decl.h @@ -659,7 +659,7 @@ class alignas(1 << DeclAlignInBits) Decl : public ASTAllocated { HasAnyUnavailableValues : 1 ); - SWIFT_INLINE_BITFIELD(ModuleDecl, TypeDecl, 1+1+1+1+1+1+1+1+1+1+1+1+1+1+1, + SWIFT_INLINE_BITFIELD(ModuleDecl, TypeDecl, 1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1, /// If the module is compiled as static library. StaticLibrary : 1, @@ -709,7 +709,10 @@ class alignas(1 << DeclAlignInBits) Decl : public ASTAllocated { /// If the map from @objc provided name to top level swift::Decl in this /// module is populated - ObjCNameLookupCachePopulated : 1 + ObjCNameLookupCachePopulated : 1, + + /// Whether this module has been built with C++ interoperability enabled. + HasCxxInteroperability : 1 ); SWIFT_INLINE_BITFIELD(PrecedenceGroupDecl, Decl, 1+2, diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 314bfab93b424..f5bd8f400f2a0 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -874,6 +874,12 @@ ERROR(need_hermetic_seal_to_import_module,none, "module %0 was built with -experimental-hermetic-seal-at-link, but " "current compilation does not have -experimental-hermetic-seal-at-link", (Identifier)) +ERROR(need_cxx_interop_to_import_module,none, + "module %0 was built with C++ interoperability enabled, but " + "current compilation does not enable C++ interoperability", + (Identifier)) +NOTE(enable_cxx_interop_docs,none, + "visit https://www.swift.org/documentation/cxx-interop/project-build-setup to learn how to enable C++ interoperability", ()) ERROR(modularization_issue_decl_moved,Fatal, "reference to %select{top-level declaration|type}0 %1 broken by a context change; " diff --git a/include/swift/AST/Module.h b/include/swift/AST/Module.h index b15b881b5bf29..b1f0c7f6a24a9 100644 --- a/include/swift/AST/Module.h +++ b/include/swift/AST/Module.h @@ -654,6 +654,14 @@ class ModuleDecl Bits.ModuleDecl.HasHermeticSealAtLink = enabled; } + /// Returns true if this module was built with C++ interoperability enabled. + bool hasCxxInteroperability() const { + return Bits.ModuleDecl.HasCxxInteroperability; + } + void setHasCxxInteroperability(bool enabled = true) { + Bits.ModuleDecl.HasCxxInteroperability = enabled; + } + /// \returns true if this module is a system module; note that the StdLib is /// considered a system module. bool isSystemModule() const { diff --git a/include/swift/Basic/LangOptions.h b/include/swift/Basic/LangOptions.h index 782803d85d6fc..8f77bc68ae512 100644 --- a/include/swift/Basic/LangOptions.h +++ b/include/swift/Basic/LangOptions.h @@ -313,6 +313,10 @@ namespace swift { /// Imports getters and setters as computed properties. bool CxxInteropGettersSettersAsProperties = false; + /// Should the compiler require C++ interoperability to be enabled + /// when importing Swift modules that enable C++ interoperability. + bool RequireCxxInteropToImportCxxInteropModule = true; + /// On Darwin platforms, use the pre-stable ABI's mark bit for Swift /// classes instead of the stable ABI's bit. This is needed when /// targeting OSes prior to macOS 10.14.4 and iOS 12.2, where diff --git a/include/swift/Option/FrontendOptions.td b/include/swift/Option/FrontendOptions.td index df09e0f4ac622..8e8ed8d5b46bb 100644 --- a/include/swift/Option/FrontendOptions.td +++ b/include/swift/Option/FrontendOptions.td @@ -865,6 +865,11 @@ def cxx_interop_getters_setters_as_properties : HelpText<"Import getters and setters as computed properties in Swift">, Flags<[FrontendOption, HelpHidden]>; +def cxx_interop_disable_requirement_at_import : + Flag<["-"], "disable-cxx-interop-requirement-at-import">, + HelpText<"Do not require C++ interoperability to be enabled when importing a Swift module that enables C++ interoperability">, + Flags<[FrontendOption, HelpHidden]>; + def use_malloc : Flag<["-"], "use-malloc">, HelpText<"Allocate internal data structures using malloc " "(for memory debugging)">; diff --git a/include/swift/Serialization/Validation.h b/include/swift/Serialization/Validation.h index b2404b8516579..a82380e74963a 100644 --- a/include/swift/Serialization/Validation.h +++ b/include/swift/Serialization/Validation.h @@ -131,6 +131,7 @@ class ExtendedValidationInfo { unsigned IsBuiltFromInterface : 1; unsigned IsAllowModuleWithCompilerErrorsEnabled : 1; unsigned IsConcurrencyChecked : 1; + unsigned HasCxxInteroperability : 1; } Bits; public: ExtendedValidationInfo() : Bits() {} @@ -232,6 +233,10 @@ class ExtendedValidationInfo { void setIsConcurrencyChecked(bool val = true) { Bits.IsConcurrencyChecked = val; } + bool hasCxxInteroperability() const { return Bits.HasCxxInteroperability; } + void setHasCxxInteroperability(bool val) { + Bits.HasCxxInteroperability = val; + } }; struct SearchPath { diff --git a/lib/AST/Module.cpp b/lib/AST/Module.cpp index 0c695de33dfba..dd8a02dcb3f1b 100644 --- a/lib/AST/Module.cpp +++ b/lib/AST/Module.cpp @@ -646,6 +646,7 @@ ModuleDecl::ModuleDecl(Identifier name, ASTContext &ctx, Bits.ModuleDecl.HasHermeticSealAtLink = 0; Bits.ModuleDecl.IsConcurrencyChecked = 0; Bits.ModuleDecl.ObjCNameLookupCachePopulated = 0; + Bits.ModuleDecl.HasCxxInteroperability = 0; } void ModuleDecl::setIsSystemModule(bool flag) { diff --git a/lib/Frontend/CompilerInvocation.cpp b/lib/Frontend/CompilerInvocation.cpp index 6849bc3f8e690..c75d5a467a4bd 100644 --- a/lib/Frontend/CompilerInvocation.cpp +++ b/lib/Frontend/CompilerInvocation.cpp @@ -1043,6 +1043,8 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args, Args.hasArg(OPT_experimental_c_foreign_reference_types); Opts.CxxInteropGettersSettersAsProperties = Args.hasArg(OPT_cxx_interop_getters_setters_as_properties); + Opts.RequireCxxInteropToImportCxxInteropModule = + !Args.hasArg(OPT_cxx_interop_disable_requirement_at_import); Opts.VerifyAllSubstitutionMaps |= Args.hasArg(OPT_verify_all_substitution_maps); diff --git a/lib/Frontend/Frontend.cpp b/lib/Frontend/Frontend.cpp index b4ff90136cf24..461c34938b15c 100644 --- a/lib/Frontend/Frontend.cpp +++ b/lib/Frontend/Frontend.cpp @@ -1319,6 +1319,9 @@ ModuleDecl *CompilerInstance::getMainModule() const { MainModule->setResilienceStrategy(ResilienceStrategy::Resilient); if (Invocation.getLangOptions().isSwiftVersionAtLeast(6)) MainModule->setIsConcurrencyChecked(true); + if (Invocation.getLangOptions().EnableCXXInterop && + Invocation.getLangOptions().RequireCxxInteropToImportCxxInteropModule) + MainModule->setHasCxxInteroperability(); // Register the main module with the AST context. Context->addLoadedModule(MainModule); diff --git a/lib/Serialization/ModuleFile.h b/lib/Serialization/ModuleFile.h index 919f0f577e162..158b4f5edd0d6 100644 --- a/lib/Serialization/ModuleFile.h +++ b/lib/Serialization/ModuleFile.h @@ -621,6 +621,11 @@ class ModuleFile return Core->Bits.HasHermeticSealAtLink; } + /// Whether this module was built with C++ interoperability enabled. + bool hasCxxInteroperability() const { + return Core->Bits.HasCxxInteroperability; + } + /// Whether the module is resilient. ('-enable-library-evolution') ResilienceStrategy getResilienceStrategy() const { return ResilienceStrategy(Core->Bits.ResilienceStrategy); diff --git a/lib/Serialization/ModuleFileSharedCore.cpp b/lib/Serialization/ModuleFileSharedCore.cpp index a96dea3cd0106..d4c325e22c6ea 100644 --- a/lib/Serialization/ModuleFileSharedCore.cpp +++ b/lib/Serialization/ModuleFileSharedCore.cpp @@ -181,6 +181,9 @@ static bool readOptionsBlock(llvm::BitstreamCursor &cursor, case options_block::MODULE_EXPORT_AS_NAME: extendedInfo.setExportAsName(blobData); break; + case options_block::HAS_CXX_INTEROPERABILITY_ENABLED: + extendedInfo.setHasCxxInteroperability(true); + break; default: // Unknown options record, possibly for use by a future version of the // module format. @@ -1378,6 +1381,7 @@ ModuleFileSharedCore::ModuleFileSharedCore( Bits.IsAllowModuleWithCompilerErrorsEnabled = extInfo.isAllowModuleWithCompilerErrorsEnabled(); Bits.IsConcurrencyChecked = extInfo.isConcurrencyChecked(); + Bits.HasCxxInteroperability = extInfo.hasCxxInteroperability(); MiscVersion = info.miscVersion; ModuleABIName = extInfo.getModuleABIName(); ModulePackageName = extInfo.getModulePackageName(); diff --git a/lib/Serialization/ModuleFileSharedCore.h b/lib/Serialization/ModuleFileSharedCore.h index 9ffdd6138a25a..13824c66a3407 100644 --- a/lib/Serialization/ModuleFileSharedCore.h +++ b/lib/Serialization/ModuleFileSharedCore.h @@ -386,8 +386,11 @@ class ModuleFileSharedCore { /// \c true if this module was built with complete checking for concurrency. unsigned IsConcurrencyChecked: 1; + /// Whether this module is built with C++ interoperability enabled. + unsigned HasCxxInteroperability : 1; + // Explicitly pad out to the next word boundary. - unsigned : 4; + unsigned : 3; } Bits = {}; static_assert(sizeof(ModuleBits) <= 8, "The bit set should be small"); diff --git a/lib/Serialization/ModuleFormat.h b/lib/Serialization/ModuleFormat.h index e2f871b82df3f..885790790ccd7 100644 --- a/lib/Serialization/ModuleFormat.h +++ b/lib/Serialization/ModuleFormat.h @@ -58,7 +58,7 @@ const uint16_t SWIFTMODULE_VERSION_MAJOR = 0; /// describe what change you made. The content of this comment isn't important; /// it just ensures a conflict if two people change the module format. /// Don't worry about adhering to the 80-column limit for this line. -const uint16_t SWIFTMODULE_VERSION_MINOR = 790; // add `out` kind to mark uninitialized instruction +const uint16_t SWIFTMODULE_VERSION_MINOR = 791; // HasCxxInteroperability /// A standard hash seed used for all string hashes in a serialized module. /// @@ -888,6 +888,7 @@ namespace options_block { EXTERNAL_SEARCH_PLUGIN_PATH, COMPILER_PLUGIN_LIBRARY_PATH, COMPILER_PLUGIN_EXECUTABLE_PATH, + HAS_CXX_INTEROPERABILITY_ENABLED, }; using SDKPathLayout = BCRecordLayout< @@ -976,6 +977,10 @@ namespace options_block { MODULE_EXPORT_AS_NAME, BCBlob >; + + using HasCxxInteroperabilityEnabledLayout = BCRecordLayout< + HAS_CXX_INTEROPERABILITY_ENABLED + >; } /// The record types within the input block. diff --git a/lib/Serialization/Serialization.cpp b/lib/Serialization/Serialization.cpp index 02bc669075a96..8f1758049b6cb 100644 --- a/lib/Serialization/Serialization.cpp +++ b/lib/Serialization/Serialization.cpp @@ -843,6 +843,7 @@ void Serializer::writeBlockInfoBlock() { BLOCK_RECORD(options_block, IS_ALLOW_MODULE_WITH_COMPILER_ERRORS_ENABLED); BLOCK_RECORD(options_block, MODULE_ABI_NAME); BLOCK_RECORD(options_block, IS_CONCURRENCY_CHECKED); + BLOCK_RECORD(options_block, HAS_CXX_INTEROPERABILITY_ENABLED); BLOCK_RECORD(options_block, MODULE_PACKAGE_NAME); BLOCK_RECORD(options_block, MODULE_EXPORT_AS_NAME); BLOCK_RECORD(options_block, PLUGIN_SEARCH_PATH); @@ -1090,6 +1091,12 @@ void Serializer::writeHeader(const SerializationOptions &options) { IsConcurrencyChecked.emit(ScratchRecord); } + if (M->hasCxxInteroperability()) { + options_block::HasCxxInteroperabilityEnabledLayout + CxxInteroperabilityEnabled(Out); + CxxInteroperabilityEnabled.emit(ScratchRecord); + } + if (options.SerializeOptionsForDebugging) { options_block::SDKPathLayout SDKPath(Out); options_block::XCCLayout XCC(Out); diff --git a/lib/Serialization/SerializedModuleLoader.cpp b/lib/Serialization/SerializedModuleLoader.cpp index 03984e311e507..6b952b96295ae 100644 --- a/lib/Serialization/SerializedModuleLoader.cpp +++ b/lib/Serialization/SerializedModuleLoader.cpp @@ -824,6 +824,8 @@ LoadedFile *SerializedModuleLoaderBase::loadAST( M.setABIName(Ctx.getIdentifier(loadedModuleFile->getModuleABIName())); if (loadedModuleFile->isConcurrencyChecked()) M.setIsConcurrencyChecked(); + if (loadedModuleFile->hasCxxInteroperability()) + M.setHasCxxInteroperability(); if (!loadedModuleFile->getModulePackageName().empty()) { M.setPackageName(Ctx.getIdentifier(loadedModuleFile->getModulePackageName())); } @@ -896,6 +898,19 @@ LoadedFile *SerializedModuleLoaderBase::loadAST( diag::need_hermetic_seal_to_import_module, M.getName()); } + // Non-resilient modules built with C++ interoperability enabled + // are typically incompatible with clients that do not enable + // C++ interoperability. + if (M.hasCxxInteroperability() && + M.getResilienceStrategy() != ResilienceStrategy::Resilient && + !Ctx.LangOpts.EnableCXXInterop && + Ctx.LangOpts.RequireCxxInteropToImportCxxInteropModule) { + auto loc = diagLoc.value_or(SourceLoc()); + Ctx.Diags.diagnose(loc, diag::need_cxx_interop_to_import_module, + M.getName()); + Ctx.Diags.diagnose(loc, diag::enable_cxx_interop_docs); + } + return fileUnit; } diff --git a/test/Interop/Cxx/implementation-only-imports/import-implementation-only-cxx-interop-module-without-interop.swift b/test/Interop/Cxx/implementation-only-imports/import-implementation-only-cxx-interop-module-without-interop.swift index ecb08c10d0641..1f8a69da51a32 100644 --- a/test/Interop/Cxx/implementation-only-imports/import-implementation-only-cxx-interop-module-without-interop.swift +++ b/test/Interop/Cxx/implementation-only-imports/import-implementation-only-cxx-interop-module-without-interop.swift @@ -1,11 +1,11 @@ // RUN: %empty-directory(%t) -// RUN: %target-swift-frontend %S/Inputs/use-module-a-impl-only.swift -I %S/Inputs/ -module-name UseModuleAImplOnly -emit-module -emit-module-path %t/UseModuleAImplOnly.swiftmodule -cxx-interoperability-mode=default +// RUN: %target-swift-frontend %S/Inputs/use-module-a-impl-only.swift -I %S/Inputs/ -module-name UseModuleAImplOnly -emit-module -emit-module-path %t/UseModuleAImplOnly.swiftmodule -cxx-interoperability-mode=default -enable-library-evolution // RUN: %target-swift-frontend %s -typecheck -module-name TestMod -I %t -I %S/Inputs // Check that we have used something from CxxShim in 'UseModuleAImplOnly' -// RUN: %target-swift-frontend %S/Inputs/use-module-a-impl-only.swift -I %S/Inputs/ -module-name UseModuleAImplOnly -emit-module -emit-module-path %t/UseModuleAImplOnly.swiftmodule -cxx-interoperability-mode=default -emit-sil -o - | %FileCheck %s +// RUN: %target-swift-frontend %S/Inputs/use-module-a-impl-only.swift -I %S/Inputs/ -module-name UseModuleAImplOnly -emit-module -emit-module-path %t/UseModuleAImplOnly.swiftmodule -cxx-interoperability-mode=default -emit-sil -o - -enable-library-evolution | %FileCheck %s // CHECK: __swift_interopStaticCast import UseModuleAImplOnly diff --git a/test/Interop/Cxx/implementation-only-imports/import-implementation-only-cxxstdlib-interop-module-without-interop.swift b/test/Interop/Cxx/implementation-only-imports/import-implementation-only-cxxstdlib-interop-module-without-interop.swift index 430fce25bbbf7..f020d6c941dec 100644 --- a/test/Interop/Cxx/implementation-only-imports/import-implementation-only-cxxstdlib-interop-module-without-interop.swift +++ b/test/Interop/Cxx/implementation-only-imports/import-implementation-only-cxxstdlib-interop-module-without-interop.swift @@ -1,6 +1,6 @@ // RUN: %empty-directory(%t) -// RUN: %target-swift-frontend %S/Inputs/use-cxx-stdlib-impl-only.swift -I %S/Inputs/ -module-name UseCxxStdlibImplOnly -emit-module -emit-module-path %t/UseCxxStdlibImplOnly.swiftmodule -cxx-interoperability-mode=default +// RUN: %target-swift-frontend %S/Inputs/use-cxx-stdlib-impl-only.swift -I %S/Inputs/ -module-name UseCxxStdlibImplOnly -emit-module -emit-module-path %t/UseCxxStdlibImplOnly.swiftmodule -cxx-interoperability-mode=default -enable-library-evolution // RUN: %target-swift-frontend %s -typecheck -module-name TestMod -I %t -I %S/Inputs diff --git a/test/Interop/Cxx/modules/prohibit-import-into-non-interop-compilation.swift b/test/Interop/Cxx/modules/prohibit-import-into-non-interop-compilation.swift new file mode 100644 index 0000000000000..ce36822467f41 --- /dev/null +++ b/test/Interop/Cxx/modules/prohibit-import-into-non-interop-compilation.swift @@ -0,0 +1,21 @@ +// RUN: %empty-directory(%t) + +// RUN: %target-swift-frontend %s -module-name UsesCxxInterop -emit-module -emit-module-path %t/UsesCxxInterop.swiftmodule -cxx-interoperability-mode=default + +// RUN: %target-swift-frontend %s -D USE -typecheck -module-name TestMod -I %t -verify + +// RUN: %target-swift-frontend %s -D USE -typecheck -module-name TestMod -I %t -disable-cxx-interop-requirement-at-import + +// Verify that 'disable-cxx-interop-requirement-at-import' works for the module being built. +// RUN: %empty-directory(%t) +// RUN: %target-swift-frontend %s -module-name UsesCxxInterop -emit-module -emit-module-path %t/UsesCxxInterop.swiftmodule -cxx-interoperability-mode=default -disable-cxx-interop-requirement-at-import +// RUN: %target-swift-frontend %s -D USE -typecheck -module-name TestMod -I %t + +#if USE +import UsesCxxInterop // expected-error {{module 'UsesCxxInterop' was built with C++ interoperability enabled, but current compilation does not enable C++ interoperability}} +// expected-note@-1 {{visit https://www.swift.org/documentation/cxx-interop/project-build-setup to learn how to enable C++ interoperability}} + +#else +public func testFun() { } + +#endif From e7590072569f267bb4885657e84c591d69505899 Mon Sep 17 00:00:00 2001 From: Ben Langmuir Date: Fri, 9 Jun 2023 16:33:51 -0700 Subject: [PATCH 04/10] [Immediate] Workaround for loading merged frameworks in immediate mode Some frameworks that previously had a separate swift overlay have been merged in macOS 14. This is causing symbols to not resolve if using the new SDK but running on an older OS in immediate mode. Ideally we would handle this automatically in JITLink as is done by the system linker, but in the meantime add a workaround to load the correct libraries manually. rdar://110371405 --- lib/Immediate/Immediate.cpp | 63 +++++++++++++++++++++++++++---------- 1 file changed, 46 insertions(+), 17 deletions(-) diff --git a/lib/Immediate/Immediate.cpp b/lib/Immediate/Immediate.cpp index 5e20fe1485b25..e2d9f2d089b36 100644 --- a/lib/Immediate/Immediate.cpp +++ b/lib/Immediate/Immediate.cpp @@ -191,6 +191,50 @@ bool swift::immediate::tryLoadLibraries(ArrayRef LinkLibraries, [](bool Value) { return Value; }); } +/// Workaround for rdar://94645534. +/// +/// The framework layout of some frameworks have changed over time, causing +/// unresolved symbol errors in immediate mode when running on older OS versions +/// with a newer SDK. This workaround scans through the list of dependencies and +/// manually adds the right libraries as necessary. +/// +/// FIXME: JITLink should emulate the Darwin linker's handling of ld$previous +/// mappings so this is handled automatically. +static void addMergedLibraries(SmallVectorImpl &AllLinkLibraries, + const llvm::Triple &Target) { + assert(Target.isMacOSX()); + + struct MergedLibrary { + StringRef OldLibrary; + llvm::VersionTuple MovedIn; + }; + + using VersionTuple = llvm::VersionTuple; + + static const llvm::StringMap MergedLibs = { + // Merged in macOS 14.0 + {"AppKit", {"libswiftAppKit.dylib", VersionTuple{14}}}, + {"HealthKit", {"libswiftHealthKit.dylib", VersionTuple{14}}}, + {"Network", {"libswiftNetwork.dylib", VersionTuple{14}}}, + {"Photos", {"libswiftPhotos.dylib", VersionTuple{14}}}, + {"PhotosUI", {"libswiftPhotosUI.dylib", VersionTuple{14}}}, + {"SoundAnalysis", {"libswiftSoundAnalysis.dylib", VersionTuple{14}}}, + {"Virtualization", {"libswiftVirtualization.dylib", VersionTuple{14}}}, + // Merged in macOS 13.0 + {"Foundation", {"libswiftFoundation.dylib", VersionTuple{13}}}, + }; + + SmallVector NewLibs; + for (auto &Lib : AllLinkLibraries) { + auto I = MergedLibs.find(Lib.getName()); + if (I != MergedLibs.end() && Target.getOSVersion() < I->second.MovedIn) + NewLibs.push_back(I->second.OldLibrary); + } + + for (StringRef NewLib : NewLibs) + AllLinkLibraries.push_back(LinkLibrary(NewLib, LibraryKind::Library)); +} + bool swift::immediate::autolinkImportedModules(ModuleDecl *M, const IRGenOptions &IRGenOpts) { // Perform autolinking. @@ -201,24 +245,9 @@ bool swift::immediate::autolinkImportedModules(ModuleDecl *M, M->collectLinkLibraries(addLinkLibrary); - // Workaround for rdar://94645534. - // - // The framework layout of Foundation has changed in 13.0, causing unresolved symbol - // errors to libswiftFoundation in immediate mode when running on older OS versions - // with a 13.0 SDK. This workaround scans through the list of dependencies and - // manually adds libswiftFoundation if necessary. auto &Target = M->getASTContext().LangOpts.Target; - if (Target.isMacOSX() && Target.getOSMajorVersion() < 13) { - bool linksFoundation = std::any_of(AllLinkLibraries.begin(), - AllLinkLibraries.end(), [](auto &Lib) { - return Lib.getName() == "Foundation"; - }); - - if (linksFoundation) { - AllLinkLibraries.push_back(LinkLibrary("libswiftFoundation.dylib", - LibraryKind::Library)); - } - } + if (Target.isMacOSX()) + addMergedLibraries(AllLinkLibraries, Target); tryLoadLibraries(AllLinkLibraries, M->getASTContext().SearchPathOpts, M->getASTContext().Diags); From a72fb83034ce66e075720949ee6114c94dd96ff6 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Thu, 8 Jun 2023 10:15:32 -0700 Subject: [PATCH 05/10] Requestify `AbstractStorageDecl::hasStorage()`. The `hasStorage()` computation is used in many places to determine the signatures of other declarations. It currently needs to expand accessor macros, which causes a number of cyclic references. Provide a simplified request to determine `hasStorage` without expanding or resolving macros, breaking a common pattern of cycles when using macros. Fixes rdar://109668383. --- include/swift/AST/Decl.h | 9 +-- include/swift/AST/DiagnosticsSema.def | 4 ++ include/swift/AST/Evaluator.h | 6 ++ include/swift/AST/NameLookup.h | 7 ++ include/swift/AST/TypeCheckRequests.h | 20 ++++++ include/swift/AST/TypeCheckerTypeIDZone.def | 3 + lib/AST/Decl.cpp | 19 +++++ lib/AST/NameLookup.cpp | 2 +- lib/ClangImporter/ClangImporter.cpp | 2 + lib/ClangImporter/ImportDecl.cpp | 1 + .../DerivedConformanceEquatableHashable.cpp | 9 ++- lib/Sema/TypeCheckMacros.cpp | 49 ++++++++++++- lib/Sema/TypeCheckMacros.h | 5 ++ lib/Sema/TypeCheckStorage.cpp | 71 ++++++++++++++++++- lib/Serialization/Deserialization.cpp | 13 ++-- .../Sources/Observation/Observable.swift | 2 +- test/Macros/accessor_has_storage_cycle.swift | 21 ++++++ test/Serialization/macros.swift | 1 + 18 files changed, 222 insertions(+), 22 deletions(-) create mode 100644 test/Macros/accessor_has_storage_cycle.swift diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h index 792568c22d518..fd9b526d7c149 100644 --- a/include/swift/AST/Decl.h +++ b/include/swift/AST/Decl.h @@ -5293,10 +5293,7 @@ class AbstractStorageDecl : public ValueDecl { /// Overwrite the registered implementation-info. This should be /// used carefully. - void setImplInfo(StorageImplInfo implInfo) { - LazySemanticInfo.ImplInfoComputed = 1; - ImplInfo = implInfo; - } + void setImplInfo(StorageImplInfo implInfo); ReadImplKind getReadImpl() const { return getImplInfo().getReadImpl(); @@ -5311,9 +5308,7 @@ class AbstractStorageDecl : public ValueDecl { /// Return true if this is a VarDecl that has storage associated with /// it. - bool hasStorage() const { - return getImplInfo().hasStorage(); - } + bool hasStorage() const; /// Return true if this storage has the basic accessors/capability /// to be mutated. This is generally constant after the accessors are diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index d77f402f63d7b..a7f33d8472ca1 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -7181,6 +7181,10 @@ ERROR(invalid_macro_role_for_macro_syntax,none, (unsigned)) ERROR(macro_cannot_introduce_names,none, "'%0' macros are not allowed to introduce names", (StringRef)) +ERROR(macro_accessor_missing_from_expansion,none, + "expansion of macro %0 did not produce a %select{non-|}1observing " + "accessor", + (DeclName, bool)) ERROR(macro_resolve_circular_reference, none, "circular reference resolving %select{freestanding|attached}0 macro %1", diff --git a/include/swift/AST/Evaluator.h b/include/swift/AST/Evaluator.h index 70b89958befe5..7e42e727eb61f 100644 --- a/include/swift/AST/Evaluator.h +++ b/include/swift/AST/Evaluator.h @@ -316,6 +316,12 @@ class Evaluator { cache.insert(request, std::move(output)); } + template::type* = nullptr> + bool hasCachedResult(const Request &request) { + return cache.find_as(request) != cache.end(); + } + /// Do not introduce new callers of this function. template::type* = nullptr> diff --git a/include/swift/AST/NameLookup.h b/include/swift/AST/NameLookup.h index efc0ebc3014d8..a4b8a0a3c14f4 100644 --- a/include/swift/AST/NameLookup.h +++ b/include/swift/AST/NameLookup.h @@ -556,6 +556,13 @@ void forEachPotentialResolvedMacro( llvm::function_ref body ); +/// For each macro with the given role that might be attached to the given +/// declaration, call the body. +void forEachPotentialAttachedMacro( + Decl *decl, MacroRole role, + llvm::function_ref body +); + } // end namespace namelookup /// Describes an inherited nominal entry. diff --git a/include/swift/AST/TypeCheckRequests.h b/include/swift/AST/TypeCheckRequests.h index 1e1e9a515c36f..8574c38fa42f1 100644 --- a/include/swift/AST/TypeCheckRequests.h +++ b/include/swift/AST/TypeCheckRequests.h @@ -1686,6 +1686,26 @@ class InitAccessorPropertiesRequest : ArrayRef evaluate(Evaluator &evaluator, NominalTypeDecl *decl) const; + // Evaluation. + bool evaluate(Evaluator &evaluator, AbstractStorageDecl *decl) const; + +public: + bool isCached() const { return true; } +}; + +class HasStorageRequest : + public SimpleRequest { +public: + using SimpleRequest::SimpleRequest; + +private: + friend SimpleRequest; + + // Evaluation. + bool evaluate(Evaluator &evaluator, AbstractStorageDecl *decl) const; + public: bool isCached() const { return true; } }; diff --git a/include/swift/AST/TypeCheckerTypeIDZone.def b/include/swift/AST/TypeCheckerTypeIDZone.def index 581a7bd27d8b2..e13aced855628 100644 --- a/include/swift/AST/TypeCheckerTypeIDZone.def +++ b/include/swift/AST/TypeCheckerTypeIDZone.def @@ -302,6 +302,9 @@ SWIFT_REQUEST(TypeChecker, SelfAccessKindRequest, SelfAccessKind(FuncDecl *), SWIFT_REQUEST(TypeChecker, StorageImplInfoRequest, StorageImplInfo(AbstractStorageDecl *), SeparatelyCached, NoLocationInfo) +SWIFT_REQUEST(TypeChecker, HasStorageRequest, + bool(AbstractStorageDecl *), Cached, + NoLocationInfo) SWIFT_REQUEST(TypeChecker, StoredPropertiesAndMissingMembersRequest, ArrayRef(NominalTypeDecl *), Cached, NoLocationInfo) SWIFT_REQUEST(TypeChecker, StoredPropertiesRequest, diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index 47dd732090d67..1a350932ed48c 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -6393,6 +6393,13 @@ bool ProtocolDecl::hasCircularInheritedProtocols() const { ctx.evaluator, HasCircularInheritedProtocolsRequest{mutableThis}, true); } +bool AbstractStorageDecl::hasStorage() const { + ASTContext &ctx = getASTContext(); + return evaluateOrDefault(ctx.evaluator, + HasStorageRequest{const_cast(this)}, + false); +} + StorageImplInfo AbstractStorageDecl::getImplInfo() const { ASTContext &ctx = getASTContext(); return evaluateOrDefault(ctx.evaluator, @@ -6400,6 +6407,18 @@ StorageImplInfo AbstractStorageDecl::getImplInfo() const { StorageImplInfo::getSimpleStored(StorageIsMutable)); } +void AbstractStorageDecl::setImplInfo(StorageImplInfo implInfo) { + LazySemanticInfo.ImplInfoComputed = 1; + ImplInfo = implInfo; + + if (isImplicit()) { + auto &evaluator = getASTContext().evaluator; + HasStorageRequest request{this}; + if (!evaluator.hasCachedResult(request)) + evaluator.cacheOutput(request, implInfo.hasStorage()); + } +} + bool AbstractStorageDecl::hasPrivateAccessor() const { for (auto accessor : getAllAccessors()) { if (hasPrivateOrFilePrivateFormalAccess(accessor)) diff --git a/lib/AST/NameLookup.cpp b/lib/AST/NameLookup.cpp index ad9aaa935d034..77e029d8e3199 100644 --- a/lib/AST/NameLookup.cpp +++ b/lib/AST/NameLookup.cpp @@ -1627,7 +1627,7 @@ void namelookup::forEachPotentialResolvedMacro( /// For each macro with the given role that might be attached to the given /// declaration, call the body. -static void forEachPotentialAttachedMacro( +void namelookup::forEachPotentialAttachedMacro( Decl *decl, MacroRole role, llvm::function_ref body ) { diff --git a/lib/ClangImporter/ClangImporter.cpp b/lib/ClangImporter/ClangImporter.cpp index 8b9e4a2663b82..86ed63dbe3ba8 100644 --- a/lib/ClangImporter/ClangImporter.cpp +++ b/lib/ClangImporter/ClangImporter.cpp @@ -34,6 +34,7 @@ #include "swift/AST/NameLookupRequests.h" #include "swift/AST/PrettyStackTrace.h" #include "swift/AST/SourceFile.h" +#include "swift/AST/TypeCheckRequests.h" #include "swift/AST/Types.h" #include "swift/Basic/Defer.h" #include "swift/Basic/Platform.h" @@ -5056,6 +5057,7 @@ cloneBaseMemberDecl(ValueDecl *decl, DeclContext *newContext) { out->setIsObjC(var->isObjC()); out->setIsDynamic(var->isDynamic()); out->copyFormalAccessFrom(var); + out->getASTContext().evaluator.cacheOutput(HasStorageRequest{out}, false); out->setAccessors(SourceLoc(), makeBaseClassMemberAccessors(newContext, out, var), SourceLoc()); diff --git a/lib/ClangImporter/ImportDecl.cpp b/lib/ClangImporter/ImportDecl.cpp index c83e5877ad9de..d7d3527a6c7a0 100644 --- a/lib/ClangImporter/ImportDecl.cpp +++ b/lib/ClangImporter/ImportDecl.cpp @@ -126,6 +126,7 @@ void ClangImporter::Implementation::makeComputed(AbstractStorageDecl *storage, AccessorDecl *getter, AccessorDecl *setter) { assert(getter); + storage->getASTContext().evaluator.cacheOutput(HasStorageRequest{storage}, false); if (setter) { storage->setImplInfo(StorageImplInfo::getMutableComputed()); storage->setAccessors(SourceLoc(), {getter, setter}, SourceLoc()); diff --git a/lib/Sema/DerivedConformanceEquatableHashable.cpp b/lib/Sema/DerivedConformanceEquatableHashable.cpp index 85c818a066782..f28b7823b2442 100644 --- a/lib/Sema/DerivedConformanceEquatableHashable.cpp +++ b/lib/Sema/DerivedConformanceEquatableHashable.cpp @@ -886,6 +886,10 @@ static ValueDecl *deriveHashable_hashValue(DerivedConformance &derived) { SourceLoc(), C.Id_hashValue, parentDC); hashValueDecl->setInterfaceType(intType); hashValueDecl->setSynthesized(); + hashValueDecl->setImplicit(); + hashValueDecl->setImplInfo(StorageImplInfo::getImmutableComputed()); + hashValueDecl->copyFormalAccessFrom(derived.Nominal, + /*sourceIsParentContext*/ true); ParameterList *params = ParameterList::createEmpty(C); @@ -904,12 +908,7 @@ static ValueDecl *deriveHashable_hashValue(DerivedConformance &derived) { /*sourceIsParentContext*/ true); // Finish creating the property. - hashValueDecl->setImplicit(); - hashValueDecl->setInterfaceType(intType); - hashValueDecl->setImplInfo(StorageImplInfo::getImmutableComputed()); hashValueDecl->setAccessors(SourceLoc(), {getterDecl}, SourceLoc()); - hashValueDecl->copyFormalAccessFrom(derived.Nominal, - /*sourceIsParentContext*/ true); // The derived hashValue of an actor must be nonisolated. if (!addNonIsolatedToSynthesized(derived.Nominal, hashValueDecl) && diff --git a/lib/Sema/TypeCheckMacros.cpp b/lib/Sema/TypeCheckMacros.cpp index b6a6dacaa5d97..16e8d2e6d9920 100644 --- a/lib/Sema/TypeCheckMacros.cpp +++ b/lib/Sema/TypeCheckMacros.cpp @@ -1240,6 +1240,37 @@ static SourceFile *evaluateAttachedMacro(MacroDecl *macro, Decl *attachedTo, return macroSourceFile; } +bool swift::accessorMacroOnlyIntroducesObservers( + MacroDecl *macro, + const MacroRoleAttr *attr +) { + // Will this macro introduce observers? + bool foundObserver = false; + for (auto name : attr->getNames()) { + if (name.getKind() == MacroIntroducedDeclNameKind::Named && + (name.getName().getBaseName().userFacingName() == "willSet" || + name.getName().getBaseName().userFacingName() == "didSet")) { + foundObserver = true; + } else { + // Introduces something other than an observer. + return false; + } + } + + if (foundObserver) + return true; + + // WORKAROUND: Older versions of the Observation library make + // `ObservationIgnored` an accessor macro that implies that it makes a + // stored property computed. Override that, because we know it produces + // nothing. + if (macro->getName().getBaseName().userFacingName() == "ObservationIgnored") { + return true; + } + + return false; +} + Optional swift::expandAccessors( AbstractStorageDecl *storage, CustomAttr *attr, MacroDecl *macro ) { @@ -1251,11 +1282,12 @@ Optional swift::expandAccessors( return None; PrettyStackTraceDecl debugStack( - "type checking expanded declaration macro", storage); + "type checking expanded accessor macro", storage); // Trigger parsing of the sequence of accessor declarations. This has the // side effect of registering those accessor declarations with the storage // declaration, so there is nothing further to do. + bool foundNonObservingAccessor = false; for (auto decl : macroSourceFile->getTopLevelItems()) { auto accessor = dyn_cast_or_null(decl.dyn_cast()); if (!accessor) @@ -1264,17 +1296,30 @@ Optional swift::expandAccessors( if (accessor->isObservingAccessor()) continue; + foundNonObservingAccessor = true; + } + + auto roleAttr = macro->getMacroRoleAttr(MacroRole::Accessor); + bool expectedNonObservingAccessor = + !accessorMacroOnlyIntroducesObservers(macro, roleAttr); + if (foundNonObservingAccessor) { // If any non-observing accessor was added, mark the initializer as // subsumed. if (auto var = dyn_cast(storage)) { if (auto binding = var->getParentPatternBinding()) { unsigned index = binding->getPatternEntryIndexForVarDecl(var); binding->setInitializerSubsumed(index); - break; } } } + // Make sure we got non-observing accessors exactly where we expected to. + if (foundNonObservingAccessor != expectedNonObservingAccessor) { + storage->diagnose( + diag::macro_accessor_missing_from_expansion, macro->getName(), + !expectedNonObservingAccessor); + } + return macroSourceFile->getBufferID(); } diff --git a/lib/Sema/TypeCheckMacros.h b/lib/Sema/TypeCheckMacros.h index 4fe1bce6fa780..6378b16c25ad2 100644 --- a/lib/Sema/TypeCheckMacros.h +++ b/lib/Sema/TypeCheckMacros.h @@ -77,6 +77,11 @@ Optional expandPeers(CustomAttr *attr, MacroDecl *macro, Decl *decl); Optional expandConformances(CustomAttr *attr, MacroDecl *macro, NominalTypeDecl *nominal); +/// Determine whether an accessor macro with the given attribute only +/// introduces observers like willSet and didSet. +bool accessorMacroOnlyIntroducesObservers( + MacroDecl *macro, const MacroRoleAttr *attr); + } // end namespace swift #endif /* SWIFT_SEMA_TYPECHECKMACROS_H */ diff --git a/lib/Sema/TypeCheckStorage.cpp b/lib/Sema/TypeCheckStorage.cpp index 59e5aed5f5b63..a2d9c67b89d8d 100644 --- a/lib/Sema/TypeCheckStorage.cpp +++ b/lib/Sema/TypeCheckStorage.cpp @@ -1392,7 +1392,7 @@ synthesizeTrivialGetterBody(AccessorDecl *getter, TargetImpl target, /// underlying storage. static std::pair synthesizeTrivialGetterBody(AccessorDecl *getter, ASTContext &ctx) { - assert(getter->getStorage()->hasStorage()); + assert(getter->getStorage()->getImplInfo().hasStorage()); return synthesizeTrivialGetterBody(getter, TargetImpl::Storage, ctx); } @@ -3431,6 +3431,73 @@ static StorageImplInfo classifyWithHasStorageAttr(VarDecl *var) { return StorageImplInfo(ReadImplKind::Stored, writeImpl, readWriteImpl); } +bool HasStorageRequest::evaluate(Evaluator &evaluator, + AbstractStorageDecl *storage) const { + // Parameters are always stored. + if (isa(storage)) + return true; + + // Only variables can be stored. + auto *var = dyn_cast(storage); + if (!var) + return false; + + // @_hasStorage implies that it... has storage. + if (var->getAttrs().hasAttribute()) + return true; + + // Protocol requirements never have storage. + if (isa(storage->getDeclContext())) + return false; + + // lazy declarations do not have storage. + if (storage->getAttrs().hasAttribute()) + return false; + + // @NSManaged attributes don't have storage + if (storage->getAttrs().hasAttribute()) + return false; + + // Any accessors that read or write imply that there is no storage. + if (storage->getParsedAccessor(AccessorKind::Get) || + storage->getParsedAccessor(AccessorKind::Read) || + storage->getParsedAccessor(AccessorKind::Address) || + storage->getParsedAccessor(AccessorKind::Set) || + storage->getParsedAccessor(AccessorKind::Modify) || + storage->getParsedAccessor(AccessorKind::MutableAddress)) + return false; + + // willSet or didSet in an overriding property imply that there is no storage. + if ((storage->getParsedAccessor(AccessorKind::WillSet) || + storage->getParsedAccessor(AccessorKind::DidSet)) && + storage->getAttrs().hasAttribute()) + return false; + + // The presence of a property wrapper implies that there is no storage. + if (var->hasAttachedPropertyWrapper()) + return false; + + // Look for any accessor macros that might make this property computed. + bool hasStorage = true; + namelookup::forEachPotentialAttachedMacro( + var, MacroRole::Accessor, + [&](MacroDecl *macro, const MacroRoleAttr *attr) { + // Will this macro introduce observers? + bool foundObserver = accessorMacroOnlyIntroducesObservers(macro, attr); + + // If it's not (just) introducing observers, it's making the property + // computed. + if (!foundObserver) + hasStorage = false; + + // If it will introduce observers, and there is an "override", + // the property doesn't have storage. + if (foundObserver && storage->getAttrs().hasAttribute()) + hasStorage = false; + }); + return hasStorage; +} + StorageImplInfo StorageImplInfoRequest::evaluate(Evaluator &evaluator, AbstractStorageDecl *storage) const { @@ -3590,6 +3657,8 @@ StorageImplInfoRequest::evaluate(Evaluator &evaluator, StorageImplInfo info(readImpl, writeImpl, readWriteImpl); finishStorageImplInfo(storage, info); + assert(info.hasStorage() == storage->hasStorage() || + storage->getASTContext().Diags.hadAnyError()); return info; } diff --git a/lib/Serialization/Deserialization.cpp b/lib/Serialization/Deserialization.cpp index 620012cd55556..21f3f0bcc34a4 100644 --- a/lib/Serialization/Deserialization.cpp +++ b/lib/Serialization/Deserialization.cpp @@ -3026,6 +3026,11 @@ void ModuleFile::configureStorage(AbstractStorageDecl *decl, auto readWriteImpl = getActualReadWriteImplKind(rawReadWriteImplKind); if (!readWriteImpl) return; + auto implInfo = StorageImplInfo(*readImpl, *writeImpl, *readWriteImpl); + decl->setImplInfo(implInfo); + + decl->getASTContext().evaluator.cacheOutput(HasStorageRequest{decl}, implInfo.hasStorage()); + SmallVector accessors; for (DeclID id : rawIDs.IDs) { auto accessor = dyn_cast_or_null(getDecl(id)); @@ -3033,9 +3038,6 @@ void ModuleFile::configureStorage(AbstractStorageDecl *decl, accessors.push_back(accessor); } - auto implInfo = StorageImplInfo(*readImpl, *writeImpl, *readWriteImpl); - decl->setImplInfo(implInfo); - if (implInfo.isSimpleStored() && accessors.empty()) return; @@ -3652,6 +3654,9 @@ class DeclDeserializer { var->setIsSetterMutating(isSetterMutating); declOrOffset = var; + MF.configureStorage(var, opaqueReadOwnership, + readImpl, writeImpl, readWriteImpl, accessors); + auto interfaceTypeOrError = MF.getTypeChecked(interfaceTypeID); if (!interfaceTypeOrError) return interfaceTypeOrError.takeError(); @@ -3663,8 +3668,6 @@ class DeclDeserializer { AddAttribute( new (ctx) ReferenceOwnershipAttr(referenceStorage->getOwnership())); - MF.configureStorage(var, opaqueReadOwnership, - readImpl, writeImpl, readWriteImpl, accessors); auto accessLevel = getActualAccessLevel(rawAccessLevel); if (!accessLevel) return MF.diagnoseFatal(); diff --git a/stdlib/public/Observation/Sources/Observation/Observable.swift b/stdlib/public/Observation/Sources/Observation/Observable.swift index a2f837af85db3..cd4f4710b2461 100644 --- a/stdlib/public/Observation/Sources/Observation/Observable.swift +++ b/stdlib/public/Observation/Sources/Observation/Observable.swift @@ -35,7 +35,7 @@ public macro ObservationTracked() = #externalMacro(module: "ObservationMacros", type: "ObservationTrackedMacro") @available(SwiftStdlib 5.9, *) -@attached(accessor) +@attached(accessor, names: named(willSet)) public macro ObservationIgnored() = #externalMacro(module: "ObservationMacros", type: "ObservationIgnoredMacro") diff --git a/test/Macros/accessor_has_storage_cycle.swift b/test/Macros/accessor_has_storage_cycle.swift new file mode 100644 index 0000000000000..49fb2076adfa4 --- /dev/null +++ b/test/Macros/accessor_has_storage_cycle.swift @@ -0,0 +1,21 @@ +// REQUIRES: swift_swift_parser + +// RUN: %target-typecheck-verify-swift -swift-version 5 -swift-version 5 + +struct Predicate { } + +@freestanding(expression) +macro Predicate(_ body: (T) -> Void) -> Predicate = #externalMacro(module: "A", type: "B") +// expected-warning@-1{{could not be found}} + +@attached(accessor) +@attached(peer) +macro Foo(filter: Predicate) = #externalMacro(module: "A", type: "B") +// expected-warning@-1{{could not be found}} +// expected-note@-2 2{{declared here}} + +struct Content { + @Foo(filter: #Predicate { $0 == true }) var foo: Bool = true + //expected-error@-1 2{{could not be found for macro}} + // expected-warning@-2{{result of operator '==' is unused}} +} diff --git a/test/Serialization/macros.swift b/test/Serialization/macros.swift index fa7da055d3509..4933762698d3f 100644 --- a/test/Serialization/macros.swift +++ b/test/Serialization/macros.swift @@ -21,6 +21,7 @@ func test(a: Int, b: Int) { struct TestStruct { @myWrapper var x: Int + // expected-error@-1{{expansion of macro 'myWrapper' did not produce a non-observing accessor}} } @ArbitraryMembers From 4cb720e98fece0e3cc178ed6e55e50131b50204e Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Fri, 9 Jun 2023 22:26:19 -0700 Subject: [PATCH 06/10] Separate the "set for caching" and "set programmatically" for storage impl info Slightly cleanup, and make an assertion less strict in the face of invalid code. --- include/swift/AST/Decl.h | 3 +++ lib/AST/Decl.cpp | 12 ++++++++++-- lib/AST/TypeCheckRequests.cpp | 2 +- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h index fd9b526d7c149..b6b2d8396a46f 100644 --- a/include/swift/AST/Decl.h +++ b/include/swift/AST/Decl.h @@ -5295,6 +5295,9 @@ class AbstractStorageDecl : public ValueDecl { /// used carefully. void setImplInfo(StorageImplInfo implInfo); + /// Cache the implementation-info, for use by the request-evaluator. + void cacheImplInfo(StorageImplInfo implInfo); + ReadImplKind getReadImpl() const { return getImplInfo().getReadImpl(); } diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index 1a350932ed48c..14d6a9c2849fe 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -2525,7 +2525,7 @@ AbstractStorageDecl::getAccessStrategy(AccessSemantics semantics, ResilienceExpansion expansion) const { switch (semantics) { case AccessSemantics::DirectToStorage: - assert(hasStorage()); + assert(hasStorage() || getASTContext().Diags.hadAnyError()); return AccessStrategy::getStorage(); case AccessSemantics::DistributedThunk: @@ -6407,15 +6407,23 @@ StorageImplInfo AbstractStorageDecl::getImplInfo() const { StorageImplInfo::getSimpleStored(StorageIsMutable)); } -void AbstractStorageDecl::setImplInfo(StorageImplInfo implInfo) { +void AbstractStorageDecl::cacheImplInfo(StorageImplInfo implInfo) { LazySemanticInfo.ImplInfoComputed = 1; ImplInfo = implInfo; +} + +void AbstractStorageDecl::setImplInfo(StorageImplInfo implInfo) { + cacheImplInfo(implInfo); if (isImplicit()) { auto &evaluator = getASTContext().evaluator; HasStorageRequest request{this}; if (!evaluator.hasCachedResult(request)) evaluator.cacheOutput(request, implInfo.hasStorage()); + else { + assert( + evaluateOrDefault(evaluator, request, false) == implInfo.hasStorage()); + } } } diff --git a/lib/AST/TypeCheckRequests.cpp b/lib/AST/TypeCheckRequests.cpp index 0b7666c79bddf..e499f32abb3c0 100644 --- a/lib/AST/TypeCheckRequests.cpp +++ b/lib/AST/TypeCheckRequests.cpp @@ -687,7 +687,7 @@ StorageImplInfoRequest::getCachedResult() const { void StorageImplInfoRequest::cacheResult(StorageImplInfo value) const { auto *storage = std::get<0>(getStorage()); - storage->setImplInfo(value); + storage->cacheImplInfo(value); } //----------------------------------------------------------------------------// From 1209ef89ec89eb3cb99eb3295bd1a8d349dcb264 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Sun, 11 Jun 2023 08:48:43 -0700 Subject: [PATCH 07/10] Ensure that macros within init accessors are expanded early enough Now that we've made accessor macro expansion more lazy, ensure that when querying for init accessors (e.g., to build a memberwise initializer), we also expand any accessor macros that might produce an init accessor. This is a partial step toward the real goal, which is that `AbstractStorageDecl::getAccessor()` should lazily expand macros if needed. Update the Observable macro to document that it produces an `init` accessor. --- include/swift/AST/DiagnosticsSema.def | 3 + lib/Sema/TypeCheckMacros.cpp | 29 +++++++- lib/Sema/TypeCheckMacros.h | 5 ++ lib/Sema/TypeCheckStorage.cpp | 67 ++++++++++++++++--- .../Sources/Observation/Observable.swift | 4 +- test/ModuleInterface/Observable.swift | 2 +- test/stdlib/Observation/Observable.swift | 20 +++--- 7 files changed, 106 insertions(+), 24 deletions(-) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index a7f33d8472ca1..97e2fb51b35a8 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -7185,6 +7185,9 @@ ERROR(macro_accessor_missing_from_expansion,none, "expansion of macro %0 did not produce a %select{non-|}1observing " "accessor", (DeclName, bool)) +ERROR(macro_init_accessor_not_documented,none, + "expansion of macro %0 produced an unexpected 'init' accessor", + (DeclName)) ERROR(macro_resolve_circular_reference, none, "circular reference resolving %select{freestanding|attached}0 macro %1", diff --git a/lib/Sema/TypeCheckMacros.cpp b/lib/Sema/TypeCheckMacros.cpp index 16e8d2e6d9920..90e00f9fb0c78 100644 --- a/lib/Sema/TypeCheckMacros.cpp +++ b/lib/Sema/TypeCheckMacros.cpp @@ -1271,6 +1271,19 @@ bool swift::accessorMacroOnlyIntroducesObservers( return false; } +bool swift::accessorMacroIntroducesInitAccessor( + MacroDecl *macro, const MacroRoleAttr *attr +) { + for (auto name : attr->getNames()) { + if (name.getKind() == MacroIntroducedDeclNameKind::Named && + (name.getName().getBaseName().getKind() == + DeclBaseName::Kind::Constructor)) + return true; + } + + return false; +} + Optional swift::expandAccessors( AbstractStorageDecl *storage, CustomAttr *attr, MacroDecl *macro ) { @@ -1288,15 +1301,17 @@ Optional swift::expandAccessors( // side effect of registering those accessor declarations with the storage // declaration, so there is nothing further to do. bool foundNonObservingAccessor = false; + bool foundInitAccessor = false; for (auto decl : macroSourceFile->getTopLevelItems()) { auto accessor = dyn_cast_or_null(decl.dyn_cast()); if (!accessor) continue; - if (accessor->isObservingAccessor()) - continue; + if (accessor->isInitAccessor()) + foundInitAccessor = true; - foundNonObservingAccessor = true; + if (!accessor->isObservingAccessor()) + foundNonObservingAccessor = true; } auto roleAttr = macro->getMacroRoleAttr(MacroRole::Accessor); @@ -1320,6 +1335,14 @@ Optional swift::expandAccessors( !expectedNonObservingAccessor); } + // 'init' accessors must be documented in the macro role attribute. + if (foundInitAccessor && + !accessorMacroIntroducesInitAccessor(macro, roleAttr)) { + storage->diagnose( + diag::macro_init_accessor_not_documented, macro->getName()); + // FIXME: Add the appropriate "names: named(init)". + } + return macroSourceFile->getBufferID(); } diff --git a/lib/Sema/TypeCheckMacros.h b/lib/Sema/TypeCheckMacros.h index 6378b16c25ad2..b960baac4010c 100644 --- a/lib/Sema/TypeCheckMacros.h +++ b/lib/Sema/TypeCheckMacros.h @@ -82,6 +82,11 @@ Optional expandConformances(CustomAttr *attr, MacroDecl *macro, bool accessorMacroOnlyIntroducesObservers( MacroDecl *macro, const MacroRoleAttr *attr); +/// Determine whether an accessor macro (defined with the given role attribute) +/// introduces an init accessor. +bool accessorMacroIntroducesInitAccessor( + MacroDecl *macro, const MacroRoleAttr *attr); + } // end namespace swift #endif /* SWIFT_SEMA_TYPECHECKMACROS_H */ diff --git a/lib/Sema/TypeCheckStorage.cpp b/lib/Sema/TypeCheckStorage.cpp index a2d9c67b89d8d..3474f5c91a0ce 100644 --- a/lib/Sema/TypeCheckStorage.cpp +++ b/lib/Sema/TypeCheckStorage.cpp @@ -106,8 +106,16 @@ static bool hasStoredProperties(NominalTypeDecl *decl, || (decl != implDecl)))); } -static void computeLoweredStoredProperties(NominalTypeDecl *decl, - IterableDeclContext *implDecl) { +namespace { + enum class LoweredPropertiesReason { + Stored, + Memberwise + }; +} + +static void computeLoweredProperties(NominalTypeDecl *decl, + IterableDeclContext *implDecl, + LoweredPropertiesReason reason) { // Expand synthesized member macros. auto &ctx = decl->getASTContext(); (void)evaluateOrDefault(ctx.evaluator, @@ -127,15 +135,20 @@ static void computeLoweredStoredProperties(NominalTypeDecl *decl, if (!var || var->isStatic()) continue; - if (var->getAttrs().hasAttribute()) - (void) var->getLazyStorageProperty(); + if (reason == LoweredPropertiesReason::Stored) { + if (var->getAttrs().hasAttribute()) + (void) var->getLazyStorageProperty(); - if (var->hasAttachedPropertyWrapper()) { - (void) var->getPropertyWrapperAuxiliaryVariables(); - (void) var->getPropertyWrapperInitializerInfo(); + if (var->hasAttachedPropertyWrapper()) { + (void) var->getPropertyWrapperAuxiliaryVariables(); + (void) var->getPropertyWrapperInitializerInfo(); + } } } + if (reason != LoweredPropertiesReason::Stored) + return; + // If this is an actor, check conformance to the Actor protocol to // ensure that the actor storage will get created (if needed). if (auto classDecl = dyn_cast(decl)) { @@ -164,6 +177,11 @@ static void computeLoweredStoredProperties(NominalTypeDecl *decl, } } +static void computeLoweredStoredProperties(NominalTypeDecl *decl, + IterableDeclContext *implDecl) { + computeLoweredProperties(decl, implDecl, LoweredPropertiesReason::Stored); +} + /// Enumerate both the stored properties and missing members, /// in a deterministic order. static void enumerateStoredPropertiesAndMissing( @@ -283,13 +301,46 @@ StoredPropertiesAndMissingMembersRequest::evaluate(Evaluator &evaluator, return decl->getASTContext().AllocateCopy(results); } +/// Determine whether the given variable has an init accessor. +static bool hasInitAccessor(VarDecl *var) { + if (var->getAccessor(AccessorKind::Init)) + return true; + + // Look to see whether it is possible that there is an init accessor. + bool hasInitAccessor = false; + namelookup::forEachPotentialAttachedMacro( + var, MacroRole::Accessor, + [&](MacroDecl *macro, const MacroRoleAttr *attr) { + if (accessorMacroIntroducesInitAccessor(macro, attr)) + hasInitAccessor = true; + }); + + // There is no chance for an init accessor, so we're done. + if (!hasInitAccessor) + return false; + + // We might get an init accessor by expanding accessor macros; do so now. + (void)evaluateOrDefault( + var->getASTContext().evaluator, ExpandAccessorMacros{var}, { }); + + return var->getAccessor(AccessorKind::Init); +} + ArrayRef InitAccessorPropertiesRequest::evaluate(Evaluator &evaluator, NominalTypeDecl *decl) const { + IterableDeclContext *implDecl = decl->getImplementationContext(); + + if (!hasStoredProperties(decl, implDecl)) + return ArrayRef(); + + // Make sure we expand what we need to to get all of the properties. + computeLoweredProperties(decl, implDecl, LoweredPropertiesReason::Memberwise); + SmallVector results; for (auto *member : decl->getMembers()) { auto *var = dyn_cast(member); - if (!var || !var->getAccessor(AccessorKind::Init)) { + if (!var || var->isStatic() || !hasInitAccessor(var)) { continue; } diff --git a/stdlib/public/Observation/Sources/Observation/Observable.swift b/stdlib/public/Observation/Sources/Observation/Observable.swift index cd4f4710b2461..cc7d1f5732c3b 100644 --- a/stdlib/public/Observation/Sources/Observation/Observable.swift +++ b/stdlib/public/Observation/Sources/Observation/Observable.swift @@ -23,11 +23,11 @@ #endif @attached(memberAttribute) @attached(conformance) -public macro Observable() = +public macro Observable() = #externalMacro(module: "ObservationMacros", type: "ObservableMacro") @available(SwiftStdlib 5.9, *) -@attached(accessor) +@attached(accessor, names: named(init), named(get), named(set)) #if OBSERVATION_SUPPORTS_PEER_MACROS @attached(peer, names: prefixed(_)) #endif diff --git a/test/ModuleInterface/Observable.swift b/test/ModuleInterface/Observable.swift index 7bcad03394c64..181a4325ef231 100644 --- a/test/ModuleInterface/Observable.swift +++ b/test/ModuleInterface/Observable.swift @@ -1,5 +1,5 @@ // RUN: %empty-directory(%t) -// RUN: %target-swift-emit-module-interface(%t/Library.swiftinterface) %s -module-name Library -plugin-path %swift-host-lib-dir/plugins -disable-availability-checking +// RUN: %target-swift-emit-module-interface(%t/Library.swiftinterface) %s -module-name Library -plugin-path %swift-host-lib-dir/plugins -disable-availability-checking -enable-experimental-feature InitAccessors // RUN: %target-swift-typecheck-module-from-interface(%t/Library.swiftinterface) -module-name Library -disable-availability-checking // RUN: %FileCheck %s < %t/Library.swiftinterface diff --git a/test/stdlib/Observation/Observable.swift b/test/stdlib/Observation/Observable.swift index 1fef61087f849..bf3a8e117439e 100644 --- a/test/stdlib/Observation/Observable.swift +++ b/test/stdlib/Observation/Observable.swift @@ -35,7 +35,7 @@ func validateMemberwiseInitializers() { @Observable struct DefiniteInitialization { var field: Int - + init(field: Int) { self.field = field } @@ -62,21 +62,21 @@ class ContainsIUO { } class NonObservable { - + } @Observable class InheritsFromNonObservable: NonObservable { - + } protocol NonObservableProtocol { - + } @Observable class ConformsToNonObservableProtocol: NonObservableProtocol { - + } struct NonObservableContainer { @@ -91,19 +91,19 @@ class ImplementsAccessAndMutation { var field = 3 let accessCalled: (PartialKeyPath) -> Void let withMutationCalled: (PartialKeyPath) -> Void - + init(accessCalled: @escaping (PartialKeyPath) -> Void, withMutationCalled: @escaping (PartialKeyPath) -> Void) { self.accessCalled = accessCalled self.withMutationCalled = withMutationCalled } - + internal func access( keyPath: KeyPath ) { accessCalled(keyPath) _$observationRegistrar.access(self, keyPath: keyPath) } - + internal func withMutation( keyPath: KeyPath, _ mutation: () throws -> T @@ -128,7 +128,7 @@ class Entity { class Person : Entity { var firstName = "" var lastName = "" - + var friends = [Person]() var fullName: String { firstName + " " + lastName } @@ -165,7 +165,7 @@ class HasIntermediaryConformance: Intermediary { } class CapturedState: @unchecked Sendable { var state: State - + init(state: State) { self.state = state } From 545729014306782988db92c17c78602c96c2c513 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Sun, 11 Jun 2023 08:52:37 -0700 Subject: [PATCH 08/10] Eliminate "may have storage" hack in `PatternBindingEntryRequest`. Remove an early iteration of cycle-breaking in `PatternBindingEntryRequest` that has been subsumed by the lazy computation of `AbstractStorageDecl::hasStorage()`. We can now directly use `hasStorage()` here. --- lib/Sema/TypeCheckStorage.cpp | 37 +---------------------------------- 1 file changed, 1 insertion(+), 36 deletions(-) diff --git a/lib/Sema/TypeCheckStorage.cpp b/lib/Sema/TypeCheckStorage.cpp index 3474f5c91a0ce..c448f4dd90103 100644 --- a/lib/Sema/TypeCheckStorage.cpp +++ b/lib/Sema/TypeCheckStorage.cpp @@ -350,41 +350,6 @@ InitAccessorPropertiesRequest::evaluate(Evaluator &evaluator, return decl->getASTContext().AllocateCopy(results); } -/// Check whether the pattern may have storage. -/// -/// This query is careful not to trigger accessor macro expansion, which -/// creates a cycle. It conservatively assumes that all accessor macros -/// produce computed properties, which is... incorrect. -/// -/// The query also avoids triggering a `StorageImplInfoRequest` for patterns -/// involved in a ProtocolDecl, because we know they can never contain storage. -/// For background, vars of noncopyable type have their OpaqueReadOwnership -/// determined by the type of the var decl, but that type hasn't always been -/// determined when this query is made. -static bool mayHaveStorage(Pattern *pattern) { - // Check whether there are any accessor macros, or it's a protocol member. - bool hasAccessorMacros = false; - bool inProtocolDecl = false; - pattern->forEachVariable([&](VarDecl *VD) { - if (isa(VD->getDeclContext())) - inProtocolDecl = true; - - VD->forEachAttachedMacro(MacroRole::Accessor, - [&](CustomAttr *customAttr, MacroDecl *macro) { - hasAccessorMacros = true; - }); - }); - - if (hasAccessorMacros) - return false; - - // protocol members can never contain storage; avoid triggering request. - if (inProtocolDecl) - return false; - - return pattern->hasStorage(); -} - /// Validate the \c entryNumber'th entry in \c binding. const PatternBindingEntry *PatternBindingEntryRequest::evaluate( Evaluator &eval, PatternBindingDecl *binding, unsigned entryNumber, @@ -485,7 +450,7 @@ const PatternBindingEntry *PatternBindingEntryRequest::evaluate( // default-initializable. If so, do it. if (!pbe.isInitialized() && binding->isDefaultInitializable(entryNumber) && - mayHaveStorage(pattern)) { + pattern->hasStorage()) { if (auto defaultInit = TypeChecker::buildDefaultInitializer(patternType)) { // If we got a default initializer, install it and re-type-check it // to make sure it is properly coerced to the pattern type. From ee443d8ddb1994acf581d485a195d96a80d0a9e9 Mon Sep 17 00:00:00 2001 From: Erik Eckstein Date: Mon, 12 Jun 2023 07:53:28 +0200 Subject: [PATCH 09/10] fix an assert-nonassert mismatch in SILModule Data structures must be layout compatible when built with and without asserts. Fixes a compiler crash when C++ sources are built without asserts because SwiftCompilerSources are built with asserts. rdar://110363377 --- include/swift/SIL/SILModule.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/include/swift/SIL/SILModule.h b/include/swift/SIL/SILModule.h index 3be26d4e5ba69..77a087b21a524 100644 --- a/include/swift/SIL/SILModule.h +++ b/include/swift/SIL/SILModule.h @@ -395,9 +395,7 @@ class SILModule { /// Action to be executed for serializing the SILModule. ActionCallback SerializeSILAction; -#ifndef NDEBUG BasicBlockNameMapType basicBlockNames; -#endif SILModule(llvm::PointerUnion context, Lowering::TypeConverter &TC, const SILOptions &Options, From e45c9df2a3e0de40961989b221873803a1342ebb Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Mon, 12 Jun 2023 09:10:02 -0600 Subject: [PATCH 10/10] always use the argNames constructor for macro DeclNames (#66497) rdar://110179186 --- lib/Serialization/Deserialization.cpp | 12 ++--- .../Refactoring/shorthand_shadow.swift | 2 +- .../DeclarationFragments/Full/Macros.swift | 44 +++++++++++++------ 3 files changed, 36 insertions(+), 22 deletions(-) diff --git a/lib/Serialization/Deserialization.cpp b/lib/Serialization/Deserialization.cpp index 21f3f0bcc34a4..03fde6bb8ccac 100644 --- a/lib/Serialization/Deserialization.cpp +++ b/lib/Serialization/Deserialization.cpp @@ -5027,14 +5027,10 @@ class DeclDeserializer { // Resolve the name ids. DeclName name; - if (numArgNames > 0) { - SmallVector argNames; - for (auto argNameID : argNameAndDependencyIDs.slice(0, numArgNames)) - argNames.push_back(MF.getIdentifier(argNameID)); - name = DeclName(ctx, baseName, argNames); - } else { - name = baseName; - } + SmallVector argNames; + for (auto argNameID : argNameAndDependencyIDs.slice(0, numArgNames)) + argNames.push_back(MF.getIdentifier(argNameID)); + name = DeclName(ctx, baseName, argNames); PrettySupplementalDeclNameTrace trace(name); argNameAndDependencyIDs = argNameAndDependencyIDs.slice(numArgNames); diff --git a/test/SourceKit/Refactoring/shorthand_shadow.swift b/test/SourceKit/Refactoring/shorthand_shadow.swift index 40d898b34315d..17e347b97a8ef 100644 --- a/test/SourceKit/Refactoring/shorthand_shadow.swift +++ b/test/SourceKit/Refactoring/shorthand_shadow.swift @@ -32,7 +32,7 @@ func renameBuiltinMacroWithoutHash() { // RUN: not %sourcekitd-test -req=find-local-rename-ranges -pos=%(line+1):9 %s -- %s 2>&1 | %FileCheck %s --check-prefix=BUILTIN _ = file } - // BUILTIN: error: cannot rename system symbol 'file' + // BUILTIN: error: cannot rename system symbol 'file()' } diff --git a/test/SymbolGraph/Symbols/Mixins/DeclarationFragments/Full/Macros.swift b/test/SymbolGraph/Symbols/Mixins/DeclarationFragments/Full/Macros.swift index b89d77256be6c..530f5052d5e79 100644 --- a/test/SymbolGraph/Symbols/Mixins/DeclarationFragments/Full/Macros.swift +++ b/test/SymbolGraph/Symbols/Mixins/DeclarationFragments/Full/Macros.swift @@ -6,14 +6,32 @@ // RUN: %target-swift-frontend -load-plugin-library %t/%target-library-name(MacroDefinition) %s -module-name Macros -emit-module -emit-module-path %t/Macros.swiftmodule -emit-symbol-graph -emit-symbol-graph-dir %t/ // RUN: %{python} -m json.tool %t/Macros.symbols.json %t/Macros.formatted.symbols.json -// Make sure that the `= #externalMacro(...)` doesn't show up in declaration fragments and in names fragments. +// Make sure that the `= #externalMacro(...)` doesn't show up in declaration fragments and in names +// fragments, and also that macros with no parameters get the `()` in their name. // RUN: %FileCheck %s --input-file %t/Macros.formatted.symbols.json // RUN: %FileCheck %s --input-file %t/Macros.formatted.symbols.json --check-prefix NAMES +// RUN: %FileCheck %s --input-file %t/Macros.formatted.symbols.json --check-prefix TITLE +// Now do the same thing with a `swift-symbolgraph-extract`-processed symbol graph. + +// RUN: %empty-directory(%t) +// RUN: %host-build-swift -swift-version 5 -emit-library -o %t/%target-library-name(MacroDefinition) -module-name=MacroDefinition %S/Inputs/stringify_macro.swift -g -no-toolchain-stdlib-rpath -swift-version 5 +// RUN: %target-swift-frontend -load-plugin-library %t/%target-library-name(MacroDefinition) %s -module-name Macros -emit-module -emit-module-path %t/Macros.swiftmodule +// RUN: %target-swift-symbolgraph-extract -module-name Macros -I %t -pretty-print -output-dir %t + +// RUN: %FileCheck %s --input-file %t/Macros.symbols.json +// RUN: %FileCheck %s --input-file %t/Macros.symbols.json --check-prefix NAMES +// RUN: %FileCheck %s --input-file %t/Macros.symbols.json --check-prefix TITLE @freestanding(expression) public macro stringify(_ value: T) -> (T, String) = #externalMacro(module: "MacroDefinition", type: "StringifyMacro") +@freestanding(expression) public macro stringifySeven() -> (Int, String) = #stringify(7) + +// Note that type parameters don't have reference USRs when loading from serialized AST, so the +// following declaration/names dumps are edited to handle both situations. + +// CHECK-LABEL: "precise": "s:6Macros9stringifyyx_SStxclufm", // CHECK: "declarationFragments": [ // CHECK-NEXT: { // CHECK-NEXT: "kind": "attribute", @@ -65,18 +83,16 @@ // CHECK-NEXT: }, // CHECK-NEXT: { // CHECK-NEXT: "kind": "typeIdentifier", -// CHECK-NEXT: "spelling": "T", -// CHECK-NEXT: "preciseIdentifier": "s:6Macros1TL_xmfp" -// CHECK-NEXT: }, +// CHECK-NEXT: "spelling": "T" +// CHECK: }, // CHECK-NEXT: { // CHECK-NEXT: "kind": "text", // CHECK-NEXT: "spelling": ") -> (" // CHECK-NEXT: }, // CHECK-NEXT: { // CHECK-NEXT: "kind": "typeIdentifier", -// CHECK-NEXT: "spelling": "T", -// CHECK-NEXT: "preciseIdentifier": "s:6Macros1TL_xmfp" -// CHECK-NEXT: }, +// CHECK-NEXT: "spelling": "T" +// CHECK: }, // CHECK-NEXT: { // CHECK-NEXT: "kind": "text", // CHECK-NEXT: "spelling": ", " @@ -92,6 +108,7 @@ // CHECK-NEXT: } // CHECK-NEXT: ], +// NAMES-LABEL: "precise": "s:6Macros9stringifyyx_SStxclufm", // NAMES: "names": { // NAMES-NEXT: "title": "stringify(_:)", // NAMES-NEXT: "subHeading": [ @@ -121,18 +138,16 @@ // NAMES-NEXT: }, // NAMES-NEXT: { // NAMES-NEXT: "kind": "typeIdentifier", -// NAMES-NEXT: "spelling": "T", -// NAMES-NEXT: "preciseIdentifier": "s:6Macros1TL_xmfp" -// NAMES-NEXT: }, +// NAMES-NEXT: "spelling": "T" +// NAMES: }, // NAMES-NEXT: { // NAMES-NEXT: "kind": "text", // NAMES-NEXT: "spelling": ") -> (" // NAMES-NEXT: }, // NAMES-NEXT: { // NAMES-NEXT: "kind": "typeIdentifier", -// NAMES-NEXT: "spelling": "T", -// NAMES-NEXT: "preciseIdentifier": "s:6Macros1TL_xmfp" -// NAMES-NEXT: }, +// NAMES-NEXT: "spelling": "T" +// NAMES: }, // NAMES-NEXT: { // NAMES-NEXT: "kind": "text", // NAMES-NEXT: "spelling": ", " @@ -148,3 +163,6 @@ // NAMES-NEXT: } // NAMES-NEXT: ] // NAMES-NEXT: }, + +// TITLE-LABEL: "precise": "s:6Macros14stringifySevenSi_SStycfm", +// TITLE: "title": "stringifySeven()",