From 3c51106fadc20be78cd3a39852064a505fcd62ae Mon Sep 17 00:00:00 2001 From: Tristan Labelle Date: Fri, 9 Jun 2023 16:41:05 -0400 Subject: [PATCH 01/22] Fix tests on Windows CI with vs2022 --- .../Dependencies/one-way-merge-module-fine.swift | 4 +++- .../prefer-local-module-to-sdk-framework.swift | 14 +++++++------- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/test/Driver/Dependencies/one-way-merge-module-fine.swift b/test/Driver/Dependencies/one-way-merge-module-fine.swift index 0dddf0e783adf..951ed09b9af5d 100644 --- a/test/Driver/Dependencies/one-way-merge-module-fine.swift +++ b/test/Driver/Dependencies/one-way-merge-module-fine.swift @@ -12,7 +12,9 @@ // CHECK-FIRST-DAG: Produced master.swiftmodule // swift-driver checks existence of all outputs -// RUN: touch -t 201401240006 %t/{main,other,master}.swift{module,doc,sourceinfo} +// RUN: touch -t 201401240006 %t/*.swiftmodule +// RUN: touch -t 201401240006 %t/*.swiftdoc +// RUN: touch -t 201401240006 %t/*.swiftsourceinfo // RUN: cd %t && %swiftc_driver -driver-use-frontend-path "%{python.unquoted};%S/Inputs/update-dependencies.py;%swift-dependency-tool" -output-file-map %t/output.json -incremental -driver-always-rebuild-dependents ./main.swift ./other.swift -emit-module-path %t/master.swiftmodule -module-name main -j1 -v 2>&1 | %FileCheck -check-prefix=CHECK-SECOND %s diff --git a/test/ModuleInterface/ModuleCache/prefer-local-module-to-sdk-framework.swift b/test/ModuleInterface/ModuleCache/prefer-local-module-to-sdk-framework.swift index 817263c769088..9c6e7a2cb59a7 100644 --- a/test/ModuleInterface/ModuleCache/prefer-local-module-to-sdk-framework.swift +++ b/test/ModuleInterface/ModuleCache/prefer-local-module-to-sdk-framework.swift @@ -1,26 +1,26 @@ -// RUN: %empty-directory(%t/BuildDir/Lib.framework/Modules/Lib.swiftmodule) -// RUN: %empty-directory(%t/SecondBuildDir/Lib.framework/Modules/Lib.swiftmodule) +// RUN: %empty-directory(%t/Build1/Lib.framework/Modules/Lib.swiftmodule) +// RUN: %empty-directory(%t/Build2/Lib.framework/Modules/Lib.swiftmodule) // RUN: %empty-directory(%t/ModuleCache) // RUN: echo 'public func showsUpInBothPlaces() {}' > %t/Lib.swift // 1. Create a .swiftinterface file containing just one API, and put it inside a second build dir (without a .swiftmodule) -// RUN: %target-swift-frontend -typecheck %t/Lib.swift -emit-module-interface-path %t/SecondBuildDir/Lib.framework/Modules/Lib.swiftmodule/%target-swiftinterface-name -module-name Lib +// RUN: %target-swift-frontend -typecheck %t/Lib.swift -emit-module-interface-path %t/Build2/Lib.framework/Modules/Lib.swiftmodule/%target-swiftinterface-name -module-name Lib // 2. Add a new API to the module, and compile just the serialized version in the build dir. // RUN: echo 'public func onlyInTheCompiledModule() {}' >> %t/Lib.swift -// RUN: %target-swift-frontend -emit-module %t/Lib.swift -o %t/BuildDir/Lib.framework/Modules/Lib.swiftmodule/%target-swiftmodule-name -emit-module-interface-path %t/BuildDir/Lib.framework/Modules/Lib.swiftmodule/%target-swiftinterface-name -module-name Lib +// RUN: %target-swift-frontend -emit-module %t/Lib.swift -o %t/Build1/Lib.framework/Modules/Lib.swiftmodule/%target-swiftmodule-name -emit-module-interface-path %t/Build1/Lib.framework/Modules/Lib.swiftmodule/%target-swiftinterface-name -module-name Lib // 3. Make sure when we compile this test file, we can access both APIs since we'll // load the compiled .swiftmodule instead of the .swiftinterface in the SDK. -// RUN: %target-swift-frontend -typecheck %s -F %t/BuildDir -F %t/SecondBuildDir -module-cache-path %t/ModuleCache +// RUN: %target-swift-frontend -typecheck %s -F %t/Build1 -F %t/Build2 -module-cache-path %t/ModuleCache // 4. Make sure we didn't compile any .swiftinterfaces into the module cache. // RUN: ls %t/ModuleCache | not grep 'swiftmodule' // 5. This should also work if the swiftinterface isn't present in the first build dir. -// RUN: rm %t/BuildDir/Lib.framework/Modules/Lib.swiftmodule/%target-swiftinterface-name -// RUN: %target-swift-frontend -typecheck %s -F %t/BuildDir -F %t/SecondBuildDir -module-cache-path %t/ModuleCache +// RUN: rm %t/Build1/Lib.framework/Modules/Lib.swiftmodule/%target-swiftinterface-name +// RUN: %target-swift-frontend -typecheck %s -F %t/Build1 -F %t/Build2 -module-cache-path %t/ModuleCache // 6. Make sure we /still/ didn't compile any .swiftinterfaces into the module cache. // RUN: ls %t/ModuleCache | not grep 'swiftmodule' From 50ef384f884e21f24f002de3d492a09d7060f164 Mon Sep 17 00:00:00 2001 From: Hiroshi Yamauchi Date: Thu, 8 Jun 2023 12:55:23 -0700 Subject: [PATCH 02/22] Fix the swift-inspect dump-generic-metadata operation. ReflectionContext::allocationMetadataPointer() was reading the metadata pointer from a wrong offset because of the out-of-sync struct layouts and dump-generic-metadata was not working correctly. This change resync's the layouts and adds a static_assert to verify that the offsets match between GenericMetadataCacheEntry and GenericCacheEntry. --- .../GenericMetadataCacheEntry.h | 40 +++++++++++++++++++ .../RemoteInspection/ReflectionContext.h | 15 ++----- stdlib/public/runtime/Metadata.cpp | 11 +++++ stdlib/public/runtime/MetadataCache.h | 2 + 4 files changed, 57 insertions(+), 11 deletions(-) create mode 100644 include/swift/RemoteInspection/GenericMetadataCacheEntry.h diff --git a/include/swift/RemoteInspection/GenericMetadataCacheEntry.h b/include/swift/RemoteInspection/GenericMetadataCacheEntry.h new file mode 100644 index 0000000000000..3e2f377511ad9 --- /dev/null +++ b/include/swift/RemoteInspection/GenericMetadataCacheEntry.h @@ -0,0 +1,40 @@ +//===--- GenericMetadataCacheEntry.h ----------------------------*- C++ -*-===// +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2014 - 2023 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// +// +// Declares a struct that mirrors the layout of GenericCacheEntry in +// Metadata.cpp and use a static assert to check that the offset of +// the member Value match between the two. +// +//===----------------------------------------------------------------------===// + +#ifndef SWIFT_REFLECTION_GENERICMETADATACACHEENTRY_H +#define SWIFT_REFLECTION_GENERICMETADATACACHEENTRY_H + +#include + +namespace swift { + +template +struct GenericMetadataCacheEntry { + StoredPointer TrackingInfo; + uint16_t NumKeyParameters; + uint16_t NumWitnessTables; + uint16_t NumPacks; + uint16_t NumShapeClasses; + StoredPointer PackShapeDescriptors; + uint32_t Hash; + StoredPointer Value; +}; + +} // namespace swift + +#endif // SWIFT_REFLECTION_GENERICMETADATACACHEENTRY_H diff --git a/include/swift/RemoteInspection/ReflectionContext.h b/include/swift/RemoteInspection/ReflectionContext.h index bd1492f64dab0..2d057c5133c95 100644 --- a/include/swift/RemoteInspection/ReflectionContext.h +++ b/include/swift/RemoteInspection/ReflectionContext.h @@ -30,6 +30,7 @@ #include "swift/Concurrency/Actor.h" #include "swift/Remote/MemoryReader.h" #include "swift/Remote/MetadataReader.h" +#include "swift/RemoteInspection/GenericMetadataCacheEntry.h" #include "swift/RemoteInspection/Records.h" #include "swift/RemoteInspection/RuntimeInternals.h" #include "swift/RemoteInspection/TypeLowering.h" @@ -1289,22 +1290,14 @@ class ReflectionContext StoredPointer allocationMetadataPointer( MetadataAllocation Allocation) { if (Allocation.Tag == GenericMetadataCacheTag) { - struct GenericMetadataCacheEntry { - StoredPointer LockedStorage; - uint8_t LockedStorageKind; - uint8_t TrackingInfo; - uint16_t NumKeyParameters; - uint16_t NumWitnessTables; - uint32_t Hash; - StoredPointer Value; - }; auto AllocationBytes = getReader().readBytes(RemoteAddress(Allocation.Ptr), Allocation.Size); if (!AllocationBytes) return 0; - auto Entry = reinterpret_cast( - AllocationBytes.get()); + auto Entry = + reinterpret_cast *>( + AllocationBytes.get()); return Entry->Value; } return 0; diff --git a/stdlib/public/runtime/Metadata.cpp b/stdlib/public/runtime/Metadata.cpp index cae6faac16b74..d69a602e3640d 100644 --- a/stdlib/public/runtime/Metadata.cpp +++ b/stdlib/public/runtime/Metadata.cpp @@ -28,6 +28,7 @@ #include "swift/Basic/Range.h" #include "swift/Basic/STLExtras.h" #include "swift/Demangling/Demangler.h" +#include "swift/RemoteInspection/GenericMetadataCacheEntry.h" #include "swift/Runtime/Casting.h" #include "swift/Runtime/EnvironmentVariables.h" #include "swift/Runtime/ExistentialContainer.h" @@ -427,6 +428,16 @@ namespace { }; } // end anonymous namespace +namespace swift { + struct StaticAssertGenericMetadataCacheEntryValueOffset { + static_assert( + offsetof(GenericCacheEntry, Value) == + offsetof(swift::GenericMetadataCacheEntry, + Value), + "The generic metadata cache entry layout mismatch"); + }; +} + namespace { class GenericMetadataCache : public MetadataCache { diff --git a/stdlib/public/runtime/MetadataCache.h b/stdlib/public/runtime/MetadataCache.h index 2d67f5350ee7a..2b6397b3f0726 100644 --- a/stdlib/public/runtime/MetadataCache.h +++ b/stdlib/public/runtime/MetadataCache.h @@ -1583,6 +1583,8 @@ class VariadicMetadataCacheEntryBase : bool matchesKey(const MetadataCacheKey &key) const { return key == getKey(); } + + friend struct StaticAssertGenericMetadataCacheEntryValueOffset; }; template From f940906ac65ab1ac71900190a6a170f3cfc15f99 Mon Sep 17 00:00:00 2001 From: Ben Barham Date: Mon, 12 Jun 2023 11:16:36 -0700 Subject: [PATCH 03/22] [Test] Use SyntaxRewriter.rewrite not visit SyntaxRewriters should use `rewrite` to create a new node, not `visit`. --- test/Macros/Inputs/syntax_macro_definitions.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Macros/Inputs/syntax_macro_definitions.swift b/test/Macros/Inputs/syntax_macro_definitions.swift index ca12537ee2413..100c2d420f89f 100644 --- a/test/Macros/Inputs/syntax_macro_definitions.swift +++ b/test/Macros/Inputs/syntax_macro_definitions.swift @@ -164,7 +164,7 @@ public enum AddBlocker: ExpressionMacro { in context: some MacroExpansionContext ) -> ExprSyntax { let visitor = AddVisitor() - let result = visitor.visit(Syntax(node)) + let result = visitor.rewrite(Syntax(node)) for diag in visitor.diagnostics { context.diagnose(diag) From 6308cd08343cf8c2cad518a2c869b60f9b1e61b2 Mon Sep 17 00:00:00 2001 From: Tristan Labelle Date: Mon, 12 Jun 2023 14:21:34 -0400 Subject: [PATCH 04/22] Migrate test to touch.py --- test/Driver/Dependencies/Inputs/touch.py | 9 ++++++--- test/Driver/Dependencies/one-way-merge-module-fine.swift | 8 ++++---- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/test/Driver/Dependencies/Inputs/touch.py b/test/Driver/Dependencies/Inputs/touch.py index 3537ee2d8ad18..fec6ac5bfd98d 100755 --- a/test/Driver/Dependencies/Inputs/touch.py +++ b/test/Driver/Dependencies/Inputs/touch.py @@ -15,6 +15,7 @@ # # ---------------------------------------------------------------------------- +import glob import os import sys @@ -23,6 +24,8 @@ # Update the output file mtime, or create it if necessary. # From http://stackoverflow.com/a/1160227. -for outputFile in sys.argv[2:]: - with open(outputFile, 'a'): - os.utime(outputFile, (timeVal, timeVal)) +for filePathPattern in sys.argv[2:]: + # Support glob patterns if the shell did not expand them (like cmd.exe) + for filePath in glob.glob(filePathPattern): + with open(filePath, 'a'): + os.utime(filePath, (timeVal, timeVal)) diff --git a/test/Driver/Dependencies/one-way-merge-module-fine.swift b/test/Driver/Dependencies/one-way-merge-module-fine.swift index 951ed09b9af5d..265355e2fc94a 100644 --- a/test/Driver/Dependencies/one-way-merge-module-fine.swift +++ b/test/Driver/Dependencies/one-way-merge-module-fine.swift @@ -2,7 +2,7 @@ // RUN: %empty-directory(%t) // RUN: cp -r %S/Inputs/one-way-fine/* %t -// RUN: touch -t 201401240005 %t/* +// RUN: %{python} %S/Inputs/touch.py 201401240005 %t/* // RUN: cd %t && %swiftc_driver -driver-use-frontend-path "%{python.unquoted};%S/Inputs/update-dependencies.py;%swift-dependency-tool" -output-file-map %t/output.json -incremental -driver-always-rebuild-dependents ./main.swift ./other.swift -emit-module-path %t/master.swiftmodule -module-name main -j1 -v 2>&1 | %FileCheck -check-prefix=CHECK-FIRST %s @@ -12,9 +12,9 @@ // CHECK-FIRST-DAG: Produced master.swiftmodule // swift-driver checks existence of all outputs -// RUN: touch -t 201401240006 %t/*.swiftmodule -// RUN: touch -t 201401240006 %t/*.swiftdoc -// RUN: touch -t 201401240006 %t/*.swiftsourceinfo +// RUN: %{python} %S/Inputs/touch.py 201401240006 %t/*.swiftmodule +// RUN: %{python} %S/Inputs/touch.py 201401240006 %t/*.swiftdoc +// RUN: %{python} %S/Inputs/touch.py 201401240006 %t/*.swiftsourceinfo // RUN: cd %t && %swiftc_driver -driver-use-frontend-path "%{python.unquoted};%S/Inputs/update-dependencies.py;%swift-dependency-tool" -output-file-map %t/output.json -incremental -driver-always-rebuild-dependents ./main.swift ./other.swift -emit-module-path %t/master.swiftmodule -module-name main -j1 -v 2>&1 | %FileCheck -check-prefix=CHECK-SECOND %s From 0d5aa7fcbea03fe383cfdd653fb951fe84e052eb Mon Sep 17 00:00:00 2001 From: Tristan Labelle Date: Mon, 12 Jun 2023 14:27:03 -0400 Subject: [PATCH 05/22] Fix non-glob paths in touch.py --- test/Driver/Dependencies/Inputs/touch.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/test/Driver/Dependencies/Inputs/touch.py b/test/Driver/Dependencies/Inputs/touch.py index fec6ac5bfd98d..ad517391b13a3 100755 --- a/test/Driver/Dependencies/Inputs/touch.py +++ b/test/Driver/Dependencies/Inputs/touch.py @@ -26,6 +26,12 @@ # From http://stackoverflow.com/a/1160227. for filePathPattern in sys.argv[2:]: # Support glob patterns if the shell did not expand them (like cmd.exe) - for filePath in glob.glob(filePathPattern): + if glob.escape(filePathPattern) == filePathPattern: + # Not a glob pattern. We should touch that specific file. + filePaths = [ filePathPattern ] + else: + filePaths = glob.glob(filePathPattern) + + for filePath in filePaths: with open(filePath, 'a'): os.utime(filePath, (timeVal, timeVal)) From ee3a47b62491c67c049d945e7e5ad7e6e169190f Mon Sep 17 00:00:00 2001 From: Ben Barham Date: Wed, 7 Jun 2023 17:24:36 -0700 Subject: [PATCH 06/22] [Frontend] Ignore adjacent swiftmodule in compiler host modules `lib/swift/host` contains modules/libraries that are built by the host compiler. Their `.swiftmodule` will never be able to be read, ignore them entirely. --- include/swift/AST/DiagnosticsFrontend.def | 3 +- include/swift/Basic/StringExtras.h | 3 ++ lib/AST/Module.cpp | 16 ++-------- lib/Basic/StringExtras.cpp | 13 ++++++++ lib/Frontend/ModuleInterfaceLoader.cpp | 31 +++++++++++++++++-- .../ignore-adjacent-host-module.swift | 26 ++++++++++++++++ 6 files changed, 75 insertions(+), 17 deletions(-) create mode 100644 test/ModuleInterface/ignore-adjacent-host-module.swift diff --git a/include/swift/AST/DiagnosticsFrontend.def b/include/swift/AST/DiagnosticsFrontend.def index b7e412b4eea26..9e8abe5412ad3 100644 --- a/include/swift/AST/DiagnosticsFrontend.def +++ b/include/swift/AST/DiagnosticsFrontend.def @@ -437,7 +437,8 @@ NOTE(sdk_version_pbm_version,none, NOTE(compiled_module_ignored_reason,none, "compiled module '%0' was ignored because %select{%error" "|it belongs to a framework in the SDK" - "|loading from module interfaces is preferred}1", + "|loading from module interfaces is preferred" + "|it's a compiler host module}1", (StringRef, unsigned)) NOTE(out_of_date_module_here,none, "%select{compiled|cached|forwarding|prebuilt}0 module is out of date: '%1'", diff --git a/include/swift/Basic/StringExtras.h b/include/swift/Basic/StringExtras.h index 211ea71c801c1..606abf8329b2e 100644 --- a/include/swift/Basic/StringExtras.h +++ b/include/swift/Basic/StringExtras.h @@ -519,6 +519,9 @@ class NullTerminatedStringRef { /// where escaped Unicode characters lead to malformed/invalid JSON. void writeEscaped(llvm::StringRef Str, llvm::raw_ostream &OS); +/// Whether the path components of `path` begin with those from `prefix`. +bool pathStartsWith(StringRef prefix, StringRef path); + } // end namespace swift #endif // SWIFT_BASIC_STRINGEXTRAS_H diff --git a/lib/AST/Module.cpp b/lib/AST/Module.cpp index dd8a02dcb3f1b..3071fb7ac6b20 100644 --- a/lib/AST/Module.cpp +++ b/lib/AST/Module.cpp @@ -43,6 +43,7 @@ #include "swift/Basic/Compiler.h" #include "swift/Basic/SourceManager.h" #include "swift/Basic/Statistic.h" +#include "swift/Basic/StringExtras.h" #include "swift/Demangling/ManglingMacros.h" #include "swift/Parse/Token.h" #include "swift/Strings.h" @@ -4235,18 +4236,6 @@ FrontendStatsTracer::getTraceFormatter() { return &TF; } -static bool prefixMatches(StringRef prefix, StringRef path) { - auto prefixIt = llvm::sys::path::begin(prefix), - prefixEnd = llvm::sys::path::end(prefix); - for (auto pathIt = llvm::sys::path::begin(path), - pathEnd = llvm::sys::path::end(path); - prefixIt != prefixEnd && pathIt != pathEnd; ++prefixIt, ++pathIt) { - if (*prefixIt != *pathIt) - return false; - } - return prefixIt == prefixEnd; -} - bool IsNonUserModuleRequest::evaluate(Evaluator &evaluator, ModuleDecl *mod) const { // stdlib is non-user by definition if (mod->isStdlibModule()) @@ -4274,5 +4263,6 @@ bool IsNonUserModuleRequest::evaluate(Evaluator &evaluator, ModuleDecl *mod) con return false; StringRef runtimePath = searchPathOpts.RuntimeResourcePath; - return (!runtimePath.empty() && prefixMatches(runtimePath, modulePath)) || (!sdkPath.empty() && prefixMatches(sdkPath, modulePath)); + return (!runtimePath.empty() && pathStartsWith(runtimePath, modulePath)) || + (!sdkPath.empty() && pathStartsWith(sdkPath, modulePath)); } diff --git a/lib/Basic/StringExtras.cpp b/lib/Basic/StringExtras.cpp index 99170b27f5e7e..a2cdf482f2493 100644 --- a/lib/Basic/StringExtras.cpp +++ b/lib/Basic/StringExtras.cpp @@ -23,6 +23,7 @@ #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringSwitch.h" #include "llvm/Support/Compiler.h" +#include "llvm/Support/Path.h" #include "llvm/Support/raw_ostream.h" #include @@ -1417,3 +1418,15 @@ void swift::writeEscaped(llvm::StringRef Str, llvm::raw_ostream &OS) { } } } + +bool swift::pathStartsWith(StringRef prefix, StringRef path) { + auto prefixIt = llvm::sys::path::begin(prefix), + prefixEnd = llvm::sys::path::end(prefix); + for (auto pathIt = llvm::sys::path::begin(path), + pathEnd = llvm::sys::path::end(path); + prefixIt != prefixEnd && pathIt != pathEnd; ++prefixIt, ++pathIt) { + if (*prefixIt != *pathIt) + return false; + } + return prefixIt == prefixEnd; +} diff --git a/lib/Frontend/ModuleInterfaceLoader.cpp b/lib/Frontend/ModuleInterfaceLoader.cpp index af9a42220d986..9b70f29e0e80d 100644 --- a/lib/Frontend/ModuleInterfaceLoader.cpp +++ b/lib/Frontend/ModuleInterfaceLoader.cpp @@ -20,6 +20,7 @@ #include "swift/AST/FileSystem.h" #include "swift/AST/Module.h" #include "swift/Basic/Platform.h" +#include "swift/Basic/StringExtras.h" #include "swift/Frontend/CachingUtils.h" #include "swift/Frontend/Frontend.h" #include "swift/Frontend/ModuleInterfaceSupport.h" @@ -233,6 +234,7 @@ struct ModuleRebuildInfo { NotIgnored, PublicFramework, InterfacePreferred, + CompilerHostModule, }; struct CandidateModule { std::string path; @@ -704,7 +706,28 @@ class ModuleInterfaceLoaderImpl { bool isInResourceDir(StringRef path) { StringRef resourceDir = ctx.SearchPathOpts.RuntimeResourcePath; if (resourceDir.empty()) return false; - return path.startswith(resourceDir); + return pathStartsWith(resourceDir, path); + } + + bool isInResourceHostDir(StringRef path) { + StringRef resourceDir = ctx.SearchPathOpts.RuntimeResourcePath; + if (resourceDir.empty()) return false; + + SmallString<128> hostPath; + llvm::sys::path::append(hostPath, + resourceDir, "host"); + return pathStartsWith(hostPath, path); + } + + bool isInSystemFrameworks(StringRef path) { + StringRef sdkPath = ctx.SearchPathOpts.getSDKPath(); + if (sdkPath.empty()) return false; + + SmallString<128> publicFrameworksPath; + llvm::sys::path::append(publicFrameworksPath, + sdkPath, "System", "Library", "Frameworks"); + + return pathStartsWith(publicFrameworksPath, path); } std::pair getCompiledModuleCandidates() { @@ -719,10 +742,12 @@ class ModuleInterfaceLoaderImpl { llvm::sys::path::append(publicFrameworksPath, ctx.SearchPathOpts.getSDKPath(), "System", "Library", "Frameworks"); - if (!ctx.SearchPathOpts.getSDKPath().empty() && - modulePath.startswith(publicFrameworksPath)) { + if (isInSystemFrameworks(modulePath)) { shouldLoadAdjacentModule = false; rebuildInfo.addIgnoredModule(modulePath, ReasonIgnored::PublicFramework); + } else if (isInResourceHostDir(modulePath)) { + shouldLoadAdjacentModule = false; + rebuildInfo.addIgnoredModule(modulePath, ReasonIgnored::CompilerHostModule); } switch (loadMode) { diff --git a/test/ModuleInterface/ignore-adjacent-host-module.swift b/test/ModuleInterface/ignore-adjacent-host-module.swift new file mode 100644 index 0000000000000..1abe459da9e01 --- /dev/null +++ b/test/ModuleInterface/ignore-adjacent-host-module.swift @@ -0,0 +1,26 @@ +// RUN: %empty-directory(%t) +// RUN: %empty-directory(%t/build/host) +// RUN: %empty-directory(%t/cache) +// RUN: split-file %s %t + +/// Modules loaded from within lib/swift/host should also be rebuilt from +/// their interface (which actually means anything within resource-dir/host). + +// RUN: %target-swift-frontend -emit-module %t/Lib.swift \ +// RUN: -swift-version 5 -enable-library-evolution \ +// RUN: -parse-stdlib -module-cache-path %t/cache \ +// RUN: -o %t/build/host -emit-module-interface-path %t/build/host/Lib.swiftinterface + +// RUN: %target-swift-frontend -typecheck %t/Client.swift \ +// RUN: -resource-dir %t/build -I %t/build/host \ +// RUN: -parse-stdlib -module-cache-path %t/cache \ +// RUN: -Rmodule-loading 2>&1 | %FileCheck %s + +// CHECK: remark: loaded module 'Lib'; source: '{{.*}}{{/|\\}}host{{/|\\}}Lib.swiftinterface', loaded: '{{.*}}{{/|\\}}cache{{/|\\}}Lib-{{.*}}.swiftmodule' + +//--- Lib.swift +public func foo() {} + +//--- Client.swift +import Lib +foo() From 11f34b7ec93cb56cd7b5b87f49f9bd37f0053da1 Mon Sep 17 00:00:00 2001 From: Tristan Labelle Date: Tue, 13 Jun 2023 10:28:42 -0400 Subject: [PATCH 07/22] Fix linting error --- test/Driver/Dependencies/Inputs/touch.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Driver/Dependencies/Inputs/touch.py b/test/Driver/Dependencies/Inputs/touch.py index ad517391b13a3..85b0908984cec 100755 --- a/test/Driver/Dependencies/Inputs/touch.py +++ b/test/Driver/Dependencies/Inputs/touch.py @@ -28,7 +28,7 @@ # Support glob patterns if the shell did not expand them (like cmd.exe) if glob.escape(filePathPattern) == filePathPattern: # Not a glob pattern. We should touch that specific file. - filePaths = [ filePathPattern ] + filePaths = [filePathPattern] else: filePaths = glob.glob(filePathPattern) From 56130069444534a6eb9441d3ae46d0fc5087dbbf Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Wed, 7 Jun 2023 14:31:32 -0700 Subject: [PATCH 08/22] [Sema] PreCheck: Diagnose standalone self within init accessors 'self' within init accessor could only be used to refer to properties listed in `initializes` and `accesses` attributes. --- include/swift/AST/DeclContext.h | 13 ++++++++++ include/swift/AST/DiagnosticsSema.def | 4 +++ lib/AST/DeclContext.cpp | 18 +++++++++++++ lib/Sema/PreCheckExpr.cpp | 20 +++++++++++++-- test/decl/var/init_accessors.swift | 37 +++++++++++++++++++++++++++ 5 files changed, 90 insertions(+), 2 deletions(-) diff --git a/include/swift/AST/DeclContext.h b/include/swift/AST/DeclContext.h index d6e21e3167959..41b20f8e5b5b3 100644 --- a/include/swift/AST/DeclContext.h +++ b/include/swift/AST/DeclContext.h @@ -82,6 +82,7 @@ namespace swift { class SerializedDefaultArgumentInitializer; class SerializedTopLevelCodeDecl; class StructDecl; + class AccessorDecl; namespace serialization { using DeclID = llvm::PointerEmbeddedInt; @@ -457,6 +458,18 @@ class alignas(1 << DeclContextAlignInBits) DeclContext return const_cast(this)->getInnermostMethodContext(); } + /// Returns the innermost accessor context that belongs to a property. + /// + /// This routine looks through closure, initializer, and local function + /// contexts to find the innermost accessor declaration. + /// + /// \returns the innermost accessor, or null if there is no such context. + LLVM_READONLY + AccessorDecl *getInnermostPropertyAccessorContext(); + const AccessorDecl *getInnermostPropertyAccessorContext() const { + return const_cast(this)->getInnermostPropertyAccessorContext(); + } + /// Returns the innermost type context. /// /// This routine looks through closure, initializer, and local function diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index d77f402f63d7b..2b5a55cdd88b2 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -7342,6 +7342,10 @@ ERROR(init_accessor_accesses_attribute_on_other_declaration,none, ERROR(init_accessor_property_both_init_and_accessed,none, "property %0 cannot be both initialized and accessed", (DeclName)) +ERROR(invalid_use_of_self_in_init_accessor,none, + "'self' within init accessors can only be used to reference " + "properties listed in 'initializes' and 'accesses'; " + "init accessors are run before 'self' is fully available", ()) #define UNDEFINE_DIAGNOSTIC_MACROS diff --git a/lib/AST/DeclContext.cpp b/lib/AST/DeclContext.cpp index 9a02c2f6cc9e0..88e9cf8da265b 100644 --- a/lib/AST/DeclContext.cpp +++ b/lib/AST/DeclContext.cpp @@ -211,6 +211,24 @@ AbstractFunctionDecl *DeclContext::getInnermostMethodContext() { return nullptr; } +AccessorDecl *DeclContext::getInnermostPropertyAccessorContext() { + auto dc = this; + do { + if (auto decl = dc->getAsDecl()) { + auto accessor = dyn_cast(decl); + // If we found a non-accessor decl, we're done. + if (accessor == nullptr) + return nullptr; + + auto *storage = accessor->getStorage(); + if (isa(storage) && storage->getDeclContext()->isTypeContext()) + return accessor; + } + } while ((dc = dc->getParent())); + + return nullptr; +} + bool DeclContext::isTypeContext() const { if (auto decl = getAsDecl()) return isa(decl) || isa(decl); diff --git a/lib/Sema/PreCheckExpr.cpp b/lib/Sema/PreCheckExpr.cpp index f634cdf72cdaa..c80255133c9bf 100644 --- a/lib/Sema/PreCheckExpr.cpp +++ b/lib/Sema/PreCheckExpr.cpp @@ -1066,8 +1066,24 @@ namespace { if (auto unresolved = dyn_cast(expr)) { TypeChecker::checkForForbiddenPrefix( getASTContext(), unresolved->getName().getBaseName()); - return finish(true, TypeChecker::resolveDeclRefExpr(unresolved, DC, - UseErrorExprs)); + auto *refExpr = + TypeChecker::resolveDeclRefExpr(unresolved, DC, UseErrorExprs); + + // Check whether this is standalone `self` in init accessor, which + // is invalid. + if (auto *accessor = DC->getInnermostPropertyAccessorContext()) { + if (accessor->isInitAccessor() && isa(refExpr)) { + auto *DRE = cast(refExpr); + if (accessor->getImplicitSelfDecl() == DRE->getDecl() && + !isa_and_nonnull(Parent.getAsExpr())) { + Ctx.Diags.diagnose(unresolved->getLoc(), + diag::invalid_use_of_self_in_init_accessor); + refExpr = new (Ctx) ErrorExpr(unresolved->getSourceRange()); + } + } + } + + return finish(true, refExpr); } // Let's try to figure out if `InOutExpr` is out of place early diff --git a/test/decl/var/init_accessors.swift b/test/decl/var/init_accessors.swift index 93eeb37bdc5af..582e1c057ce8d 100644 --- a/test/decl/var/init_accessors.swift +++ b/test/decl/var/init_accessors.swift @@ -136,3 +136,40 @@ func test_duplicate_and_computed_lazy_properties() { lazy var c: Int = 42 } } + +func test_invalid_self_uses() { + func capture(_: T) -> Int? { nil } + + struct Test { + var a: Int { + init(initialValue) { + let x = self + // expected-error@-1 {{'self' within init accessors can only be used to reference properties listed in 'initializes' and 'accesses'; init accessors are run before 'self' is fully available}} + + _ = { + print(self) + // expected-error@-1 {{'self' within init accessors can only be used to reference properties listed in 'initializes' and 'accesses'; init accessors are run before 'self' is fully available}} + } + + _ = { [weak self] in + // expected-error@-1 {{'self' within init accessors can only be used to reference properties listed in 'initializes' and 'accesses'; init accessors are run before 'self' is fully available}} + } + + _ = { + guard let _ = capture(self) else { + // expected-error@-1 {{'self' within init accessors can only be used to reference properties listed in 'initializes' and 'accesses'; init accessors are run before 'self' is fully available}} + return + } + } + + Test.test(self) + // expected-error@-1 {{'self' within init accessors can only be used to reference properties listed in 'initializes' and 'accesses'; init accessors are run before 'self' is fully available}} + } + + get { 42 } + set { } + } + + static func test(_: T) {} + } +} From 935142ff496189da3ca697e32341141cdc531e36 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Wed, 7 Jun 2023 16:18:21 -0700 Subject: [PATCH 09/22] [ConstraintSystem] InitAccessors: Detect invalid references to members within init accessors Only properties that are listed in 'initializes' and 'accesses' attributes could be referenced within init accessor. Detect any and all invalid member references in the solver. --- include/swift/Sema/ConstraintSystem.h | 5 ++++ lib/Sema/CSSimplify.cpp | 41 +++++++++++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 6d6bb23a2712c..a18165f4ac5ab 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -1879,6 +1879,11 @@ struct MemberLookupResult { /// This is a static member being access through a protocol metatype /// but its result type doesn't conform to this protocol. UR_InvalidStaticMemberOnProtocolMetatype, + + /// This is a member that doesn't appear in 'initializes' and/or + /// 'accesses' attributes of the init accessor and therefore canno + /// t be referenced in its body. + UR_UnavailableWithinInitAccessor, }; /// This is a list of considered (but rejected) candidates, along with a diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index da48e9cd8286a..fa8b9c6bdbd83 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -9632,6 +9632,44 @@ performMemberLookup(ConstraintKind constraintKind, DeclNameRef memberName, } } + if (auto *UDE = + getAsExpr(memberLocator->getAnchor())) { + auto *base = UDE->getBase(); + if (auto *accessor = DC->getInnermostPropertyAccessorContext()) { + if (accessor->isInitAccessor() && isa(base) && + accessor->getImplicitSelfDecl() == + cast(base)->getDecl()) { + bool isValidReference = false; + + // If name doesn't appear in either `initializes` or `accesses` + // then it's invalid instance member. + + if (auto *initializesAttr = + accessor->getAttrs().getAttribute()) { + isValidReference |= llvm::any_of( + initializesAttr->getProperties(), [&](Identifier name) { + return DeclNameRef(name) == memberName; + }); + } + + if (auto *accessesAttr = + accessor->getAttrs().getAttribute()) { + isValidReference |= llvm::any_of( + accessesAttr->getProperties(), [&](Identifier name) { + return DeclNameRef(name) == memberName; + }); + } + + if (!isValidReference) { + result.addUnviable( + candidate, + MemberLookupResult::UR_UnavailableWithinInitAccessor); + return; + } + } + } + } + // If the underlying type of a typealias is fully concrete, it is legal // to access the type with a protocol metatype base. } else if (instanceTy->isExistentialType() && @@ -10208,6 +10246,9 @@ fixMemberRef(ConstraintSystem &cs, Type baseTy, case MemberLookupResult::UR_InvalidStaticMemberOnProtocolMetatype: return AllowInvalidStaticMemberRefOnProtocolMetatype::create(cs, locator); + + case MemberLookupResult::UR_UnavailableWithinInitAccessor: + return nullptr; } } From 3c924be2a058be26bd7c1d224623103445fb2c80 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Wed, 7 Jun 2023 16:21:54 -0700 Subject: [PATCH 10/22] [CSFix] InitAccessors: Add a fix that tracks invalid member references within init accessors --- include/swift/Sema/CSFix.h | 37 +++++++++++++++++++++++++++++++++++++ lib/Sema/CSFix.cpp | 13 +++++++++++++ lib/Sema/CSSimplify.cpp | 7 ++++++- 3 files changed, 56 insertions(+), 1 deletion(-) diff --git a/include/swift/Sema/CSFix.h b/include/swift/Sema/CSFix.h index 55d9349726ec3..4b05b20679351 100644 --- a/include/swift/Sema/CSFix.h +++ b/include/swift/Sema/CSFix.h @@ -447,6 +447,10 @@ enum class FixKind : uint8_t { /// Ignore missing 'each' keyword before value pack reference. IgnoreMissingEachKeyword, + + /// Ignore the fact that member couldn't be referenced within init accessor + /// because its name doesn't appear in 'initializes' or 'accesses' attributes. + AllowInvalidMemberReferenceInInitAccessor, }; class ConstraintFix { @@ -3536,6 +3540,39 @@ class IgnoreMissingEachKeyword final : public ConstraintFix { } }; +class AllowInvalidMemberReferenceInInitAccessor final : public ConstraintFix { + DeclNameRef MemberName; + + AllowInvalidMemberReferenceInInitAccessor(ConstraintSystem &cs, + DeclNameRef memberName, + ConstraintLocator *locator) + : ConstraintFix(cs, FixKind::AllowInvalidMemberReferenceInInitAccessor, + locator), + MemberName(memberName) {} + +public: + std::string getName() const override { + llvm::SmallVector scratch; + auto memberName = MemberName.getString(scratch); + return "allow reference to member '" + memberName.str() + + "' in init accessor"; + } + + bool diagnose(const Solution &solution, bool asNote = false) const override; + + bool diagnoseForAmbiguity(CommonFixesArray commonFixes) const override { + return diagnose(*commonFixes.front().first); + } + + static AllowInvalidMemberReferenceInInitAccessor * + create(ConstraintSystem &cs, DeclNameRef memberName, + ConstraintLocator *locator); + + static bool classof(const ConstraintFix *fix) { + return fix->getKind() == FixKind::AllowInvalidMemberReferenceInInitAccessor; + } +}; + } // end namespace constraints } // end namespace swift diff --git a/lib/Sema/CSFix.cpp b/lib/Sema/CSFix.cpp index 9b9373bd993c2..2e027dc036a50 100644 --- a/lib/Sema/CSFix.cpp +++ b/lib/Sema/CSFix.cpp @@ -2796,3 +2796,16 @@ IgnoreMissingEachKeyword::create(ConstraintSystem &cs, Type valuePackTy, return new (cs.getAllocator()) IgnoreMissingEachKeyword(cs, valuePackTy, locator); } + +bool AllowInvalidMemberReferenceInInitAccessor::diagnose( + const Solution &solution, bool asNote) const { + return false; +} + +AllowInvalidMemberReferenceInInitAccessor * +AllowInvalidMemberReferenceInInitAccessor::create(ConstraintSystem &cs, + DeclNameRef memberName, + ConstraintLocator *locator) { + return new (cs.getAllocator()) + AllowInvalidMemberReferenceInInitAccessor(cs, memberName, locator); +} diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index fa8b9c6bdbd83..303e8161ea8dd 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -10248,7 +10248,8 @@ fixMemberRef(ConstraintSystem &cs, Type baseTy, return AllowInvalidStaticMemberRefOnProtocolMetatype::create(cs, locator); case MemberLookupResult::UR_UnavailableWithinInitAccessor: - return nullptr; + return AllowInvalidMemberReferenceInInitAccessor::create(cs, memberName, + locator); } } @@ -14674,6 +14675,10 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint( return recordFix(fix, 100) ? SolutionKind::Error : SolutionKind::Solved; } + case FixKind::AllowInvalidMemberReferenceInInitAccessor: { + return recordFix(fix, 5) ? SolutionKind::Error : SolutionKind::Solved; + } + case FixKind::ExplicitlyConstructRawRepresentable: { // Let's increase impact of this fix for binary operators because // it's possible to get both `.rawValue` and construction fixes for From db024d973e5c840fc2e62c33b349a9bebc7cba58 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Wed, 7 Jun 2023 16:52:48 -0700 Subject: [PATCH 11/22] [CSDiagnostics] InitAccessors: Implement invalid member reference diagnostics within init accessors --- include/swift/AST/DiagnosticsSema.def | 6 +++- lib/Sema/CSDiagnostics.cpp | 5 +++ lib/Sema/CSDiagnostics.h | 13 ++++++++ lib/Sema/CSFix.cpp | 4 ++- test/decl/var/init_accessors.swift | 48 +++++++++++++++++++++++++++ 5 files changed, 74 insertions(+), 2 deletions(-) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 2b5a55cdd88b2..d1360f1c88d03 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -7346,7 +7346,11 @@ ERROR(invalid_use_of_self_in_init_accessor,none, "'self' within init accessors can only be used to reference " "properties listed in 'initializes' and 'accesses'; " "init accessors are run before 'self' is fully available", ()) - +ERROR(init_accessor_invalid_member_ref,none, + "cannot reference instance member %0; init accessors can only " + "refer to instance properties listed in 'initializes' and " + "'accesses' attributes", + (DeclNameRef)) #define UNDEFINE_DIAGNOSTIC_MACROS #include "DefineDiagnosticMacros.h" diff --git a/lib/Sema/CSDiagnostics.cpp b/lib/Sema/CSDiagnostics.cpp index c2ed909db3352..19f63f946a384 100644 --- a/lib/Sema/CSDiagnostics.cpp +++ b/lib/Sema/CSDiagnostics.cpp @@ -9058,3 +9058,8 @@ bool MissingEachForValuePackReference::diagnoseAsError() { return true; } + +bool InvalidMemberReferenceWithinInitAccessor::diagnoseAsError() { + emitDiagnostic(diag::init_accessor_invalid_member_ref, MemberName); + return true; +} diff --git a/lib/Sema/CSDiagnostics.h b/lib/Sema/CSDiagnostics.h index 446d5e2dfe6b8..858279d586786 100644 --- a/lib/Sema/CSDiagnostics.h +++ b/lib/Sema/CSDiagnostics.h @@ -3015,6 +3015,19 @@ class MissingEachForValuePackReference final : public FailureDiagnostic { bool diagnoseAsError() override; }; +class InvalidMemberReferenceWithinInitAccessor final + : public FailureDiagnostic { + DeclNameRef MemberName; + +public: + InvalidMemberReferenceWithinInitAccessor(const Solution &solution, + DeclNameRef memberName, + ConstraintLocator *locator) + : FailureDiagnostic(solution, locator), MemberName(memberName) {} + + bool diagnoseAsError() override; +}; + } // end namespace constraints } // end namespace swift diff --git a/lib/Sema/CSFix.cpp b/lib/Sema/CSFix.cpp index 2e027dc036a50..f4611c0b92758 100644 --- a/lib/Sema/CSFix.cpp +++ b/lib/Sema/CSFix.cpp @@ -2799,7 +2799,9 @@ IgnoreMissingEachKeyword::create(ConstraintSystem &cs, Type valuePackTy, bool AllowInvalidMemberReferenceInInitAccessor::diagnose( const Solution &solution, bool asNote) const { - return false; + InvalidMemberReferenceWithinInitAccessor failure(solution, MemberName, + getLocator()); + return failure.diagnose(asNote); } AllowInvalidMemberReferenceInInitAccessor * diff --git a/test/decl/var/init_accessors.swift b/test/decl/var/init_accessors.swift index 582e1c057ce8d..d7477f40898fb 100644 --- a/test/decl/var/init_accessors.swift +++ b/test/decl/var/init_accessors.swift @@ -173,3 +173,51 @@ func test_invalid_self_uses() { static func test(_: T) {} } } + +func test_invalid_references() { + struct Test { + var _a: Int + var _b: Int + + var c: String + static var c: String = "" + + var _d: String + + var data: Int { + init(initialValue) initializes(_a) accesses(_d) { + _a = initialValue // Ok + + print(_d) // Ok + self._d = "" // Ok + + if self._b > 0 { // expected-error {{cannot reference instance member '_b'; init accessors can only refer to instance properties listed in 'initializes' and 'accesses' attributes}} + } + + let x = c.lowercased() + // expected-error@-1 {{static member 'c' cannot be used on instance of type 'Test'}} + + print(Test.c.lowercased()) // Ok + + guard let v = test() else { + // expected-error@-1 {{cannot reference instance member 'test'; init accessors can only refer to instance properties listed in 'initializes' and 'accesses' attributes}} + return + } + + _ = { + if true { + print(_b) + // expected-error@-1 {{cannot reference instance member '_b'; init accessors can only refer to instance properties listed in 'initializes' and 'accesses' attributes}} + print(self._b) + // expected-error@-1 {{cannot reference instance member '_b'; init accessors can only refer to instance properties listed in 'initializes' and 'accesses' attributes}} + } + } + } + + get { _a } + set { } + } + + func test() -> Int? { 42 } + } +} From f6fd0bc1a722b7794cffa4876ae0cbc53a589819 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Thu, 8 Jun 2023 15:40:35 -0700 Subject: [PATCH 12/22] [DI] InitAccessors: Enforce that @out parameters are fully initialized before every terminator This closes a hole where an early return could leave some properties from `initializes(...)` list uninitialized. For example: ```swift init(initialValue) initializes(_a, _b) { _a = initialValue.0 if _a > 0 { return } _b = initialValue.1 } ``` Here `_b` is not initialized when `_a > 0`. --- .../Mandatory/DefiniteInitialization.cpp | 40 +++++++++++++++---- ...t_accessor_definite_init_diagnostics.swift | 40 +++++++++++++++++++ 2 files changed, 72 insertions(+), 8 deletions(-) diff --git a/lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp b/lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp index 12cb54a377583..4ac28a2b993b6 100644 --- a/lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp +++ b/lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp @@ -1154,14 +1154,38 @@ void LifetimeChecker::doIt() { // All of the indirect results marked as "out" have to be fully initialized // before their lifetime ends. - if (TheMemory.isOut() && Uses.empty()) { - auto loc = TheMemory.getLoc(); - - std::string propertyName; - auto *property = TheMemory.getPathStringToElement(0, propertyName); - diagnose(Module, F.getLocation(), - diag::ivar_not_initialized_by_init_accessor, property->getName()); - EmittedErrorLocs.push_back(loc); + if (TheMemory.isOut()) { + auto diagnoseMissingInit = [&]() { + std::string propertyName; + auto *property = TheMemory.getPathStringToElement(0, propertyName); + diagnose(Module, F.getLocation(), + diag::ivar_not_initialized_by_init_accessor, + property->getName()); + EmittedErrorLocs.push_back(TheMemory.getLoc()); + }; + + // No uses means that there was no initialization. + if (Uses.empty()) { + diagnoseMissingInit(); + return; + } + + // Go over every return block and check whether member is fully initialized + // because it's possible that there is branch that doesn't have any use of + // the memory and nothing else is going to diagnose that. This is different + // from `self`, for example, because it would always have either `copy_addr` + // or `load` before return. + + auto returnBB = F.findReturnBB(); + + while (returnBB != F.end()) { + auto *terminator = returnBB->getTerminator(); + + if (!isInitializedAtUse(DIMemoryUse(terminator, DIUseKind::Load, 0, 1))) + diagnoseMissingInit(); + + ++returnBB; + } } // If the memory object has nontrivial type, then any destroy/release of the diff --git a/test/SILOptimizer/init_accessor_definite_init_diagnostics.swift b/test/SILOptimizer/init_accessor_definite_init_diagnostics.swift index 5198db703c0c8..2f5137d25b814 100644 --- a/test/SILOptimizer/init_accessor_definite_init_diagnostics.swift +++ b/test/SILOptimizer/init_accessor_definite_init_diagnostics.swift @@ -139,3 +139,43 @@ struct TestAccessBeforeInit { self.y = y } } + +class TestInitWithGuard { + var _a: Int + var _b: Int + + var pair1: (Int, Int) { + init(initialValue) initializes(_a, _b) { // expected-error {{property '_b' not initialized by init accessor}} + _a = initialValue.0 + + if _a > 0 { + return + } + + _b = initialValue.1 + } + + get { (_a, _b) } + set { } + } + + var pair2: (Int, Int) { + init(initialValue) initializes(_a, _b) { // Ok + _a = initialValue.0 + + if _a > 0 { + _b = 0 + return + } + + _b = initialValue.1 + } + + get { (_a, _b) } + set { } + } + + init(a: Int, b: Int) { + self.pair2 = (a, b) + } +} From ddcfe01ba51207ea25618e8aab8fe0c95ed2ed78 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Fri, 9 Jun 2023 10:20:40 -0700 Subject: [PATCH 13/22] [Sema/SILGen] InitAccessors: Emit intersecting init accessor calls in memberwise init If init accessor initialize the same properties, let's emit them in sequence and emit `destroy_addr` in-between to make sure that there is no double initialization. --- lib/SILGen/SILGenConstructor.cpp | 50 ++++--- lib/Sema/CodeSynthesis.cpp | 30 ++--- test/Interpreter/init_accessors.swift | 122 ++++++++++++++++++ .../init_accessor_raw_sil_lowering.swift | 62 +++++++++ test/decl/var/init_accessors.swift | 114 ++++++++++++++++ 5 files changed, 348 insertions(+), 30 deletions(-) diff --git a/lib/SILGen/SILGenConstructor.cpp b/lib/SILGen/SILGenConstructor.cpp index 93a158f8eeb60..1c510e0561103 100644 --- a/lib/SILGen/SILGenConstructor.cpp +++ b/lib/SILGen/SILGenConstructor.cpp @@ -272,22 +272,31 @@ static RValue maybeEmitPropertyWrapperInitFromValue( subs, std::move(arg)); } -static void emitApplyOfInitAccessor(SILGenFunction &SGF, SILLocation loc, - AccessorDecl *accessor, SILValue selfValue, - SILType selfTy, RValue &&initialValue) { +static void +emitApplyOfInitAccessor(SILGenFunction &SGF, SILLocation loc, + AccessorDecl *accessor, SILValue selfValue, + SILType selfTy, RValue &&initialValue, + llvm::SmallPtrSetImpl &initializedFields) { SmallVector arguments; - auto emitFieldReference = [&](VarDecl *field) { + auto emitFieldReference = [&](VarDecl *field, bool forInit = false) { auto fieldTy = selfTy.getFieldType(field, SGF.SGM.M, SGF.getTypeExpansionContext()); - return SGF.B.createStructElementAddr(loc, selfValue, field, - fieldTy.getAddressType()); + auto fieldAddr = SGF.B.createStructElementAddr(loc, selfValue, field, + fieldTy.getAddressType()); + + // If another init accessor already initialized this field then we need + // to emit destroy before calling this accessor. + if (forInit && !initializedFields.insert(field).second) + SGF.B.emitDestroyAddr(loc, fieldAddr); + + return fieldAddr; }; // First, let's emit all of the indirect results. if (auto *initAttr = accessor->getAttrs().getAttribute()) { for (auto *property : initAttr->getPropertyDecls(accessor)) { - arguments.push_back(emitFieldReference(property)); + arguments.push_back(emitFieldReference(property, /*forInit=*/true)); } } @@ -402,6 +411,9 @@ static void emitImplicitValueConstructor(SILGenFunction &SGF, // Tracks all the init accessors we have emitted // because they can initialize more than one property. llvm::SmallPtrSet emittedInitAccessors; + // Tracks all the fields that have been already initialized + // via an init accessor. + llvm::SmallPtrSet fieldsInitializedViaAccessor; auto elti = elements.begin(), eltEnd = elements.end(); for (VarDecl *field : decl->getStoredProperties()) { @@ -409,17 +421,25 @@ static void emitImplicitValueConstructor(SILGenFunction &SGF, // Handle situations where this stored propery is initialized // via a call to an init accessor on some other property. if (initializedViaAccessor.count(field)) { - auto *initProperty = initializedViaAccessor.find(field)->second; - auto *initAccessor = initProperty->getAccessor(AccessorKind::Init); + for (auto iter = initializedViaAccessor.find(field); + iter != initializedViaAccessor.end(); ++iter) { + auto *initProperty = iter->second; + auto *initAccessor = initProperty->getAccessor(AccessorKind::Init); - if (emittedInitAccessors.count(initAccessor)) - continue; + if (!emittedInitAccessors.insert(initAccessor).second) + continue; - emitApplyOfInitAccessor(SGF, Loc, initAccessor, resultSlot, selfTy, - std::move(*elti)); + assert(elti != eltEnd && + "number of args does not match number of fields"); - emittedInitAccessors.insert(initAccessor); - ++elti; + emitApplyOfInitAccessor(SGF, Loc, initAccessor, resultSlot, selfTy, + std::move(*elti), + fieldsInitializedViaAccessor); + ++elti; + } + + // After all init accessors are emitted let's move on to the next + // property. continue; } diff --git a/lib/Sema/CodeSynthesis.cpp b/lib/Sema/CodeSynthesis.cpp index 26e6a088d0cea..6279b44b2267c 100644 --- a/lib/Sema/CodeSynthesis.cpp +++ b/lib/Sema/CodeSynthesis.cpp @@ -303,6 +303,11 @@ static ConstructorDecl *createImplicitConstructor(NominalTypeDecl *decl, std::multimap initializedViaAccessor; decl->collectPropertiesInitializableByInitAccessors(initializedViaAccessor); + auto createParameter = [&](VarDecl *property) { + accessLevel = std::min(accessLevel, property->getFormalAccess()); + params.push_back(createMemberwiseInitParameter(decl, Loc, property)); + }; + // A single property could be used to initialize N other stored // properties via a call to its init accessor. llvm::SmallPtrSet usedInitProperties; @@ -315,22 +320,17 @@ static ConstructorDecl *createImplicitConstructor(NominalTypeDecl *decl, continue; // Check whether this property could be initialized via init accessor. - // - // Note that we check for a single match here because intersecting - // properties are going to be diagnosed. - if (initializedViaAccessor.count(var) == 1) { - auto *initializerProperty = initializedViaAccessor.find(var)->second; - // Parameter for this property is already emitted. - if (usedInitProperties.count(initializerProperty)) - continue; - - var = initializerProperty; - usedInitProperties.insert(initializerProperty); - } - - accessLevel = std::min(accessLevel, var->getFormalAccess()); + if (initializedViaAccessor.count(var)) { + for (auto iter = initializedViaAccessor.find(var); + iter != initializedViaAccessor.end(); ++iter) { + auto *initializerProperty = iter->second; - params.push_back(createMemberwiseInitParameter(decl, Loc, var)); + if (usedInitProperties.insert(initializerProperty).second) + createParameter(initializerProperty); + } + } else { + createParameter(var); + } } } else if (ICK == ImplicitConstructorKind::DefaultDistributedActor) { auto classDecl = dyn_cast(decl); diff --git a/test/Interpreter/init_accessors.swift b/test/Interpreter/init_accessors.swift index 69e5f52ea99fa..765c6a9323e0e 100644 --- a/test/Interpreter/init_accessors.swift +++ b/test/Interpreter/init_accessors.swift @@ -332,3 +332,125 @@ test_assignments() // CHECK-NEXT: test-assignments-1: (3, 42) // CHECK-NEXT: a-init-accessor: 0 // CHECK-NEXT: test-assignments-2: (0, 2) + +func test_memberwise_with_overlaps() { + struct Test1 { + var _a: T + var _b: Int + + var a: T { + init(initialValue) initializes(_a) { + _a = initialValue + } + + get { _a } + set { } + } + + var pair: (T, Int) { + init(initialValue) initializes(_a, _b) { + _a = initialValue.0 + _b = initialValue.1 + } + + get { (_a, _b) } + set { } + } + + var c: U + } + + let test1 = Test1(a: "a", pair: ("b", 1), c: [3.0]) + print("memberwise-overlaps-1: \(test1)") + + struct Test2 { + var _a: T + var _b: Int + + var a: T { + init(initialValue) initializes(_a) { + _a = initialValue + } + + get { _a } + set { } + } + + var b: Int { + init(initialValue) initializes(_b) { + _b = initialValue + } + + get { _b } + set { } + } + + var _c: U + + var pair: (T, U) { + init(initialValue) initializes(_a, _c) { + _a = initialValue.0 + _c = initialValue.1 + } + + get { (_a, _c) } + set { } + } + } + + let test2 = Test2(a: "a", pair: ("c", 2), b: 0) + print("memberwise-overlaps-2: \(test2)") + + struct Test3 { + var _a: T + var _b: Int + + var a: T { + init(initialValue) initializes(_a) { + _a = initialValue + } + + get { _a } + set { } + } + + var b: Int { + init(initialValue) initializes(_b) { + _b = initialValue + } + + get { _b } + set { } + } + + var _c: U + + var c: U { + init(initialValue) initializes(_c) { + _c = initialValue + } + + get { _c } + set { } + } + + var triple: (T, Int, U) { + init(initialValue) initializes(_a, _b, _c) { + _a = initialValue.0 + _b = initialValue.1 + _c = initialValue.2 + } + + get { (_a, _b, _c) } + set { } + } + } + + let test3 = Test3(a: "a", triple: ("b", 2, [1.0, 2.0]), b: 0, c: [1.0]) + print("memberwise-overlaps-3: \(test3)") +} + +test_memberwise_with_overlaps() +// CHECK: memberwise-overlaps-1: Test1>(_a: "b", _b: 1, c: [3.0]) +// CHECK-NEXT: memberwise-overlaps-2: Test2(_a: "c", _b: 0, _c: 2) +// CHECK-NEXT: memberwise-overlaps-3: Test3>(_a: "b", _b: 0, _c: [1.0]) diff --git a/test/SILOptimizer/init_accessor_raw_sil_lowering.swift b/test/SILOptimizer/init_accessor_raw_sil_lowering.swift index f12398a4d37d9..92816c958c5bb 100644 --- a/test/SILOptimizer/init_accessor_raw_sil_lowering.swift +++ b/test/SILOptimizer/init_accessor_raw_sil_lowering.swift @@ -2,6 +2,68 @@ // REQUIRES: asserts +// Makes sure that SILGen for memberwise initializer has destroy_addr for overlapping properties. +struct TestMemberwiseWithOverlap { + var _a: Int + var _b: Int + var _c: Int + + var a: Int { + init(initialValue) initializes(_a) { + _a = initialValue + } + + get { _a } + set { } + } + + var pair: (Int, Int) { + init(initialValue) initializes(_a, _b) { + _a = initialValue.0 + _b = initialValue.1 + } + + get { (_a, _b) } + set { } + } + + var c: Int { + init(initialValue) initializes(_b, _c) accesses(_a) { + _b = -1 + _c = initialValue + } + + get { _c } + set { } + } + + // CHECK-LABEL: sil hidden [ossa] @$s23assign_or_init_lowering25TestMemberwiseWithOverlapV1a4pair1cACSi_Si_SitSitcfC : $@convention(method) (Int, Int, Int, Int, @thin TestMemberwiseWithOverlap.Type) -> TestMemberwiseWithOverlap + // + // CHECK: [[SELF:%.*]] = alloc_stack $TestMemberwiseWithOverlap + // + // CHECK: [[A_REF:%.*]] = struct_element_addr [[SELF]] : $*TestMemberwiseWithOverlap, #TestMemberwiseWithOverlap._a + // CHECK: [[A_INIT:%.*]] = function_ref @$s23assign_or_init_lowering25TestMemberwiseWithOverlapV1aSivi : $@convention(thin) (Int) -> @out Int + // CHECK-NEXT: {{.*}} = apply [[A_INIT]]([[A_REF]], %0) : $@convention(thin) (Int) -> @out Int + // + // CHECK: [[A_REF:%.*]] = struct_element_addr [[SELF]] : $*TestMemberwiseWithOverlap, #TestMemberwiseWithOverlap._a + // CHECK-NEXT: destroy_addr [[A_REF]] : $*Int + // CHECK-NEXT: [[B_REF:%.*]] = struct_element_addr [[SELF]] : $*TestMemberwiseWithOverlap, #TestMemberwiseWithOverlap._b + // CHECK: [[PAIR_INIT:%.*]] = function_ref @$s23assign_or_init_lowering25TestMemberwiseWithOverlapV4pairSi_Sitvi : $@convention(thin) (Int, Int) -> (@out Int, @out Int) + // CHECK-NEXT: {{.*}} = apply [[PAIR_INIT]]([[A_REF]], [[B_REF]], %1, %2) : $@convention(thin) (Int, Int) -> (@out Int, @out Int) + // + // CHECK: [[B_REF:%.*]] = struct_element_addr [[SELF]] : $*TestMemberwiseWithOverlap, #TestMemberwiseWithOverlap._b + // CHECK-NEXT: destroy_addr [[B_REF]] : $*Int + // CHECK-NEXT: [[C_REF:%.*]] = struct_element_addr [[SELF]] : $*TestMemberwiseWithOverlap, #TestMemberwiseWithOverlap._c + // CHECK-NEXT: [[A_REF:%.*]] = struct_element_addr [[SELF]] : $*TestMemberwiseWithOverlap, #TestMemberwiseWithOverlap._a + // CHECK: [[C_INIT:%.*]] = function_ref @$s23assign_or_init_lowering25TestMemberwiseWithOverlapV1cSivi : $@convention(thin) (Int, @inout Int) -> (@out Int, @out Int) + // CHECK-NEXT: {{.*}} = apply [[C_INIT]]([[B_REF]], [[C_REF]], %3, [[A_REF]]) : $@convention(thin) (Int, @inout Int) -> (@out Int, @out Int) + // CHECK-NEXT: [[RESULT:%.*]] = load [trivial] [[SELF]] : $*TestMemberwiseWithOverlap + // CHECK-NEXT: dealloc_stack [[SELF]] : $*TestMemberwiseWithOverlap + // CHECK-NEXT: return [[RESULT]] : $TestMemberwiseWithOverlap +} + +_ = TestMemberwiseWithOverlap(a: 0, pair: (1, 2), c: 3) + struct Test1 { var _a: Int var _b: String diff --git a/test/decl/var/init_accessors.swift b/test/decl/var/init_accessors.swift index d7477f40898fb..988ad3dc539c0 100644 --- a/test/decl/var/init_accessors.swift +++ b/test/decl/var/init_accessors.swift @@ -221,3 +221,117 @@ func test_invalid_references() { func test() -> Int? { 42 } } } + +func test_memberwise_with_overlaps() { + struct Test1 { + var _a: T + var _b: Int + + var a: T { + init(initialValue) initializes(_a) { + _a = initialValue + } + + get { _a } + set { } + } + + var pair: (T, Int) { + init(initialValue) initializes(_a, _b) { + _a = initialValue.0 + _b = initialValue.1 + } + + get { (_a, _b) } + set { } + } + + var c: U + } + + _ = Test1(a: "a", pair: ("b", 1), c: [3.0]) // Ok + + struct Test2 { + var _a: T + var _b: Int + + var a: T { + init(initialValue) initializes(_a) { + _a = initialValue + } + + get { _a } + set { } + } + + var b: Int { + init(initialValue) initializes(_b) { + _b = initialValue + } + + get { _b } + set { } + } + + var _c: U + + var pair: (T, U) { + init(initialValue) initializes(_a, _c) { + _a = initialValue.0 + _c = initialValue.1 + } + + get { (_a, _c) } + set { } + } + } + + _ = Test2(a: "a", pair: ("c", 2), b: 0) // Ok + + struct Test3 { + var _a: T + var _b: Int + + var a: T { + init(initialValue) initializes(_a) { + _a = initialValue + } + + get { _a } + set { } + } + + var b: Int { + init(initialValue) initializes(_b) { + _b = initialValue + } + + get { _b } + set { } + } + + var _c: U + + var c: U { + init(initialValue) initializes(_c) { + _c = initialValue + } + + get { _c } + set { } + } + + var triple: (T, Int, U) { + init(initialValue) initializes(_a, _b, _c) { + _a = initialValue.0 + _b = initialValue.1 + _c = initialValue.2 + } + + get { (_a, _b, _c) } + set { } + } + } + + _ = Test3(a: "a", triple: ("b", 2, [1.0, 2.0]), b: 0, c: [1.0]) // Ok +} From 4f59538eba6628d842afbcc7fc9c2d3ba8878b54 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Fri, 9 Jun 2023 13:30:21 -0700 Subject: [PATCH 14/22] [Sema] InitAccessors: Diagnose situations when memberwise init cannot be synthesized If some of the properties with init accessors have out of order accesses diagnose that while checking whether memberwise init could be synthesized. --- include/swift/AST/DiagnosticsSema.def | 7 +++ lib/Sema/CodeSynthesis.cpp | 63 ++++++++++++++++++++++++++- test/decl/var/init_accessors.swift | 37 ++++++++++++++++ 3 files changed, 106 insertions(+), 1 deletion(-) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index d1360f1c88d03..e849dd17348d6 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -7351,6 +7351,13 @@ ERROR(init_accessor_invalid_member_ref,none, "refer to instance properties listed in 'initializes' and " "'accesses' attributes", (DeclNameRef)) +ERROR(cannot_synthesize_memberwise_due_to_property_init_order,none, + "cannot synthesize memberwise initializer", + ()) +NOTE(out_of_order_access_in_init_accessor,none, + "init accessor for %0 cannot access stored property %1 because it " + "is called before %1 is initialized", + (Identifier, Identifier)) #define UNDEFINE_DIAGNOSTIC_MACROS #include "DefineDiagnosticMacros.h" diff --git a/lib/Sema/CodeSynthesis.cpp b/lib/Sema/CodeSynthesis.cpp index 6279b44b2267c..db8bdc7c3c973 100644 --- a/lib/Sema/CodeSynthesis.cpp +++ b/lib/Sema/CodeSynthesis.cpp @@ -1317,6 +1317,12 @@ HasMemberwiseInitRequest::evaluate(Evaluator &evaluator, if (hasUserDefinedDesignatedInit(evaluator, decl)) return false; + std::multimap initializedViaAccessor; + decl->collectPropertiesInitializableByInitAccessors(initializedViaAccessor); + + llvm::SmallPtrSet initializedProperties; + llvm::SmallVector> invalidOrderings; + for (auto *member : decl->getMembers()) { if (auto *var = dyn_cast(member)) { // If this is a backing storage property for a property wrapper, @@ -1324,10 +1330,65 @@ HasMemberwiseInitRequest::evaluate(Evaluator &evaluator, if (var->getOriginalWrappedProperty()) continue; - if (var->isMemberwiseInitialized(/*preferDeclaredProperties=*/true)) + if (!var->isMemberwiseInitialized(/*preferDeclaredProperties=*/true)) + continue; + + // If init accessors are not involved, we are done. + if (initializedViaAccessor.empty()) return true; + + if (!initializedViaAccessor.count(var)) { + initializedProperties.insert(var); + continue; + } + + // Check whether use of init accessors results in access to uninitialized + // properties. + + for (auto iter = initializedViaAccessor.find(var); + iter != initializedViaAccessor.end(); ++iter) { + auto *initializerProperty = iter->second; + auto *initAccessor = + initializerProperty->getAccessor(AccessorKind::Init); + + // Make sure that all properties accessed by init accessor + // are previously initialized. + if (auto accessAttr = + initAccessor->getAttrs().getAttribute()) { + for (auto *property : accessAttr->getPropertyDecls(initAccessor)) { + if (!initializedProperties.count(property)) + invalidOrderings.push_back( + {initializerProperty, property->getName()}); + } + } + + // Record all of the properties initialized by calling init accessor. + if (auto initAttr = + initAccessor->getAttrs().getAttribute()) { + auto properties = initAttr->getPropertyDecls(initAccessor); + initializedProperties.insert(properties.begin(), properties.end()); + } + } } } + + if (invalidOrderings.empty()) + return !initializedProperties.empty(); + + { + auto &diags = decl->getASTContext().Diags; + + diags.diagnose( + decl, diag::cannot_synthesize_memberwise_due_to_property_init_order); + + for (const auto &invalid : invalidOrderings) { + auto *accessor = invalid.first->getAccessor(AccessorKind::Init); + diags.diagnose(accessor->getLoc(), + diag::out_of_order_access_in_init_accessor, + invalid.first->getName(), invalid.second); + } + } + return false; } diff --git a/test/decl/var/init_accessors.swift b/test/decl/var/init_accessors.swift index 988ad3dc539c0..0b1765d74c0cd 100644 --- a/test/decl/var/init_accessors.swift +++ b/test/decl/var/init_accessors.swift @@ -50,6 +50,7 @@ func test_use_of_initializes_accesses_on_non_inits() { } get { x } + set { } } var _y: String { @@ -63,6 +64,11 @@ func test_use_of_initializes_accesses_on_non_inits() { set(initialValue) accesses(x) {} // expected-error@-1 {{accesses(...) attribute could only be used with init accessors}} } + + init(x: Int, y: String) { + self.y = y + self._x = x + } } } @@ -103,6 +109,13 @@ func test_assignment_to_let_properties() { self.x = initialValue.0 // Ok self.y = initialValue.1 // Ok } + + get { (x, y) } + set { } + } + + init(x: Int, y: Int) { + self.point = (x, y) } } } @@ -116,6 +129,12 @@ func test_duplicate_and_computed_lazy_properties() { init(initialValue) initializes(_b, _a) accesses(_a) { // expected-error@-1 {{property '_a' cannot be both initialized and accessed}} } + + get { _a } + set { } + } + + init() { } } @@ -219,6 +238,8 @@ func test_invalid_references() { } func test() -> Int? { 42 } + + init() {} } } @@ -335,3 +356,19 @@ func test_memberwise_with_overlaps() { _ = Test3(a: "a", triple: ("b", 2, [1.0, 2.0]), b: 0, c: [1.0]) // Ok } + +func test_invalid_memberwise() { + struct Test1 { // expected-error {{cannot synthesize memberwise initializer}} + var _a: Int + var _b: Int + + var a: Int { + init(initialValue) initializes(_a) accesses(_b) { + // expected-note@-1 {{init accessor for 'a' cannot access stored property '_b' because it is called before '_b' is initialized}} + _a = initialValue + } + + get { _a } + } + } +} From cffc3fd73d099132cdbf496394a474cfec8015cd Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 13 Jun 2023 12:01:23 -0700 Subject: [PATCH 15/22] [Sema/SILGen] InitAccessors: Don't synthesize memberwise init if `initializes` intersect If some property is initializable by one than one init accessor let's not sythesize a memberwise initializer in that case because it's ambiguous what is the best init accessor to use. --- lib/SILGen/SILGenConstructor.cpp | 45 ++----- lib/Sema/CodeSynthesis.cpp | 25 ++-- test/Interpreter/init_accessors.swift | 122 ------------------ .../init_accessor_raw_sil_lowering.swift | 62 --------- test/decl/var/init_accessors.swift | 11 +- 5 files changed, 36 insertions(+), 229 deletions(-) diff --git a/lib/SILGen/SILGenConstructor.cpp b/lib/SILGen/SILGenConstructor.cpp index 1c510e0561103..945441484c9b4 100644 --- a/lib/SILGen/SILGenConstructor.cpp +++ b/lib/SILGen/SILGenConstructor.cpp @@ -275,22 +275,14 @@ static RValue maybeEmitPropertyWrapperInitFromValue( static void emitApplyOfInitAccessor(SILGenFunction &SGF, SILLocation loc, AccessorDecl *accessor, SILValue selfValue, - SILType selfTy, RValue &&initialValue, - llvm::SmallPtrSetImpl &initializedFields) { + SILType selfTy, RValue &&initialValue) { SmallVector arguments; auto emitFieldReference = [&](VarDecl *field, bool forInit = false) { auto fieldTy = selfTy.getFieldType(field, SGF.SGM.M, SGF.getTypeExpansionContext()); - auto fieldAddr = SGF.B.createStructElementAddr(loc, selfValue, field, - fieldTy.getAddressType()); - - // If another init accessor already initialized this field then we need - // to emit destroy before calling this accessor. - if (forInit && !initializedFields.insert(field).second) - SGF.B.emitDestroyAddr(loc, fieldAddr); - - return fieldAddr; + return SGF.B.createStructElementAddr(loc, selfValue, field, + fieldTy.getAddressType()); }; // First, let's emit all of the indirect results. @@ -411,35 +403,24 @@ static void emitImplicitValueConstructor(SILGenFunction &SGF, // Tracks all the init accessors we have emitted // because they can initialize more than one property. llvm::SmallPtrSet emittedInitAccessors; - // Tracks all the fields that have been already initialized - // via an init accessor. - llvm::SmallPtrSet fieldsInitializedViaAccessor; - auto elti = elements.begin(), eltEnd = elements.end(); for (VarDecl *field : decl->getStoredProperties()) { // Handle situations where this stored propery is initialized // via a call to an init accessor on some other property. - if (initializedViaAccessor.count(field)) { - for (auto iter = initializedViaAccessor.find(field); - iter != initializedViaAccessor.end(); ++iter) { - auto *initProperty = iter->second; - auto *initAccessor = initProperty->getAccessor(AccessorKind::Init); - - if (!emittedInitAccessors.insert(initAccessor).second) - continue; + if (initializedViaAccessor.count(field) == 1) { + auto *initProperty = initializedViaAccessor.find(field)->second; + auto *initAccessor = initProperty->getAccessor(AccessorKind::Init); - assert(elti != eltEnd && - "number of args does not match number of fields"); + if (!emittedInitAccessors.insert(initAccessor).second) + continue; - emitApplyOfInitAccessor(SGF, Loc, initAccessor, resultSlot, selfTy, - std::move(*elti), - fieldsInitializedViaAccessor); - ++elti; - } + assert(elti != eltEnd && + "number of args does not match number of fields"); - // After all init accessors are emitted let's move on to the next - // property. + emitApplyOfInitAccessor(SGF, Loc, initAccessor, resultSlot, selfTy, + std::move(*elti)); + ++elti; continue; } diff --git a/lib/Sema/CodeSynthesis.cpp b/lib/Sema/CodeSynthesis.cpp index db8bdc7c3c973..f918286f5f8ab 100644 --- a/lib/Sema/CodeSynthesis.cpp +++ b/lib/Sema/CodeSynthesis.cpp @@ -320,14 +320,10 @@ static ConstructorDecl *createImplicitConstructor(NominalTypeDecl *decl, continue; // Check whether this property could be initialized via init accessor. - if (initializedViaAccessor.count(var)) { - for (auto iter = initializedViaAccessor.find(var); - iter != initializedViaAccessor.end(); ++iter) { - auto *initializerProperty = iter->second; - - if (usedInitProperties.insert(initializerProperty).second) - createParameter(initializerProperty); - } + if (initializedViaAccessor.count(var) == 1) { + auto *initializerProperty = initializedViaAccessor.find(var)->second; + if (usedInitProperties.insert(initializerProperty).second) + createParameter(initializerProperty); } else { createParameter(var); } @@ -1337,9 +1333,20 @@ HasMemberwiseInitRequest::evaluate(Evaluator &evaluator, if (initializedViaAccessor.empty()) return true; - if (!initializedViaAccessor.count(var)) { + switch (initializedViaAccessor.count(var)) { + // Not covered by an init accessor. + case 0: initializedProperties.insert(var); continue; + + // Covered by a single init accessor. + case 1: + break; + + // Covered by one than one init accessor which means that we + // cannot synthesize memberwise initializer. + default: + return false; } // Check whether use of init accessors results in access to uninitialized diff --git a/test/Interpreter/init_accessors.swift b/test/Interpreter/init_accessors.swift index 765c6a9323e0e..69e5f52ea99fa 100644 --- a/test/Interpreter/init_accessors.swift +++ b/test/Interpreter/init_accessors.swift @@ -332,125 +332,3 @@ test_assignments() // CHECK-NEXT: test-assignments-1: (3, 42) // CHECK-NEXT: a-init-accessor: 0 // CHECK-NEXT: test-assignments-2: (0, 2) - -func test_memberwise_with_overlaps() { - struct Test1 { - var _a: T - var _b: Int - - var a: T { - init(initialValue) initializes(_a) { - _a = initialValue - } - - get { _a } - set { } - } - - var pair: (T, Int) { - init(initialValue) initializes(_a, _b) { - _a = initialValue.0 - _b = initialValue.1 - } - - get { (_a, _b) } - set { } - } - - var c: U - } - - let test1 = Test1(a: "a", pair: ("b", 1), c: [3.0]) - print("memberwise-overlaps-1: \(test1)") - - struct Test2 { - var _a: T - var _b: Int - - var a: T { - init(initialValue) initializes(_a) { - _a = initialValue - } - - get { _a } - set { } - } - - var b: Int { - init(initialValue) initializes(_b) { - _b = initialValue - } - - get { _b } - set { } - } - - var _c: U - - var pair: (T, U) { - init(initialValue) initializes(_a, _c) { - _a = initialValue.0 - _c = initialValue.1 - } - - get { (_a, _c) } - set { } - } - } - - let test2 = Test2(a: "a", pair: ("c", 2), b: 0) - print("memberwise-overlaps-2: \(test2)") - - struct Test3 { - var _a: T - var _b: Int - - var a: T { - init(initialValue) initializes(_a) { - _a = initialValue - } - - get { _a } - set { } - } - - var b: Int { - init(initialValue) initializes(_b) { - _b = initialValue - } - - get { _b } - set { } - } - - var _c: U - - var c: U { - init(initialValue) initializes(_c) { - _c = initialValue - } - - get { _c } - set { } - } - - var triple: (T, Int, U) { - init(initialValue) initializes(_a, _b, _c) { - _a = initialValue.0 - _b = initialValue.1 - _c = initialValue.2 - } - - get { (_a, _b, _c) } - set { } - } - } - - let test3 = Test3(a: "a", triple: ("b", 2, [1.0, 2.0]), b: 0, c: [1.0]) - print("memberwise-overlaps-3: \(test3)") -} - -test_memberwise_with_overlaps() -// CHECK: memberwise-overlaps-1: Test1>(_a: "b", _b: 1, c: [3.0]) -// CHECK-NEXT: memberwise-overlaps-2: Test2(_a: "c", _b: 0, _c: 2) -// CHECK-NEXT: memberwise-overlaps-3: Test3>(_a: "b", _b: 0, _c: [1.0]) diff --git a/test/SILOptimizer/init_accessor_raw_sil_lowering.swift b/test/SILOptimizer/init_accessor_raw_sil_lowering.swift index 92816c958c5bb..f12398a4d37d9 100644 --- a/test/SILOptimizer/init_accessor_raw_sil_lowering.swift +++ b/test/SILOptimizer/init_accessor_raw_sil_lowering.swift @@ -2,68 +2,6 @@ // REQUIRES: asserts -// Makes sure that SILGen for memberwise initializer has destroy_addr for overlapping properties. -struct TestMemberwiseWithOverlap { - var _a: Int - var _b: Int - var _c: Int - - var a: Int { - init(initialValue) initializes(_a) { - _a = initialValue - } - - get { _a } - set { } - } - - var pair: (Int, Int) { - init(initialValue) initializes(_a, _b) { - _a = initialValue.0 - _b = initialValue.1 - } - - get { (_a, _b) } - set { } - } - - var c: Int { - init(initialValue) initializes(_b, _c) accesses(_a) { - _b = -1 - _c = initialValue - } - - get { _c } - set { } - } - - // CHECK-LABEL: sil hidden [ossa] @$s23assign_or_init_lowering25TestMemberwiseWithOverlapV1a4pair1cACSi_Si_SitSitcfC : $@convention(method) (Int, Int, Int, Int, @thin TestMemberwiseWithOverlap.Type) -> TestMemberwiseWithOverlap - // - // CHECK: [[SELF:%.*]] = alloc_stack $TestMemberwiseWithOverlap - // - // CHECK: [[A_REF:%.*]] = struct_element_addr [[SELF]] : $*TestMemberwiseWithOverlap, #TestMemberwiseWithOverlap._a - // CHECK: [[A_INIT:%.*]] = function_ref @$s23assign_or_init_lowering25TestMemberwiseWithOverlapV1aSivi : $@convention(thin) (Int) -> @out Int - // CHECK-NEXT: {{.*}} = apply [[A_INIT]]([[A_REF]], %0) : $@convention(thin) (Int) -> @out Int - // - // CHECK: [[A_REF:%.*]] = struct_element_addr [[SELF]] : $*TestMemberwiseWithOverlap, #TestMemberwiseWithOverlap._a - // CHECK-NEXT: destroy_addr [[A_REF]] : $*Int - // CHECK-NEXT: [[B_REF:%.*]] = struct_element_addr [[SELF]] : $*TestMemberwiseWithOverlap, #TestMemberwiseWithOverlap._b - // CHECK: [[PAIR_INIT:%.*]] = function_ref @$s23assign_or_init_lowering25TestMemberwiseWithOverlapV4pairSi_Sitvi : $@convention(thin) (Int, Int) -> (@out Int, @out Int) - // CHECK-NEXT: {{.*}} = apply [[PAIR_INIT]]([[A_REF]], [[B_REF]], %1, %2) : $@convention(thin) (Int, Int) -> (@out Int, @out Int) - // - // CHECK: [[B_REF:%.*]] = struct_element_addr [[SELF]] : $*TestMemberwiseWithOverlap, #TestMemberwiseWithOverlap._b - // CHECK-NEXT: destroy_addr [[B_REF]] : $*Int - // CHECK-NEXT: [[C_REF:%.*]] = struct_element_addr [[SELF]] : $*TestMemberwiseWithOverlap, #TestMemberwiseWithOverlap._c - // CHECK-NEXT: [[A_REF:%.*]] = struct_element_addr [[SELF]] : $*TestMemberwiseWithOverlap, #TestMemberwiseWithOverlap._a - // CHECK: [[C_INIT:%.*]] = function_ref @$s23assign_or_init_lowering25TestMemberwiseWithOverlapV1cSivi : $@convention(thin) (Int, @inout Int) -> (@out Int, @out Int) - // CHECK-NEXT: {{.*}} = apply [[C_INIT]]([[B_REF]], [[C_REF]], %3, [[A_REF]]) : $@convention(thin) (Int, @inout Int) -> (@out Int, @out Int) - // CHECK-NEXT: [[RESULT:%.*]] = load [trivial] [[SELF]] : $*TestMemberwiseWithOverlap - // CHECK-NEXT: dealloc_stack [[SELF]] : $*TestMemberwiseWithOverlap - // CHECK-NEXT: return [[RESULT]] : $TestMemberwiseWithOverlap -} - -_ = TestMemberwiseWithOverlap(a: 0, pair: (1, 2), c: 3) - struct Test1 { var _a: Int var _b: String diff --git a/test/decl/var/init_accessors.swift b/test/decl/var/init_accessors.swift index 0b1765d74c0cd..ae44b7ff82ff1 100644 --- a/test/decl/var/init_accessors.swift +++ b/test/decl/var/init_accessors.swift @@ -243,7 +243,7 @@ func test_invalid_references() { } } -func test_memberwise_with_overlaps() { +func test_memberwise_with_overlaps_dont_synthesize_inits() { struct Test1 { var _a: T var _b: Int @@ -270,7 +270,8 @@ func test_memberwise_with_overlaps() { var c: U } - _ = Test1(a: "a", pair: ("b", 1), c: [3.0]) // Ok + _ = Test1(a: "a", pair: ("b", 1), c: [3.0]) + // expected-error@-1 {{'Test1' cannot be constructed because it has no accessible initializers}} struct Test2 { var _a: T @@ -307,7 +308,8 @@ func test_memberwise_with_overlaps() { } } - _ = Test2(a: "a", pair: ("c", 2), b: 0) // Ok + _ = Test2(a: "a", pair: ("c", 2), b: 0) + // expected-error@-1 {{'Test2' cannot be constructed because it has no accessible initializers}} struct Test3 { var _a: T @@ -354,7 +356,8 @@ func test_memberwise_with_overlaps() { } } - _ = Test3(a: "a", triple: ("b", 2, [1.0, 2.0]), b: 0, c: [1.0]) // Ok + _ = Test3(a: "a", triple: ("b", 2, [1.0, 2.0]), b: 0, c: [1.0]) + // expected-error@-1 {{'Test3' cannot be constructed because it has no accessible initializers}} } func test_invalid_memberwise() { From 6ca9728079585aaa961c32f15c9073f7e92bd2e1 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 13 Jun 2023 13:50:08 -0700 Subject: [PATCH 16/22] [AST] InitAccessors: Mark properties that init other properties as memberwise initializable --- lib/AST/Decl.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index 47dd732090d67..6373df6bb389b 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -7132,6 +7132,12 @@ bool VarDecl::isMemberwiseInitialized(bool preferDeclaredProperties) const { isBackingStorageForDeclaredProperty(this)) return false; + // If this is a computed property with `init` accessor, it's + // memberwise initializable when it could be used to initialize + // other stored properties. + if (auto *init = getAccessor(AccessorKind::Init)) + return init->getAttrs().hasAttribute(); + // If this is a computed property, it's not memberwise initialized unless // the caller has asked for the declared properties and it is either a // `lazy` property or a property with an attached wrapper. From 34c8cf60f18bbe8dbe9a28c870b4ef1a3cfa363f Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 13 Jun 2023 13:56:30 -0700 Subject: [PATCH 17/22] [Sema/SILGen] InitAccessors: Memberwise initializers with init accessors should follow field order Skip stored properties that are initialized via init accessors and emit parameters/initializations in field order which allows us to cover more use-cases. --- lib/SILGen/SILGenConstructor.cpp | 42 +++++++++------ lib/Sema/CodeSynthesis.cpp | 70 +++++++++++-------------- test/Interpreter/init_accessors.swift | 61 ++++++++++++++++++++++ test/decl/var/init_accessors.swift | 73 ++++++++++++++++++++++++++- 4 files changed, 187 insertions(+), 59 deletions(-) diff --git a/lib/SILGen/SILGenConstructor.cpp b/lib/SILGen/SILGenConstructor.cpp index 945441484c9b4..8631323e84b30 100644 --- a/lib/SILGen/SILGenConstructor.cpp +++ b/lib/SILGen/SILGenConstructor.cpp @@ -400,29 +400,39 @@ static void emitImplicitValueConstructor(SILGenFunction &SGF, // If we have an indirect return slot, initialize it in-place. if (resultSlot) { - // Tracks all the init accessors we have emitted - // because they can initialize more than one property. - llvm::SmallPtrSet emittedInitAccessors; auto elti = elements.begin(), eltEnd = elements.end(); - for (VarDecl *field : decl->getStoredProperties()) { + + llvm::SmallPtrSet storedProperties; + { + auto properties = decl->getStoredProperties(); + storedProperties.insert(properties.begin(), properties.end()); + } + + for (auto *member : decl->getMembers()) { + auto *field = dyn_cast(member); + if (!field) + continue; + + if (initializedViaAccessor.count(field)) + continue; // Handle situations where this stored propery is initialized // via a call to an init accessor on some other property. - if (initializedViaAccessor.count(field) == 1) { - auto *initProperty = initializedViaAccessor.find(field)->second; - auto *initAccessor = initProperty->getAccessor(AccessorKind::Init); - - if (!emittedInitAccessors.insert(initAccessor).second) + if (auto *initAccessor = field->getAccessor(AccessorKind::Init)) { + if (field->isMemberwiseInitialized(/*preferDeclaredProperties=*/true)) { + assert(elti != eltEnd && + "number of args does not match number of fields"); + + emitApplyOfInitAccessor(SGF, Loc, initAccessor, resultSlot, selfTy, + std::move(*elti)); + ++elti; continue; + } + } - assert(elti != eltEnd && - "number of args does not match number of fields"); - - emitApplyOfInitAccessor(SGF, Loc, initAccessor, resultSlot, selfTy, - std::move(*elti)); - ++elti; + // If this is not one of the stored properties, let's move on. + if (!storedProperties.count(field)) continue; - } auto fieldTy = selfTy.getFieldType(field, SGF.SGM.M, SGF.getTypeExpansionContext()); diff --git a/lib/Sema/CodeSynthesis.cpp b/lib/Sema/CodeSynthesis.cpp index f918286f5f8ab..364c3cbade25d 100644 --- a/lib/Sema/CodeSynthesis.cpp +++ b/lib/Sema/CodeSynthesis.cpp @@ -303,14 +303,6 @@ static ConstructorDecl *createImplicitConstructor(NominalTypeDecl *decl, std::multimap initializedViaAccessor; decl->collectPropertiesInitializableByInitAccessors(initializedViaAccessor); - auto createParameter = [&](VarDecl *property) { - accessLevel = std::min(accessLevel, property->getFormalAccess()); - params.push_back(createMemberwiseInitParameter(decl, Loc, property)); - }; - - // A single property could be used to initialize N other stored - // properties via a call to its init accessor. - llvm::SmallPtrSet usedInitProperties; for (auto member : decl->getMembers()) { auto var = dyn_cast(member); if (!var) @@ -319,14 +311,11 @@ static ConstructorDecl *createImplicitConstructor(NominalTypeDecl *decl, if (!var->isMemberwiseInitialized(/*preferDeclaredProperties=*/true)) continue; - // Check whether this property could be initialized via init accessor. - if (initializedViaAccessor.count(var) == 1) { - auto *initializerProperty = initializedViaAccessor.find(var)->second; - if (usedInitProperties.insert(initializerProperty).second) - createParameter(initializerProperty); - } else { - createParameter(var); - } + if (initializedViaAccessor.count(var)) + continue; + + accessLevel = std::min(accessLevel, var->getFormalAccess()); + params.push_back(createMemberwiseInitParameter(decl, Loc, var)); } } else if (ICK == ImplicitConstructorKind::DefaultDistributedActor) { auto classDecl = dyn_cast(decl); @@ -1333,48 +1322,47 @@ HasMemberwiseInitRequest::evaluate(Evaluator &evaluator, if (initializedViaAccessor.empty()) return true; - switch (initializedViaAccessor.count(var)) { - // Not covered by an init accessor. - case 0: - initializedProperties.insert(var); - continue; - - // Covered by a single init accessor. - case 1: - break; - - // Covered by one than one init accessor which means that we - // cannot synthesize memberwise initializer. - default: - return false; - } - // Check whether use of init accessors results in access to uninitialized // properties. - for (auto iter = initializedViaAccessor.find(var); - iter != initializedViaAccessor.end(); ++iter) { - auto *initializerProperty = iter->second; - auto *initAccessor = - initializerProperty->getAccessor(AccessorKind::Init); - + if (auto *initAccessor = var->getAccessor(AccessorKind::Init)) { // Make sure that all properties accessed by init accessor // are previously initialized. if (auto accessAttr = - initAccessor->getAttrs().getAttribute()) { + initAccessor->getAttrs().getAttribute()) { for (auto *property : accessAttr->getPropertyDecls(initAccessor)) { if (!initializedProperties.count(property)) invalidOrderings.push_back( - {initializerProperty, property->getName()}); + {var, property->getName()}); } } // Record all of the properties initialized by calling init accessor. if (auto initAttr = - initAccessor->getAttrs().getAttribute()) { + initAccessor->getAttrs().getAttribute()) { auto properties = initAttr->getPropertyDecls(initAccessor); initializedProperties.insert(properties.begin(), properties.end()); } + + continue; + } + + switch (initializedViaAccessor.count(var)) { + // Not covered by an init accessor. + case 0: + initializedProperties.insert(var); + continue; + + // Covered by a single init accessor, we'll handle that + // once we get to the property with init accessor. + case 1: + continue; + + // Covered by more than one init accessor which means that we + // cannot synthesize memberwise initializer due to intersecting + // initializations. + default: + return false; } } } diff --git a/test/Interpreter/init_accessors.swift b/test/Interpreter/init_accessors.swift index 69e5f52ea99fa..df8be55a75d2f 100644 --- a/test/Interpreter/init_accessors.swift +++ b/test/Interpreter/init_accessors.swift @@ -332,3 +332,64 @@ test_assignments() // CHECK-NEXT: test-assignments-1: (3, 42) // CHECK-NEXT: a-init-accessor: 0 // CHECK-NEXT: test-assignments-2: (0, 2) + +func test_memberwise_ordering() { + struct Test1 { + var _a: Int + var _b: Int + + var a: Int { + init(initialValue) initializes(_a) accesses(_b) { + _a = initialValue + } + + get { _a } + set { } + } + } + + let test1 = Test1(_b: 42, a: 0) + print("test-memberwise-ordering-1: \(test1)") + + struct Test2 { + var _a: Int + + var pair: (Int, Int) { + init(initialValue) initializes(_a, _b) { + _a = initialValue.0 + _b = initialValue.1 + } + + get { (_a, _b) } + set { } + } + + var _b: Int + } + + let test2 = Test2(pair: (-1, -2)) + print("test-memberwise-ordering-2: \(test2)") + + struct Test3 { + var _a: Int + var _b: Int + + var pair: (Int, Int) { + init(initialValue) accesses(_a, _b) { + } + + get { (_a, _b) } + set { } + } + + var _c: Int + } + + let test3 = Test3(_a: 1, _b: 2, _c: 3) + print("test-memberwise-ordering-3: \(test3)") +} + +test_memberwise_ordering() +// CHECK: test-memberwise-ordering-1: Test1(_a: 0, _b: 42) +// CHECK-NEXT: test-memberwise-ordering-2: Test2(_a: -1, _b: -2) +// CHECK-NEXT: test-memberwise-ordering-3: Test3(_a: 1, _b: 2, _c: 3) diff --git a/test/decl/var/init_accessors.swift b/test/decl/var/init_accessors.swift index ae44b7ff82ff1..6d8633bac190f 100644 --- a/test/decl/var/init_accessors.swift +++ b/test/decl/var/init_accessors.swift @@ -360,11 +360,26 @@ func test_memberwise_with_overlaps_dont_synthesize_inits() { // expected-error@-1 {{'Test3' cannot be constructed because it has no accessible initializers}} } -func test_invalid_memberwise() { - struct Test1 { // expected-error {{cannot synthesize memberwise initializer}} +func test_memberwise_ordering() { + struct Test1 { var _a: Int var _b: Int + var a: Int { + init(initialValue) initializes(_a) accesses(_b) { + _a = initialValue + } + + get { _a } + set { } + } + } + + _ = Test1(_b: 42, a: 0) // Ok + + struct Test2 { // expected-error {{cannot synthesize memberwise initializer}} + var _a: Int + var a: Int { init(initialValue) initializes(_a) accesses(_b) { // expected-note@-1 {{init accessor for 'a' cannot access stored property '_b' because it is called before '_b' is initialized}} @@ -373,5 +388,59 @@ func test_invalid_memberwise() { get { _a } } + + var _b: Int } + + struct Test3 { + var _a: Int + + var pair: (Int, Int) { + init(initialValue) initializes(_a, _b) { + _a = initialValue.0 + _b = initialValue.1 + } + + get { (_a, _b) } + set { } + } + + var _b: Int + } + + _ = Test3(pair: (0, 1)) // Ok + + struct Test4 { + var _a: Int + var _b: Int + + var pair: (Int, Int) { + init(initalValue) accesses(_a, _b) { + } + + get { (_a, _b) } + set { } + } + + var _c: Int + } + + _ = Test4(_a: 0, _b: 1, _c: 2) // Ok + + struct Test5 { + var _a: Int + var _b: Int + + var c: Int { + init(initalValue) initializes(_c) accesses(_a, _b) { + } + + get { _c } + set { } + } + + var _c: Int + } + + _ = Test5(_a: 0, _b: 1, c: 2) // Ok } From 6758fdb51864609e2bc5d50824baa7b31e931115 Mon Sep 17 00:00:00 2001 From: Becca Royal-Gordon Date: Tue, 13 Jun 2023 15:22:45 -0700 Subject: [PATCH 18/22] Diagnose conformances on @objcImpl extensions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit @objcImpl extensions aren’t allowed to declare new conformances; instead, they should either be declared in the header or in an ordinary extensions. (If they were permitted, they’d be ignored.) Fixes rdar://110669366. --- include/swift/AST/DiagnosticsSema.def | 5 +++++ lib/Sema/TypeCheckDeclObjC.cpp | 12 ++++++++++++ test/decl/ext/Inputs/objc_implementation.h | 3 +++ test/decl/ext/objc_implementation.swift | 6 +++++- 4 files changed, 25 insertions(+), 1 deletion(-) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 98a58ce0f3f02..3101bcae672c5 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -1676,6 +1676,11 @@ ERROR(attr_objc_implementation_category_not_found,none, NOTE(attr_objc_implementation_fixit_remove_category_name,none, "remove arguments to implement the main '@interface' for this class", ()) +ERROR(attr_objc_implementation_no_conformance,none, + "'@_objcImplementation' extension cannot add conformance to %0; " + "add this conformance %select{with an ordinary extension|" + "in the Objective-C header}1", + (Type, bool)) ERROR(member_of_objc_implementation_not_objc_or_final,none, "%0 %1 does not match any %0 declared in the headers for %2; did you use " diff --git a/lib/Sema/TypeCheckDeclObjC.cpp b/lib/Sema/TypeCheckDeclObjC.cpp index 2a0972ab954d7..899221483cabe 100644 --- a/lib/Sema/TypeCheckDeclObjC.cpp +++ b/lib/Sema/TypeCheckDeclObjC.cpp @@ -2899,6 +2899,18 @@ class ObjCImplementationChecker { { assert(!ext->hasClangNode() && "passed interface, not impl, to checker"); + // Conformances are declared exclusively in the interface, so diagnose any + // in the implementation right away. + for (auto &inherited : ext->getInherited()) { + bool isImportedProtocol = false; + if (auto protoNominal = inherited.getType()->getAnyNominal()) + isImportedProtocol = protoNominal->hasClangNode(); + + diagnose(inherited.getLoc(), + diag::attr_objc_implementation_no_conformance, + inherited.getType(), isImportedProtocol); + } + // Did we actually match this extension to an interface? (In invalid code, // we might not have.) auto interfaceDecl = ext->getImplementedObjCDecl(); diff --git a/test/decl/ext/Inputs/objc_implementation.h b/test/decl/ext/Inputs/objc_implementation.h index f6fc4856df29f..c4798088dd528 100644 --- a/test/decl/ext/Inputs/objc_implementation.h +++ b/test/decl/ext/Inputs/objc_implementation.h @@ -142,3 +142,6 @@ struct ObjCStruct { int foo; }; + +@protocol EmptyObjCProto +@end diff --git a/test/decl/ext/objc_implementation.swift b/test/decl/ext/objc_implementation.swift index ba0cba881b161..053024e235a5d 100644 --- a/test/decl/ext/objc_implementation.swift +++ b/test/decl/ext/objc_implementation.swift @@ -1,13 +1,17 @@ // RUN: %target-typecheck-verify-swift -import-objc-header %S/Inputs/objc_implementation.h // REQUIRES: objc_interop -@_objcImplementation extension ObjCClass { +protocol EmptySwiftProto {} + +@_objcImplementation extension ObjCClass: EmptySwiftProto, EmptyObjCProto { // expected-note@-1 {{previously implemented by extension here}} // expected-error@-2 {{extension for main class interface should provide implementation for instance method 'method(fromHeader4:)'}} // expected-error@-3 {{extension for main class interface should provide implementation for property 'propertyFromHeader9'}} // FIXME: give better diagnostic expected-error@-4 {{extension for main class interface should provide implementation for property 'propertyFromHeader8'}} // FIXME: give better diagnostic expected-error@-5 {{extension for main class interface should provide implementation for property 'propertyFromHeader7'}} // FIXME: give better diagnostic expected-error@-6 {{extension for main class interface should provide implementation for instance method 'method(fromHeader3:)'}} + // expected-error@-7 {{'@_objcImplementation' extension cannot add conformance to 'EmptySwiftProto'; add this conformance with an ordinary extension}} + // expected-error@-8 {{'@_objcImplementation' extension cannot add conformance to 'EmptyObjCProto'; add this conformance in the Objective-C header}} func method(fromHeader1: CInt) { // OK, provides an implementation for the header's method. From dd7acb1739d9e9c2cad0f2097a73e0cf0d262c9f Mon Sep 17 00:00:00 2001 From: Alastair Houghton Date: Wed, 14 Jun 2023 13:40:51 +0100 Subject: [PATCH 19/22] [Backtracing][Linux] Properly align the stacks. We have two stacks that we set up during crash handling, both of which were potentially misaligned. This only tripped us up on Ubuntu 20.04 on aarch64 - my guess is that maybe `clone()` aligns the stack pointer on newer kernels which is why we didn't see it on 22.04. rdar://110743884 --- stdlib/public/runtime/Backtrace.cpp | 2 +- stdlib/public/runtime/CrashHandlerLinux.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/stdlib/public/runtime/Backtrace.cpp b/stdlib/public/runtime/Backtrace.cpp index b30a8866ad171..53d663958a36d 100644 --- a/stdlib/public/runtime/Backtrace.cpp +++ b/stdlib/public/runtime/Backtrace.cpp @@ -796,7 +796,7 @@ struct spawn_info { int memserver; }; -uint8_t spawn_stack[4096]; +uint8_t spawn_stack[4096] __attribute__((aligned(SWIFT_PAGE_SIZE))); int do_spawn(void *ptr) { diff --git a/stdlib/public/runtime/CrashHandlerLinux.cpp b/stdlib/public/runtime/CrashHandlerLinux.cpp index 526b7a7a3c04e..e737a64954c90 100644 --- a/stdlib/public/runtime/CrashHandlerLinux.cpp +++ b/stdlib/public/runtime/CrashHandlerLinux.cpp @@ -607,7 +607,7 @@ wait_paused(uint32_t expected, const struct timespec *timeout) We don't want to require CAP_SYS_PTRACE because we're potentially being used inside of a Docker container, which won't have that enabled. */ -char memserver_stack[4096]; +char memserver_stack[4096] __attribute__((aligned(SWIFT_PAGE_SIZE))); char memserver_buffer[4096]; int memserver_fd; bool memserver_has_ptrace; From 110f428780689bfcf5cf5bc3db7f804205256929 Mon Sep 17 00:00:00 2001 From: Mike Ash Date: Tue, 6 Jun 2023 14:12:31 -0400 Subject: [PATCH 20/22] [Runtime] Add tracing for section scans. Section scans (for metadata, protocols, etc.) can be costly. This change adds tracing calls to those scans so we can more easily see how much time is spent in these scans and where they're initiated. This adds an os_signpost implementation controlled by SWIFT_STDLIB_TRACING, and a default empty implementation for when that's disabled. rdar://110266743 --- stdlib/cmake/modules/AddSwiftStdlib.cmake | 6 +- stdlib/cmake/modules/StdlibOptions.cmake | 7 + stdlib/public/runtime/AccessibleFunction.cpp | 5 +- stdlib/public/runtime/CMakeLists.txt | 1 + stdlib/public/runtime/Demangle.cpp | 3 + stdlib/public/runtime/MetadataLookup.cpp | 9 +- stdlib/public/runtime/ProtocolConformance.cpp | 10 +- stdlib/public/runtime/Tracing.cpp | 39 ++++ stdlib/public/runtime/Tracing.h | 190 ++++++++++++++++++ utils/build-script-impl | 7 + .../products/minimalstdlib.py | 1 + 11 files changed, 273 insertions(+), 5 deletions(-) create mode 100644 stdlib/public/runtime/Tracing.cpp create mode 100644 stdlib/public/runtime/Tracing.h diff --git a/stdlib/cmake/modules/AddSwiftStdlib.cmake b/stdlib/cmake/modules/AddSwiftStdlib.cmake index d1e5053b8f267..5f2415f87b025 100644 --- a/stdlib/cmake/modules/AddSwiftStdlib.cmake +++ b/stdlib/cmake/modules/AddSwiftStdlib.cmake @@ -417,6 +417,10 @@ function(_add_target_variant_c_compile_flags) list(APPEND result "-DSWIFT_STDLIB_ENABLE_UNICODE_DATA") endif() + if(SWIFT_STDLIB_TRACING) + list(APPEND result "-DSWIFT_STDLIB_TRACING") + endif() + if(SWIFT_STDLIB_CONCURRENCY_TRACING) list(APPEND result "-DSWIFT_STDLIB_CONCURRENCY_TRACING") endif() @@ -897,7 +901,7 @@ function(add_swift_target_library_single target name) # Define availability macros. foreach(def ${SWIFT_STDLIB_AVAILABILITY_DEFINITIONS}) - list(APPEND SWIFTLIB_SINGLE_SWIFT_COMPILE_FLAGS "-Xfrontend" "-define-availability" "-Xfrontend" "${def}") + list(APPEND SWIFTLIB_SINGLE_SWIFT_COMPILE_FLAGS "-Xfrontend" "-define-availability" "-Xfrontend" "${def}") endforeach() # Enable -target-min-inlining-version diff --git a/stdlib/cmake/modules/StdlibOptions.cmake b/stdlib/cmake/modules/StdlibOptions.cmake index 3733ee1e841e7..2e946e7cdb927 100644 --- a/stdlib/cmake/modules/StdlibOptions.cmake +++ b/stdlib/cmake/modules/StdlibOptions.cmake @@ -221,11 +221,18 @@ set(SWIFT_STDLIB_ENABLE_LTO OFF CACHE STRING "Build Swift stdlib with LTO. One option only affects the standard library and runtime, not tools.") if("${SWIFT_HOST_VARIANT_SDK}" IN_LIST SWIFT_DARWIN_PLATFORMS) + set(SWIFT_STDLIB_TRACING_default TRUE) set(SWIFT_STDLIB_CONCURRENCY_TRACING_default TRUE) else() + set(SWIFT_STDLIB_TRACING_default FALSE) set(SWIFT_STDLIB_CONCURRENCY_TRACING_default FALSE) endif() +option(SWIFT_STDLIB_TRACING + "Enable tracing in the runtime; assumes the presence of os_log(3) + and the os_signpost(3) API." + "${SWIFT_STDLIB_TRACING_default}") + option(SWIFT_STDLIB_CONCURRENCY_TRACING "Enable concurrency tracing in the runtime; assumes the presence of os_log(3) and the os_signpost(3) API." diff --git a/stdlib/public/runtime/AccessibleFunction.cpp b/stdlib/public/runtime/AccessibleFunction.cpp index e71cf09206af8..41b4d24589b11 100644 --- a/stdlib/public/runtime/AccessibleFunction.cpp +++ b/stdlib/public/runtime/AccessibleFunction.cpp @@ -21,6 +21,7 @@ #include "swift/Runtime/AccessibleFunction.h" #include "swift/Runtime/Concurrent.h" #include "swift/Runtime/Metadata.h" +#include "Tracing.h" #include #include @@ -120,12 +121,14 @@ void swift::addImageAccessibleFunctionsBlockCallback( static const AccessibleFunctionRecord * _searchForFunctionRecord(AccessibleFunctionsState &S, llvm::StringRef name) { + auto traceState = runtime::trace::accessible_function_scan_begin(name); + for (const auto §ion : S.SectionsToScan.snapshot()) { for (auto &record : section) { auto recordName = swift::Demangle::makeSymbolicMangledNameStringRef(record.Name.get()); if (recordName == name) - return &record; + return traceState.end(&record); } } return nullptr; diff --git a/stdlib/public/runtime/CMakeLists.txt b/stdlib/public/runtime/CMakeLists.txt index 379f72b010a72..3771d972d5400 100644 --- a/stdlib/public/runtime/CMakeLists.txt +++ b/stdlib/public/runtime/CMakeLists.txt @@ -76,6 +76,7 @@ set(swift_runtime_sources SwiftDtoa.cpp SwiftTLSContext.cpp ThreadingError.cpp + Tracing.cpp AccessibleFunction.cpp Win32.cpp) diff --git a/stdlib/public/runtime/Demangle.cpp b/stdlib/public/runtime/Demangle.cpp index 8a8827ec9767f..7a585ccc43f4a 100644 --- a/stdlib/public/runtime/Demangle.cpp +++ b/stdlib/public/runtime/Demangle.cpp @@ -822,6 +822,9 @@ swift::_swift_buildDemanglingForMetadata(const Metadata *type, auto metatype = static_cast(type); auto instance = _swift_buildDemanglingForMetadata(metatype->InstanceType, Dem); + if (!instance) + return nullptr; + auto typeNode = Dem.createNode(Node::Kind::Type); typeNode->addChild(instance, Dem); auto node = Dem.createNode(Node::Kind::Metatype); diff --git a/stdlib/public/runtime/MetadataLookup.cpp b/stdlib/public/runtime/MetadataLookup.cpp index 557112086bd55..82f382d55976c 100644 --- a/stdlib/public/runtime/MetadataLookup.cpp +++ b/stdlib/public/runtime/MetadataLookup.cpp @@ -17,6 +17,7 @@ #include "../CompatibilityOverride/CompatibilityOverride.h" #include "ImageInspection.h" #include "Private.h" +#include "Tracing.h" #include "swift/ABI/TypeIdentity.h" #include "swift/Basic/Lazy.h" #include "swift/Demangling/Demangler.h" @@ -742,11 +743,13 @@ _searchTypeMetadataRecords(TypeMetadataPrivateState &T, return nullptr; #endif + auto traceState = runtime::trace::metadata_scan_begin(node); + for (auto §ion : T.SectionsToScan.snapshot()) { for (const auto &record : section) { if (auto context = record.getContextDescriptor()) { if (_contextDescriptorMatchesMangling(context, node)) { - return context; + return traceState.end(context); } } } @@ -970,11 +973,13 @@ void swift::swift_registerProtocols(const ProtocolRecord *begin, static const ProtocolDescriptor * _searchProtocolRecords(ProtocolMetadataPrivateState &C, NodePointer node) { + auto traceState = runtime::trace::protocol_scan_begin(node); + for (auto §ion : C.SectionsToScan.snapshot()) { for (const auto &record : section) { if (auto protocol = record.Protocol.getPointer()) { if (_contextDescriptorMatchesMangling(protocol, node)) - return protocol; + return traceState.end(protocol); } } } diff --git a/stdlib/public/runtime/ProtocolConformance.cpp b/stdlib/public/runtime/ProtocolConformance.cpp index 72e185d380745..47d4524091041 100644 --- a/stdlib/public/runtime/ProtocolConformance.cpp +++ b/stdlib/public/runtime/ProtocolConformance.cpp @@ -30,6 +30,7 @@ #include "../CompatibilityOverride/CompatibilityOverride.h" #include "ImageInspection.h" #include "Private.h" +#include "Tracing.h" #include #include @@ -1089,6 +1090,9 @@ swift_conformsToProtocolMaybeInstantiateSuperclasses( } }; + auto traceState = + runtime::trace::protocol_conformance_scan_begin(type, protocol); + auto snapshot = C.SectionsToScan.snapshot(); if (C.scanSectionsBackwards) { for (auto §ion : llvm::reverse(snapshot)) @@ -1125,6 +1129,8 @@ swift_conformsToProtocolMaybeInstantiateSuperclasses( } noteFinalMetadataState(superclassIterator.state); + traceState.end(foundWitness); + // If it's for a superclass or if we didn't find anything, then add an // authoritative entry for this type. if (foundType != type) @@ -1170,13 +1176,15 @@ swift_conformsToProtocolImpl(const Metadata *const type, const ContextDescriptor * swift::_searchConformancesByMangledTypeName(Demangle::NodePointer node) { + auto traceState = runtime::trace::protocol_conformance_scan_begin(node); + auto &C = Conformances.get(); for (auto §ion : C.SectionsToScan.snapshot()) { for (const auto &record : section) { if (auto ntd = record->getTypeDescriptor()) { if (_contextDescriptorMatchesMangling(ntd, node)) - return ntd; + return traceState.end(ntd); } } } diff --git a/stdlib/public/runtime/Tracing.cpp b/stdlib/public/runtime/Tracing.cpp new file mode 100644 index 0000000000000..8be15546fafdc --- /dev/null +++ b/stdlib/public/runtime/Tracing.cpp @@ -0,0 +1,39 @@ +//===--- Tracing.cpp - Support code for runtime tracing ------------*- C++ -*-// +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2014 - 2021 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// +// +// Support code for tracing events in the Swift runtime +// +//===----------------------------------------------------------------------===// + +#include "Tracing.h" + +#if SWIFT_STDLIB_TRACING + +#define SWIFT_LOG_SUBSYSTEM "com.apple.swift" +#define SWIFT_LOG_SECTION_SCAN_CATEGORY "SectionScan" + +namespace swift { +namespace runtime { +namespace trace { + +os_log_t ScanLog; +swift::once_t LogsToken; + +void setupLogs(void *unused) { + ScanLog = os_log_create(SWIFT_LOG_SUBSYSTEM, SWIFT_LOG_SECTION_SCAN_CATEGORY); +} + +} // namespace trace +} // namespace runtime +} // namespace swift + +#endif diff --git a/stdlib/public/runtime/Tracing.h b/stdlib/public/runtime/Tracing.h new file mode 100644 index 0000000000000..5714962d02ea1 --- /dev/null +++ b/stdlib/public/runtime/Tracing.h @@ -0,0 +1,190 @@ +//===--- Tracing.h - Support code for runtime tracing --------------*- C++ -*-// +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2014 - 2021 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// +// +// Support code for tracing events in the Swift runtime +// +//===----------------------------------------------------------------------===// + +#ifndef SWIFT_TRACING_H +#define SWIFT_TRACING_H + +#include "llvm/ADT/StringRef.h" +#include "swift/ABI/Metadata.h" +#include "swift/Demangling/Demangler.h" + +#if SWIFT_STDLIB_TRACING +#include + +#include "swift/Runtime/HeapObject.h" + +#define SWIFT_LOG_SECTION_SCAN "section_scan" + +namespace swift { +namespace runtime { +namespace trace { + +extern os_log_t ScanLog; +extern swift::once_t LogsToken; + +void setupLogs(void *unused); + +// Every function does ENSURE_LOGS() before making any os_signpost calls, so +// we can skip availability checking on all the individual calls. +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wunguarded-availability" +#pragma clang diagnostic ignored "-Wunguarded-availability-new" + +// Check a representative os_signpost function for NULL rather than doing a +// standard availability check, for better performance if the check doesn't get +// optimized out. +#define ENSURE_LOG(log) \ + do { \ + if (!SWIFT_RUNTIME_WEAK_CHECK(os_signpost_enabled)) \ + return {}; \ + swift::once(LogsToken, setupLogs, nullptr); \ + } while (0) + +/// A struct that captures the state of a scan tracing signpost. When the scan +/// is complete, call end() with the result of the scan. If the state struct +/// goes out of scope without calling end(), then it will automatically do the +/// equivalent of end(nullptr). +struct ScanTraceState { + os_signpost_id_t signpostID; + + bool ended = false; + + template + T *end(T *result) { + ended = true; + os_signpost_interval_end(ScanLog, signpostID, SWIFT_LOG_SECTION_SCAN, + "result=%p", result); + return result; + } + + ~ScanTraceState() { + if (!ended) + end((void *)nullptr); + } +}; + +static inline ScanTraceState +accessible_function_scan_begin(llvm::StringRef name) { + ENSURE_LOG(ScanLog); + + auto id = os_signpost_id_generate(ScanLog); + os_signpost_interval_begin(ScanLog, id, SWIFT_LOG_SECTION_SCAN, + "accessible function scan for '%.*s'", + (int)name.size(), name.data()); + return {id}; +} + +static inline ScanTraceState metadata_scan_begin(Demangle::NodePointer node) { + ENSURE_LOG(ScanLog); + + auto id = os_signpost_id_generate(ScanLog); + os_signpost_interval_begin(ScanLog, id, SWIFT_LOG_SECTION_SCAN, + "metadata scan for %s", + node ? nodeToString(node).c_str() : ""); + return {id}; +} + +static inline ScanTraceState +protocol_conformance_scan_begin(Demangle::NodePointer node) { + ENSURE_LOG(ScanLog); + + auto id = os_signpost_id_generate(ScanLog); + os_signpost_interval_begin(ScanLog, id, SWIFT_LOG_SECTION_SCAN, + "protocol conformance scan for %s", + node ? nodeToString(node).c_str() : ""); + return {id}; +} + +static inline ScanTraceState +protocol_conformance_scan_begin(const Metadata *type, + const ProtocolDescriptor *protocol) { + ENSURE_LOG(ScanLog); + + auto id = os_signpost_id_generate(ScanLog); + + // Check for enablement separately to avoid the potentially expensive + // swift_getTypeName call when tracing is not enabled. + if (os_signpost_enabled(ScanLog)) { + auto typeName = swift_getTypeName(type, /*qualified*/ true); + auto protoName = protocol ? protocol->Name.get() : ""; + os_signpost_interval_begin(ScanLog, id, SWIFT_LOG_SECTION_SCAN, + "protocol conformance scan for %.*s(%p): %s(%p)", + (int)typeName.length, typeName.data, type, + protoName, protocol); + } + return {id}; +} + +static inline ScanTraceState protocol_scan_begin(Demangle::NodePointer node) { + ENSURE_LOG(ScanLog); + + auto id = os_signpost_id_generate(ScanLog); + os_signpost_interval_begin(ScanLog, id, SWIFT_LOG_SECTION_SCAN, + "protocol scan for '%s'", + node ? nodeToString(node).c_str() : ""); + return {id}; +} + +#pragma clang diagnostic pop + +} // namespace trace +} // namespace runtime +} // namespace swift + +#else + +namespace swift { +namespace runtime { +namespace trace { + +struct ScanTraceState { + template + T *end(T *result) { + return result; + } +}; + +static inline ScanTraceState +accessible_function_scan_begin(llvm::StringRef name) { + return {}; +} + +static inline ScanTraceState metadata_scan_begin(Demangle::NodePointer node) { + return {}; +} + +static inline ScanTraceState +protocol_conformance_scan_begin(Demangle::NodePointer node) { + return {}; +} + +static inline ScanTraceState +protocol_conformance_scan_begin(const Metadata *type, + const ProtocolDescriptor *protocol) { + return {}; +} + +static inline ScanTraceState protocol_scan_begin(Demangle::NodePointer node) { + return {}; +} + +} // namespace trace +} // namespace runtime +} // namespace swift + +#endif + +#endif // SWIFT_TRACING_H diff --git a/utils/build-script-impl b/utils/build-script-impl index f3941c1427948..dafe4aeaff44c 100755 --- a/utils/build-script-impl +++ b/utils/build-script-impl @@ -2044,6 +2044,13 @@ for host in "${ALL_HOSTS[@]}"; do ) fi + if [[ "${SWIFT_STDLIB_TRACING}" ]] ; then + cmake_options=( + "${cmake_options[@]}" + -DSWIFT_STDLIB_TRACING:BOOL=$(true_false "${SWIFT_STDLIB_TRACING}") + ) + fi + if [[ "${SWIFT_STDLIB_CONCURRENCY_TRACING}" ]] ; then cmake_options=( "${cmake_options[@]}" diff --git a/utils/swift_build_support/swift_build_support/products/minimalstdlib.py b/utils/swift_build_support/swift_build_support/products/minimalstdlib.py index 1264f1a434d0e..8d70554d6fe10 100644 --- a/utils/swift_build_support/swift_build_support/products/minimalstdlib.py +++ b/utils/swift_build_support/swift_build_support/products/minimalstdlib.py @@ -116,6 +116,7 @@ def build(self, host_target): self.cmake_options.define( 'SWIFT_RUNTIME_STATIC_IMAGE_INSPECTION:BOOL', 'FALSE') self.cmake_options.define('SWIFT_STDLIB_BUILD_PRIVATE:BOOL', 'TRUE') + self.cmake_options.define('SWIFT_STDLIB_TRACING:BOOL', 'FALSE') self.cmake_options.define( 'SWIFT_STDLIB_CONCURRENCY_TRACING:BOOL', 'FALSE') self.cmake_options.define( From 8b48a0d3e09df169f01f56010772e1e5f9d96992 Mon Sep 17 00:00:00 2001 From: Dario Rexin Date: Wed, 14 Jun 2023 09:31:11 -0700 Subject: [PATCH 21/22] [Runtime+IRGen] Instantiate layout strings for generic multi payload enum (#66621) Instantiating layout strings for generic types reduces code size and is expected to improve performance of generic value witnesses. --- include/swift/Runtime/Enum.h | 6 + include/swift/Runtime/RuntimeFunctions.def | 17 +- lib/IRGen/GenEnum.cpp | 84 ++++++++ lib/IRGen/GenEnum.h | 6 + lib/IRGen/GenMeta.cpp | 44 +++- lib/IRGen/GenValueWitness.cpp | 3 + lib/IRGen/TypeLayout.cpp | 1 + stdlib/public/runtime/BytecodeLayouts.cpp | 72 ++++++- stdlib/public/runtime/BytecodeLayouts.h | 1 + stdlib/public/runtime/Enum.cpp | 198 ++++++++++++------ .../layout_string_witnesses_dynamic.swift | 6 +- 11 files changed, 362 insertions(+), 76 deletions(-) diff --git a/include/swift/Runtime/Enum.h b/include/swift/Runtime/Enum.h index 47cb97376e8d3..21d8b4901b369 100644 --- a/include/swift/Runtime/Enum.h +++ b/include/swift/Runtime/Enum.h @@ -111,6 +111,12 @@ void swift_initEnumMetadataMultiPayload(EnumMetadata *enumType, unsigned numPayloads, const TypeLayout * const *payloadTypes); +SWIFT_RUNTIME_EXPORT +void swift_initEnumMetadataMultiPayloadWithLayoutString(EnumMetadata *enumType, + EnumLayoutFlags flags, + unsigned numPayloads, + const Metadata * const *payloadTypes); + /// Return an integer value representing which case of a multi-payload /// enum is inhabited. /// diff --git a/include/swift/Runtime/RuntimeFunctions.def b/include/swift/Runtime/RuntimeFunctions.def index 5a9690b13ae79..8ab6e20a1d147 100644 --- a/include/swift/Runtime/RuntimeFunctions.def +++ b/include/swift/Runtime/RuntimeFunctions.def @@ -1276,7 +1276,8 @@ FUNCTION(InitStructMetadata, // void swift_initStructMetadataWithLayoutString(Metadata *structType, // StructLayoutFlags flags, // size_t numFields, -// Metadata * const *fieldTypes, +// uint8_t * const *fieldTypes, +// const uint8_t *fieldTags // uint32_t *fieldOffsets); FUNCTION(InitStructMetadataWithLayoutString, swift_initStructMetadataWithLayoutString, C_CC, AlwaysAvailable, @@ -1312,6 +1313,7 @@ FUNCTION(InitEnumMetadataSinglePayload, EFFECT(MetaData)) // void swift_initEnumMetadataMultiPayload(Metadata *enumType, +// EnumLayoutFlags layoutFlags, // size_t numPayloads, // TypeLayout * const *payloadTypes); FUNCTION(InitEnumMetadataMultiPayload, @@ -1322,6 +1324,19 @@ FUNCTION(InitEnumMetadataMultiPayload, ATTRS(NoUnwind, WillReturn), EFFECT(MetaData)) +// void +// swift_initEnumMetadataMultiPayloadWithLayoutString(Metadata *enumType, +// EnumLayoutFlags layoutFlags, +// size_t numPayloads, +// Metadata * const *payloadTypes); +FUNCTION(InitEnumMetadataMultiPayloadWithLayoutString, + swift_initEnumMetadataMultiPayloadWithLayoutString, + C_CC, AlwaysAvailable, + RETURNS(VoidTy), + ARGS(TypeMetadataPtrTy, SizeTy, SizeTy, TypeMetadataPtrPtrTy), + ATTRS(NoUnwind, WillReturn), + EFFECT(MetaData)) + // int swift_getEnumCaseMultiPayload(opaque_t *obj, Metadata *enumTy); FUNCTION(GetEnumCaseMultiPayload, swift_getEnumCaseMultiPayload, diff --git a/lib/IRGen/GenEnum.cpp b/lib/IRGen/GenEnum.cpp index bf4c6b16f374d..42542c7e3c240 100644 --- a/lib/IRGen/GenEnum.cpp +++ b/lib/IRGen/GenEnum.cpp @@ -663,6 +663,15 @@ namespace { metadata); } + void initializeMetadataWithLayoutString(IRGenFunction &IGF, + llvm::Value *metadata, + bool isVWTMutable, + SILType T, + MetadataDependencyCollector *collector) const override { + // Not yet supported on this type, so forward to regular method + initializeMetadata(IGF, metadata, isVWTMutable, T, collector); + } + bool mayHaveExtraInhabitants(IRGenModule &IGM) const override { // FIXME: Hold off on registering extra inhabitants for dynamic enums // until initializeMetadata handles them. @@ -964,6 +973,15 @@ namespace { // witness table initialization. } + void initializeMetadataWithLayoutString(IRGenFunction &IGF, + llvm::Value *metadata, + bool isVWTMutable, + SILType T, + MetadataDependencyCollector *collector) const override { + // No-payload enums are always fixed-size so never need dynamic value + // witness table initialization. + } + /// \group Required for SingleScalarTypeInfo llvm::Type *getScalarType() const { @@ -3189,6 +3207,15 @@ namespace { {metadata, flags, payloadLayout, emptyCasesVal}); } + void initializeMetadataWithLayoutString(IRGenFunction &IGF, + llvm::Value *metadata, + bool isVWTMutable, + SILType T, + MetadataDependencyCollector *collector) const override { + // Not yet supported on this type, so forward to regular method + initializeMetadata(IGF, metadata, isVWTMutable, T, collector); + } + /// \group Extra inhabitants // Extra inhabitants from the payload that we didn't use for our empty cases @@ -5259,6 +5286,36 @@ namespace { return firstAddr; } + llvm::Value *emitPayloadMetadataArray(IRGenFunction &IGF, SILType T, + MetadataDependencyCollector *collector) const { + auto numPayloads = ElementsWithPayload.size(); + auto metadataBufferTy = llvm::ArrayType::get(IGM.TypeMetadataPtrTy, + numPayloads); + auto metadataBuffer = IGF.createAlloca(metadataBufferTy, + IGM.getPointerAlignment(), + "payload_types"); + llvm::Value *firstAddr = nullptr; + for (unsigned i = 0; i < numPayloads; ++i) { + auto &elt = ElementsWithPayload[i]; + Address eltAddr = IGF.Builder.CreateStructGEP(metadataBuffer, i, + IGM.getPointerSize() * i); + if (i == 0) firstAddr = eltAddr.getAddress(); + + auto payloadTy = + T.getEnumElementType(elt.decl, IGF.getSILModule(), + IGF.IGM.getMaximalTypeExpansionContext()); + + auto request = DynamicMetadataRequest::getNonBlocking( + MetadataState::LayoutComplete, collector); + auto metadata = IGF.emitTypeMetadataRefForLayout(payloadTy, request); + + IGF.Builder.CreateStore(metadata, eltAddr); + } + assert(firstAddr && "Expected firstAddr to be assigned to"); + + return firstAddr; + } + void initializeMetadata(IRGenFunction &IGF, llvm::Value *metadata, bool isVWTMutable, @@ -5278,6 +5335,25 @@ namespace { {metadata, flags, numPayloadsVal, payloadLayoutArray}); } + void initializeMetadataWithLayoutString(IRGenFunction &IGF, + llvm::Value *metadata, + bool isVWTMutable, + SILType T, + MetadataDependencyCollector *collector) const override { + // Fixed-size enums don't need dynamic metadata initialization. + if (TIK >= Fixed) return; + + // Ask the runtime to set up the metadata record for a dynamic enum. + auto payloadLayoutArray = emitPayloadMetadataArray(IGF, T, collector); + auto numPayloadsVal = llvm::ConstantInt::get(IGM.SizeTy, + ElementsWithPayload.size()); + + auto flags = emitEnumLayoutFlags(IGM, isVWTMutable); + IGF.Builder.CreateCall( + IGM.getInitEnumMetadataMultiPayloadWithLayoutStringFunctionPointer(), + {metadata, flags, numPayloadsVal, payloadLayoutArray}); + } + /// \group Extra inhabitants // If we didn't use all of the available tag bit representations, offer @@ -5959,6 +6035,14 @@ namespace { llvm_unreachable("resilient enums cannot be defined"); } + void initializeMetadataWithLayoutString(IRGenFunction &IGF, + llvm::Value *metadata, + bool isVWTMutable, + SILType T, + MetadataDependencyCollector *collector) const override { + llvm_unreachable("resilient enums cannot be defined"); + } + /// \group Extra inhabitants bool mayHaveExtraInhabitants(IRGenModule &) const override { diff --git a/lib/IRGen/GenEnum.h b/lib/IRGen/GenEnum.h index c0ecbae1c3205..53f35c47c4311 100644 --- a/lib/IRGen/GenEnum.h +++ b/lib/IRGen/GenEnum.h @@ -384,6 +384,12 @@ class EnumImplStrategy { SILType T, MetadataDependencyCollector *collector) const = 0; + virtual void initializeMetadataWithLayoutString(IRGenFunction &IGF, + llvm::Value *metadata, + bool isVWTMutable, + SILType T, + MetadataDependencyCollector *collector) const = 0; + virtual bool mayHaveExtraInhabitants(IRGenModule &IGM) const = 0; // Only ever called for fixed types. diff --git a/lib/IRGen/GenMeta.cpp b/lib/IRGen/GenMeta.cpp index f8bb678447849..c46a4dac25f22 100644 --- a/lib/IRGen/GenMeta.cpp +++ b/lib/IRGen/GenMeta.cpp @@ -3003,16 +3003,17 @@ static void emitInitializeValueMetadata(IRGenFunction &IGF, MetadataDependencyCollector *collector) { auto &IGM = IGF.IGM; auto loweredTy = IGM.getLoweredType(nominalDecl->getDeclaredTypeInContext()); + bool useLayoutStrings = IGM.Context.LangOpts.hasFeature(Feature::LayoutStringValueWitnesses) && + IGM.Context.LangOpts.hasFeature( + Feature::LayoutStringValueWitnessesInstantiation) && + IGM.getOptions().EnableLayoutStringValueWitnesses && + IGM.getOptions().EnableLayoutStringValueWitnessesInstantiation; if (isa(nominalDecl)) { auto &fixedTI = IGM.getTypeInfo(loweredTy); if (isa(fixedTI)) return; - if (IGM.Context.LangOpts.hasFeature(Feature::LayoutStringValueWitnesses) && - IGM.Context.LangOpts.hasFeature( - Feature::LayoutStringValueWitnessesInstantiation) && - IGM.getOptions().EnableLayoutStringValueWitnesses && - IGM.getOptions().EnableLayoutStringValueWitnessesInstantiation) { + if (useLayoutStrings) { emitInitializeFieldOffsetVectorWithLayoutString(IGF, loweredTy, metadata, isVWTMutable, collector); } else { @@ -3021,9 +3022,16 @@ static void emitInitializeValueMetadata(IRGenFunction &IGF, } } else { assert(isa(nominalDecl)); + auto &strategy = getEnumImplStrategy(IGM, loweredTy); - strategy.initializeMetadata(IGF, metadata, isVWTMutable, loweredTy, - collector); + + if (useLayoutStrings) { + strategy.initializeMetadataWithLayoutString(IGF, metadata, isVWTMutable, + loweredTy, collector); + } else { + strategy.initializeMetadata(IGF, metadata, isVWTMutable, loweredTy, + collector); + } } } @@ -5721,10 +5729,28 @@ namespace { return {global, offset, structSize}; } + bool hasLayoutString() { + if (!IGM.Context.LangOpts.hasFeature( + Feature::LayoutStringValueWitnesses) || + !IGM.getOptions().EnableLayoutStringValueWitnesses) { + return false; + } + + auto &strategy = getEnumImplStrategy(IGM, getLoweredType()); + + return !!getLayoutString() || + (IGM.Context.LangOpts.hasFeature( + Feature::LayoutStringValueWitnessesInstantiation) && + IGM.getOptions().EnableLayoutStringValueWitnessesInstantiation && + (HasDependentVWT || HasDependentMetadata) && + !isa(IGM.getTypeInfo(getLoweredType())) && + strategy.getElementsWithPayload().size() > 1); + } + llvm::Constant *emitNominalTypeDescriptor() { return EnumContextDescriptorBuilder( IGM, Target, RequireMetadata, - /*hasLayoutString*/ !!getLayoutString()) + /*hasLayoutString*/ hasLayoutString()) .emit(); } @@ -5746,7 +5772,7 @@ namespace { bool hasCompletionFunction() { return !isa(IGM.getTypeInfo(getLoweredType())) || - !!getLayoutString(); + hasLayoutString(); } }; diff --git a/lib/IRGen/GenValueWitness.cpp b/lib/IRGen/GenValueWitness.cpp index 8768ba476c48d..886297dd948a3 100644 --- a/lib/IRGen/GenValueWitness.cpp +++ b/lib/IRGen/GenValueWitness.cpp @@ -891,6 +891,9 @@ bool isRuntimeInstatiatedLayoutString(IRGenModule &IGM, IGM.Context.LangOpts.hasFeature( Feature::LayoutStringValueWitnessesInstantiation) && IGM.getOptions().EnableLayoutStringValueWitnessesInstantiation) { + if (auto *enumEntry = typeLayoutEntry->getAsEnum()) { + return enumEntry->isMultiPayloadEnum(); + } return (typeLayoutEntry->isAlignedGroup() && !typeLayoutEntry->isFixedSize(IGM)); } diff --git a/lib/IRGen/TypeLayout.cpp b/lib/IRGen/TypeLayout.cpp index c89ea702061fe..5ef1e9fbd1f60 100644 --- a/lib/IRGen/TypeLayout.cpp +++ b/lib/IRGen/TypeLayout.cpp @@ -83,6 +83,7 @@ class LayoutStringBuilder { MultiPayloadEnumFN = 0x13, // reserved // MultiPayloadEnumFNResolved = 0x14, + // MultiPayloadEnumGeneric = 0x15, Skip = 0x80, // We may use the MSB as flag that a count follows, diff --git a/stdlib/public/runtime/BytecodeLayouts.cpp b/stdlib/public/runtime/BytecodeLayouts.cpp index 5e45145f10d0a..64550ce438d41 100644 --- a/stdlib/public/runtime/BytecodeLayouts.cpp +++ b/stdlib/public/runtime/BytecodeLayouts.cpp @@ -126,6 +126,11 @@ inline static bool handleNextRefCount(const Metadata *metadata, Handler::handleMultiPayloadEnumFN(metadata, typeLayout, offset, true, addrOffset, std::forward(params)...); + } else if (SWIFT_UNLIKELY(tag == + RefCountingKind::MultiPayloadEnumGeneric)) { + Handler::handleMultiPayloadEnumGeneric(metadata, typeLayout, offset, + addrOffset, + std::forward(params)...); } else { Handler::handleReference(tag, addrOffset, std::forward(params)...); } @@ -263,16 +268,46 @@ static void handleMultiPayloadEnumFN(const Metadata *metadata, getEnumTag = readRelativeFunctionPointer(typeLayout, offset); } - size_t numCases = readBytes(typeLayout, offset); + size_t numPayloads = readBytes(typeLayout, offset); size_t refCountBytes = readBytes(typeLayout, offset); size_t enumSize = readBytes(typeLayout, offset); unsigned enumTag = getEnumTag(addr + addrOffset); - if (enumTag < numCases) { + if (enumTag < numPayloads) { + size_t nestedOffset = offset + (enumTag * sizeof(size_t)); + size_t refCountOffset = readBytes(typeLayout, nestedOffset); + nestedOffset = offset + (numPayloads * sizeof(size_t)) + refCountOffset; + + uintptr_t nestedAddrOffset = addrOffset; + handleRefCounts<0, Handler>(metadata, typeLayout, nestedOffset, + nestedAddrOffset, addr, + std::forward(params)...); + } + + offset += refCountBytes + (numPayloads * sizeof(size_t)); + addrOffset += enumSize; +} + +template +static void handleMultiPayloadEnumGeneric(const Metadata *metadata, + const uint8_t *typeLayout, + size_t &offset, + uintptr_t &addrOffset, + uint8_t *addr, + Params... params) { + auto tagBytes = readBytes(typeLayout, offset); + auto numPayloads = readBytes(typeLayout, offset); + auto refCountBytes = readBytes(typeLayout, offset); + auto enumSize = readBytes(typeLayout, offset); + auto tagBytesOffset = enumSize - tagBytes; + + auto enumTag = readTagBytes(addr + addrOffset + tagBytesOffset, tagBytes); + + if (enumTag < numPayloads) { size_t nestedOffset = offset + (enumTag * sizeof(size_t)); size_t refCountOffset = readBytes(typeLayout, nestedOffset); - nestedOffset = offset + (numCases * sizeof(size_t)) + refCountOffset; + nestedOffset = offset + (numPayloads * sizeof(size_t)) + refCountOffset; uintptr_t nestedAddrOffset = addrOffset; handleRefCounts<0, Handler>(metadata, typeLayout, nestedOffset, @@ -280,7 +315,7 @@ static void handleMultiPayloadEnumFN(const Metadata *metadata, std::forward(params)...); } - offset += refCountBytes + (numCases * sizeof(size_t)); + offset += refCountBytes + (numPayloads * sizeof(size_t)); addrOffset += enumSize; } @@ -337,6 +372,15 @@ struct DestroyHandler { resolved, addrOffset, addr); } + static inline void handleMultiPayloadEnumGeneric(const Metadata *metadata, + const uint8_t *typeLayout, + size_t &offset, + uintptr_t &addrOffset, + uint8_t *addr) { + ::handleMultiPayloadEnumGeneric(metadata, typeLayout, offset, + addrOffset, addr); + } + static inline void handleReference(RefCountingKind tag, uintptr_t addrOffset, uint8_t *addr) { const auto &destroyFunc = destroyTable[static_cast(tag)]; @@ -431,6 +475,16 @@ struct CopyHandler { resolved, addrOffset, dest, src); } + static inline void handleMultiPayloadEnumGeneric(const Metadata *metadata, + const uint8_t *typeLayout, + size_t &offset, + uintptr_t &addrOffset, + uint8_t *dest, + uint8_t *src) { + ::handleMultiPayloadEnumGeneric(metadata, typeLayout, offset, + addrOffset, dest, src); + } + static inline void handleReference(RefCountingKind tag, uintptr_t addrOffset, uint8_t *dest, uint8_t *src) { const auto &retainFunc = retainTable[static_cast(tag)]; @@ -488,6 +542,16 @@ struct TakeHandler { resolved, addrOffset, dest, src); } + static inline void handleMultiPayloadEnumGeneric(const Metadata *metadata, + const uint8_t *typeLayout, + size_t &offset, + uintptr_t &addrOffset, + uint8_t *dest, + uint8_t *src) { + ::handleMultiPayloadEnumGeneric(metadata, typeLayout, offset, + addrOffset, dest, src); + } + static inline void handleReference(RefCountingKind tag, uintptr_t addrOffset, uint8_t *dest, uint8_t *src) { if (tag == RefCountingKind::UnknownWeak) { diff --git a/stdlib/public/runtime/BytecodeLayouts.h b/stdlib/public/runtime/BytecodeLayouts.h index f5e487428927b..2a4fafd742fcc 100644 --- a/stdlib/public/runtime/BytecodeLayouts.h +++ b/stdlib/public/runtime/BytecodeLayouts.h @@ -49,6 +49,7 @@ enum class RefCountingKind : uint8_t { MultiPayloadEnumFN = 0x13, MultiPayloadEnumFNResolved = 0x14, + MultiPayloadEnumGeneric = 0x15, Skip = 0x80, // We may use the MSB as flag that a count follows, diff --git a/stdlib/public/runtime/Enum.cpp b/stdlib/public/runtime/Enum.cpp index f1fdddd3a386e..13c00ed9dcc97 100644 --- a/stdlib/public/runtime/Enum.cpp +++ b/stdlib/public/runtime/Enum.cpp @@ -18,7 +18,9 @@ #include "swift/Runtime/Enum.h" #include "swift/Runtime/Debug.h" #include "Private.h" +#include "BytecodeLayouts.h" #include "EnumImpl.h" +#include "MetadataCache.h" #include #include @@ -214,65 +216,143 @@ swift::swift_initEnumMetadataMultiPayload(EnumMetadata *enumType, vwtable->publishLayout(layout); } -// void -// swift::swift_initEnumMetadataMultiPayloadWithLayoutString(EnumMetadata *enumType, -// EnumLayoutFlags layoutFlags, -// unsigned numPayloads, -// const TypeLayout * const *payloadLayouts) { - // // Accumulate the layout requirements of the payloads. - // size_t payloadSize = 0, alignMask = 0; - // bool isPOD = true, isBT = true; - // for (unsigned i = 0; i < numPayloads; ++i) { - // const TypeLayout *payloadLayout = payloadLayouts[i]; - // payloadSize - // = std::max(payloadSize, (size_t)payloadLayout->size); - // alignMask |= payloadLayout->flags.getAlignmentMask(); - // isPOD &= payloadLayout->flags.isPOD(); - // isBT &= payloadLayout->flags.isBitwiseTakable(); - // } - - // // Store the max payload size in the metadata. - // assignUnlessEqual(enumType->getPayloadSize(), payloadSize); - - // // The total size includes space for the tag. - // auto tagCounts = getEnumTagCounts(payloadSize, - // enumType->getDescription()->getNumEmptyCases(), - // numPayloads); - // unsigned totalSize = payloadSize + tagCounts.numTagBytes; - - // // See whether there are extra inhabitants in the tag. - // unsigned numExtraInhabitants = tagCounts.numTagBytes == 4 - // ? INT_MAX - // : (1 << (tagCounts.numTagBytes * 8)) - tagCounts.numTags; - // numExtraInhabitants = std::min(numExtraInhabitants, - // unsigned(ValueWitnessFlags::MaxNumExtraInhabitants)); - - // auto vwtable = getMutableVWTableForInit(enumType, layoutFlags); - - // // Set up the layout info in the vwtable. - // auto rawStride = (totalSize + alignMask) & ~alignMask; - // TypeLayout layout{totalSize, - // rawStride == 0 ? 1 : rawStride, - // ValueWitnessFlags() - // .withAlignmentMask(alignMask) - // .withPOD(isPOD) - // .withBitwiseTakable(isBT) - // .withEnumWitnesses(true) - // .withInlineStorage(ValueWitnessTable::isValueInline( - // isBT, totalSize, alignMask + 1)), - // numExtraInhabitants}; - - // installCommonValueWitnesses(layout, vwtable); - - // // Unconditionally overwrite the enum-tag witnesses. - // // The compiler does not generate meaningful enum-tag witnesses for - // // enums in this state. - // vwtable->getEnumTagSinglePayload = swift_getMultiPayloadEnumTagSinglePayload; - // vwtable->storeEnumTagSinglePayload = - // swift_storeMultiPayloadEnumTagSinglePayload; - - // vwtable->publishLayout(layout); -//} +void swift::swift_initEnumMetadataMultiPayloadWithLayoutString( + EnumMetadata *enumType, + EnumLayoutFlags layoutFlags, + unsigned numPayloads, + const Metadata * const *payloadLayouts) { + // Accumulate the layout requirements of the payloads. + size_t payloadSize = 0, alignMask = 0; + bool isPOD = true, isBT = true; + + size_t payloadRefCountBytes = 0; + for (unsigned i = 0; i < numPayloads; ++i) { + const TypeLayout *payloadLayout = payloadLayouts[i]->getTypeLayout(); + payloadSize + = std::max(payloadSize, (size_t)payloadLayout->size); + alignMask |= payloadLayout->flags.getAlignmentMask(); + isPOD &= payloadLayout->flags.isPOD(); + isBT &= payloadLayout->flags.isBitwiseTakable(); + + payloadRefCountBytes += _swift_refCountBytesForMetatype(payloadLayouts[i]); + // NUL terminator + payloadRefCountBytes += sizeof(uint64_t); + } + + // Store the max payload size in the metadata. + assignUnlessEqual(enumType->getPayloadSize(), payloadSize); + + // The total size includes space for the tag. + auto tagCounts = getEnumTagCounts(payloadSize, + enumType->getDescription()->getNumEmptyCases(), + numPayloads); + unsigned totalSize = payloadSize + tagCounts.numTagBytes; + + // See whether there are extra inhabitants in the tag. + unsigned numExtraInhabitants = tagCounts.numTagBytes == 4 + ? INT_MAX + : (1 << (tagCounts.numTagBytes * 8)) - tagCounts.numTags; + numExtraInhabitants = std::min(numExtraInhabitants, + unsigned(ValueWitnessFlags::MaxNumExtraInhabitants)); + + auto vwtable = getMutableVWTableForInit(enumType, layoutFlags); + + // Instantiate layout string + { + const size_t fixedLayoutHeaderSize = layoutStringHeaderSize + + sizeof(uint64_t) + // Tag + offset + sizeof(size_t) + // Extra tag byte count + sizeof(size_t) * 3; // Payload count, ref count bytes, enum size + + const size_t allocationSize = fixedLayoutHeaderSize + + (numPayloads * sizeof(size_t)) + // Payload ref count offsets + payloadRefCountBytes + + sizeof(uint64_t) * 2; // Last skip bytes + NUL terminator + + uint8_t *layoutStr = (uint8_t *)MetadataAllocator(LayoutStringTag) + .Allocate(allocationSize, alignof(uint8_t)); + + size_t layoutStrOffset = sizeof(uint64_t); + uint64_t tagAndOffset = ((uint64_t)RefCountingKind::MultiPayloadEnumGeneric) << 56; + + size_t refCountBytes = allocationSize - layoutStringHeaderSize - + (sizeof(uint64_t) * 2); + writeBytes(layoutStr, layoutStrOffset, refCountBytes); + writeBytes(layoutStr, layoutStrOffset, tagAndOffset); + writeBytes(layoutStr, layoutStrOffset, size_t(tagCounts.numTagBytes)); + writeBytes(layoutStr, layoutStrOffset, size_t(numPayloads)); + writeBytes(layoutStr, layoutStrOffset, payloadRefCountBytes); + writeBytes(layoutStr, layoutStrOffset, size_t(totalSize)); + + size_t fullOffset = 0; + LayoutStringFlags flags = LayoutStringFlags::Empty; + + size_t payloadRefCountOffsetOffset = layoutStrOffset; + size_t payloadRefCountOffset = 0; + + layoutStrOffset += sizeof(size_t) * numPayloads; + + for (unsigned i = 0; i < numPayloads; ++i) { + const Metadata *payloadType = payloadLayouts[i]; + + writeBytes(layoutStr, payloadRefCountOffsetOffset, payloadRefCountOffset); + + size_t layoutStrOffsetBefore = layoutStrOffset; + size_t previousFieldOffset = 0; + _swift_addRefCountStringForMetatype(layoutStr, layoutStrOffset, flags, + payloadType, fullOffset, + previousFieldOffset); + + // NUL terminator + writeBytes(layoutStr, layoutStrOffset, 0); + + payloadRefCountOffset += (layoutStrOffset - layoutStrOffsetBefore); + } + + // Last skip bytes (always 0 for enums) + writeBytes(layoutStr, layoutStrOffset, 0); + // NUL terminator + writeBytes(layoutStr, layoutStrOffset, 0); + + // we mask out HasRelativePointers, because at this point they have all been + // resolved to metadata pointers + layoutStrOffset = 0; + writeBytes(layoutStr, layoutStrOffset, + ((uint64_t)flags) & ~((uint64_t)LayoutStringFlags::HasRelativePointers)); + + enumType->setLayoutString(layoutStr); + + vwtable->destroy = swift_generic_destroy; + vwtable->initializeWithCopy = swift_generic_initWithCopy; + vwtable->initializeWithTake = swift_generic_initWithTake; + vwtable->assignWithCopy = swift_generic_assignWithCopy; + vwtable->assignWithTake = swift_generic_assignWithTake; + } + + // Set up the layout info in the vwtable. + auto rawStride = (totalSize + alignMask) & ~alignMask; + TypeLayout layout{totalSize, + rawStride == 0 ? 1 : rawStride, + ValueWitnessFlags() + .withAlignmentMask(alignMask) + .withPOD(isPOD) + .withBitwiseTakable(isBT) + .withEnumWitnesses(true) + .withInlineStorage(ValueWitnessTable::isValueInline( + isBT, totalSize, alignMask + 1)), + numExtraInhabitants}; + + installCommonValueWitnesses(layout, vwtable); + + // Unconditionally overwrite the enum-tag witnesses. + // The compiler does not generate meaningful enum-tag witnesses for + // enums in this state. + vwtable->getEnumTagSinglePayload = swift_getMultiPayloadEnumTagSinglePayload; + vwtable->storeEnumTagSinglePayload = + swift_storeMultiPayloadEnumTagSinglePayload; + + vwtable->publishLayout(layout); +} namespace { struct MultiPayloadLayout { diff --git a/test/Interpreter/layout_string_witnesses_dynamic.swift b/test/Interpreter/layout_string_witnesses_dynamic.swift index 4d1d441c9545a..cf402abf8ef8c 100644 --- a/test/Interpreter/layout_string_witnesses_dynamic.swift +++ b/test/Interpreter/layout_string_witnesses_dynamic.swift @@ -306,7 +306,7 @@ enum TestEnum { func testGenericWithEnumNonEmpty() { let ptr = allocateInternalGenericPtr(of: TestEnum.self) - + do { let x = TestClass() testGenericInit(ptr, to: TestEnum.nonEmpty(x)) @@ -336,7 +336,7 @@ public struct ResilientWrapper { func testResilient() { let ptr = UnsafeMutablePointer.allocate(capacity: 1) - + do { let x = TestClass() testInit(ptr, to: ResilientWrapper(x: SimpleResilient(x: 23, y: x), y: 5)) @@ -369,7 +369,7 @@ public struct GenericResilientWrapper { func testGenericResilient() { let ptr = UnsafeMutablePointer>.allocate(capacity: 1) - + do { let x = TestClass() testInit(ptr, to: GenericResilientWrapper(x: GenericResilient(x: x, y: 32), y: 32)) From 68e22e1fb1a77fc255e3efb67cf9a84ec2c3d745 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Wed, 14 Jun 2023 12:49:18 -0400 Subject: [PATCH 22/22] AST: Fix logic error in TypeMatcher::visitParameterizedProtocolType() Fixes https://github.com/apple/swift/issues/63410. --- include/swift/AST/TypeMatcher.h | 10 ++++++---- test/Generics/issue-63410.swift | 8 ++++++++ 2 files changed, 14 insertions(+), 4 deletions(-) create mode 100644 test/Generics/issue-63410.swift diff --git a/include/swift/AST/TypeMatcher.h b/include/swift/AST/TypeMatcher.h index 8f3e7ca1c3ea4..076d445c8c477 100644 --- a/include/swift/AST/TypeMatcher.h +++ b/include/swift/AST/TypeMatcher.h @@ -438,10 +438,12 @@ class TypeMatcher { if (firstArgs.size() == secondArgs.size()) { for (unsigned i : indices(firstArgs)) { - return this->visit(CanType(firstArgs[i]), - secondArgs[i], - sugaredFirstType->castTo() - ->getArgs()[i]); + if (!this->visit(CanType(firstArgs[i]), + secondArgs[i], + sugaredFirstType->castTo() + ->getArgs()[i])) { + return false; + } } return true; diff --git a/test/Generics/issue-63410.swift b/test/Generics/issue-63410.swift new file mode 100644 index 0000000000000..81b2585d538ce --- /dev/null +++ b/test/Generics/issue-63410.swift @@ -0,0 +1,8 @@ +// RUN: %target-typecheck-verify-swift + +protocol Derived where C == any Derived { + associatedtype A + associatedtype B + + associatedtype C +}