Skip to content

SIL verification failure with certain conformances to Collection. #66503

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
fibrechannelscsi opened this issue Jun 9, 2023 · 5 comments
Open
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. import Feature → declarations: `import` declarations SILGen Area → compiler: The SIL generation stage

Comments

@fibrechannelscsi
Copy link
Contributor

Description
The following code fails to compile in Debug mode. The error message is:
SIL verification failed: public/shared function must have a body: F->isDefinition() || F->hasForeignBody()

Steps to reproduce
This reproducer requires four files: two Package.swift files (one is for an internal package), and two regular Swift source files.
The directory tree looks like this:

./Package.swift
./Sources/InternalPackage/Package.swift
./Sources/InternalPackage/Sources/InternalPackage/U.swift
./Sources/ReproducerServer/A.swift

Listing for ./Package.swift:

// swift-tools-version: 5.6
import PackageDescription
let package = Package(
    name: "Reproducer",
    platforms: [.macOS(.v12),],
    products: [.library(name: "Reproducer", targets: ["ReproducerServer"]),],
    dependencies: [.package(name: "InternalPackage", path: "./Sources/InternalPackage")],
    targets: [.target(name: "ReproducerServer", dependencies: [.product(name: "InternalPackage", package: "InternalPackage")])]
)

Listing for ./Sources/InternalPackage/Package.swift:

// swift-tools-version: 5.6
// The swift-tools-version declares the minimum version of Swift required to build this package.
import PackageDescription
let package = Package(
    name: "InternalPackage",
    platforms: [.macOS(.v12), .iOS(.v15)],
    products: [.library(name: "InternalPackage",targets: ["InternalPackage"])],
    dependencies: [.package(url: "https://github.com/apple/swift-collections.git",from: "1.0.4")],
    targets: [.target(name: "InternalPackage",dependencies: [.product(name: "Collections", package: "swift-collections")])]
)

Listing for ./Sources/InternalPackage/Sources/InternalPackage/U.swift:

import Foundation; import OrderedCollections
public struct L<Root>{}
public typealias LV<V> = OrderedDictionary<String, B<V>>
public typealias B<V> = OrderedDictionary<String, V>
public protocol LP: Collection where Index == OrderedDictionary<Key, L<Root>>.Index, Element == OrderedDictionary<Key, L<Root>>.Element{
    associatedtype Root
    associatedtype Key: Hashable
    var l: OrderedDictionary<Key, L<Root>> { get set }
}
public extension LP {
    var startIndex: Index {self.l.elements.startIndex}
    var endIndex: Index {self.l.elements.endIndex}
    var indices: Range<Index> {self.l.elements.indices}
    func index(after index: Index) -> Index {fatalError()}
    subscript(index: Index) -> Element {self.l.elements[index]}
}
public protocol BGP: LP {}
public struct BG<Root, Key: Hashable>: BGP {
    public typealias Index = OrderedDictionary<Key, L<Root>>.Index
    public typealias Element = OrderedDictionary<Key, L<Root>>.Element
    public var l = OrderedDictionary<Key, L<Root>>()
}

Listing for ./Sources/ReproducerServer/A.swift:

import Foundation; import Collections; import InternalPackage
public final class C<Graph: A>: U<Graph.Root>, LP {
    public var l: OrderedDictionary<QOT, L<Root>>
    public typealias Index = OrderedDictionary<QOT, L<Root>>.Index
    public typealias Element = OrderedDictionary<QOT, L<Root>>.Element
    public typealias Root = Graph.Root
    public init(){fatalError()}
    required init(from decoder: any Decoder) throws {fatalError("")}
}
public enum QOT: String, Codable, Hashable, Comparable, CaseIterable {
    case A
    public static func < (lhs: QOT, rhs: QOT) -> Bool {return true}
}
public protocol A: Q {
    associatedtype Root: QO
    var root: Root { get }
}
public class Q: Codable {}
public protocol QO: AnyObject{}
public class U<Root: QO>: Q, A {
    public var root: Root
    public init (root: Root){fatalError()}
    required init(from decoder: any Decoder) throws {fatalError()}
}

Note that condensing the contents of A.swift and U.swift into a single file causes the build to succeed.

Expected behavior
The compilation should complete successfully.

Environment

  • Swift compiler version info: Toolchain 2023-05-31a exhibits this issue. Note that the default Xcode 14.2 toolchain is able to compile this successfully.
  • Xcode version info: 14.2
  • Deployment target: M1

Additional context
Stack trace:

1.	Apple Swift version 5.9-dev (LLVM fd15f782007e262, Swift 65bb599048dbd75)
2.	Compiling with the current language version
3.	While evaluating request ASTLoweringRequest(Lowering AST to SIL for file "/Users/user/playground/A/Sources/ReproducerServer/A.swift")
4.	While verifying SIL function "@$s15InternalPackage2LPPAAEy3KeyQz3key_AA1LVy4RootQzG5valuetSicir".
 for read for subscript(_:) (in module 'InternalPackage')
Stack dump without symbol names (ensure you have llvm-symbolizer in your PATH or set the environment var `LLVM_SYMBOLIZER_PATH` to point to it):
0  swift-frontend           0x00000001071c869c llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) + 56
1  swift-frontend           0x00000001071c7a40 llvm::sys::RunSignalHandlers() + 112
2  swift-frontend           0x00000001071c8cdc SignalHandler(int) + 304
3  libsystem_platform.dylib 0x0000000197f0ea24 _sigtramp + 56
4  libsystem_pthread.dylib  0x0000000197edfc28 pthread_kill + 288
5  libsystem_c.dylib        0x0000000197dedae8 abort + 180
6  swift-frontend           0x00000001032845a8 (anonymous namespace)::SILVerifier::_require(bool, llvm::Twine const&, std::__1::function<void ()> const&) + 1472
7  swift-frontend           0x0000000103284e88 (anonymous namespace)::SILVerifier::visitSILFunction(swift::SILFunction*) + 640
8  swift-frontend           0x0000000103280ba8 swift::SILFunction::verify(swift::SILPassManager*, bool, bool, bool) const + 204
9  swift-frontend           0x0000000103283a38 swift::SILModule::verify(swift::SILPassManager*, bool, bool) const + 184
10 swift-frontend           0x0000000103283948 swift::SILModule::verify(bool, bool) const + 88
11 swift-frontend           0x0000000102a31b50 swift::Lowering::SILGenModule::~SILGenModule() + 168
12 swift-frontend           0x0000000102a3ae94 swift::ASTLoweringRequest::evaluate(swift::Evaluator&, swift::ASTLoweringDescriptor) const + 5136
13 swift-frontend           0x0000000102afeda8 swift::SimpleRequest<swift::ASTLoweringRequest, std::__1::unique_ptr<swift::SILModule, std::__1::default_delete<swift::SILModule>> (swift::ASTLoweringDescriptor), (swift::RequestFlags)9>::evaluateRequest(swift::ASTLoweringRequest const&, swift::Evaluator&) + 148
14 swift-frontend           0x0000000102a3db98 llvm::Expected<swift::ASTLoweringRequest::OutputType> swift::Evaluator::getResultUncached<swift::ASTLoweringRequest>(swift::ASTLoweringRequest const&) + 396
15 swift-frontend           0x0000000102a3b120 swift::performASTLowering(swift::FileUnit&, swift::Lowering::TypeConverter&, swift::SILOptions const&, swift::IRGenOptions const*) + 100
16 swift-frontend           0x000000010251476c swift::performCompileStepsPostSema(swift::CompilerInstance&, int&, swift::FrontendObserver*) + 524
17 swift-frontend           0x0000000102523988 withSemanticAnalysis(swift::CompilerInstance&, swift::FrontendObserver*, llvm::function_ref<bool (swift::CompilerInstance&)>, bool) + 160
18 swift-frontend           0x0000000102517278 performCompile(swift::CompilerInstance&, int&, swift::FrontendObserver*) + 724
19 swift-frontend           0x000000010251620c swift::performFrontend(llvm::ArrayRef<char const*>, char const*, void*, swift::FrontendObserver*) + 2456
20 swift-frontend           0x0000000102360940 swift::mainEntry(int, char const**) + 2144
21 dyld                     0x0000000197b87f28 start + 2236

Additional SIL-related information:

SIL verification failed: public/shared function must have a body: F->isDefinition() || F->hasForeignBody()
In function:
// LP.subscript.read
sil shared [serialized] @$s15InternalPackage2LPPAAEy3KeyQz3key_AA1LVy4RootQzG5valuetSicir : $@yield_once @convention(method) <τ_0_0 where τ_0_0 : LP> (Int, @in_guaranteed τ_0_0) -> (@yields @in_guaranteed τ_0_0.Key, @yields L<τ_0_0.Root>)

If we remove the conformance of LP to Collection, that is, we change line 5 of U.swift to:
public protocol LP {
and then delete the extension of LP (that is, deleting lines 10 to 16 of the same file), then the compilation succeeds.

@fibrechannelscsi fibrechannelscsi added bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. triage needed This issue needs more specific labels labels Jun 9, 2023
@asl asl added AutoDiff and removed triage needed This issue needs more specific labels labels Aug 3, 2023
@asl
Copy link
Contributor

asl commented Aug 3, 2023

Non-differentiability diagnostics is missed somewhere. We do not support differentiating read semantic members accessors and co-routines.

@asl
Copy link
Contributor

asl commented Aug 3, 2023

So, likely similar to #54404 and around

@asavonic
Copy link
Contributor

This issue is not related to differentiation, and the reproducer does not use it at all.

When the module U.swift defines a coroutine read accessor (subscript), and another module (A.swift) imports it, the accessor decl is deserialized without a body and it does not pass the SIL Verifier:

    SIL verification failed: public/shared function must have a body: F->isDefinition() || F->hasForeignBody()
    In function:
    // LP.subscript.read
    sil shared [serialized] @$s15InternalPackage2LPPAAEy3KeyQz3key_AA1LVy4RootQzG5valuetSicir :
      $@yield_once @convention(method) <τ_0_0 where τ_0_0 : LP> (Int, @in_guaranteed τ_0_0) ->
      (@yields @in_guaranteed τ_0_0.Key, @yields L<τ_0_0.Root>)

It is not clear to me how A.swift is supposed to define it. U.swift does it in getSynthesizedAccessor, but this function early-exists for A.swift, because the accessor already exists (but doesn't have a body)

  frame #0: 0x0000555558f0772e swift-frontend`swift::AbstractStorageDecl::getSynthesizedAccessor(this=0x0000555566aa90f8, kind=Read) const at Decl.cpp:2639:12
     2636
     2637	AccessorDecl *AbstractStorageDecl::getSynthesizedAccessor(AccessorKind kind) const {
     2638	  if (auto *accessor = getAccessor(kind))
  -> 2639	    return accessor;

I've tried the following patch, but it crashes later in IRGen, and I'm not sure if this is a correct way to fix this issue.
If anyone knows how imported accessors are supposed to be defined, please let me know.

diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp
index fb73ad1454a..9ff761dc98f 100644
--- a/lib/AST/Decl.cpp
+++ b/lib/AST/Decl.cpp
@@ -2636,7 +2636,8 @@ bool AbstractStorageDecl::requiresOpaqueModifyCoroutine() const {
 
 AccessorDecl *AbstractStorageDecl::getSynthesizedAccessor(AccessorKind kind) const {
   if (auto *accessor = getAccessor(kind))
-    return accessor;
+    if (accessor->getBodyKind() != AccessorDecl::BodyKind::None)
+      return accessor;
 
   ASTContext &ctx = getASTContext();
   return evaluateOrDefault(ctx.evaluator,
diff --git a/lib/AST/TypeCheckRequests.cpp b/lib/AST/TypeCheckRequests.cpp
index c4bcb953087..0ade927c8e8 100644
--- a/lib/AST/TypeCheckRequests.cpp
+++ b/lib/AST/TypeCheckRequests.cpp
@@ -759,7 +759,7 @@ SynthesizeAccessorRequest::getCachedResult() const {
   auto *storage = std::get<0>(getStorage());
   auto kind = std::get<1>(getStorage());
   auto *accessor = storage->getAccessor(kind);
-  if (accessor)
+  if (accessor && accessor->getBodyKind() != AccessorDecl::BodyKind::None)
     return accessor;
   return llvm::None;
 }
@@ -767,7 +767,10 @@ SynthesizeAccessorRequest::getCachedResult() const {
 void SynthesizeAccessorRequest::cacheResult(AccessorDecl *accessor) const {
   auto *storage = std::get<0>(getStorage());
   auto kind = std::get<1>(getStorage());
-
+  auto *existingAccessor = storage->getAccessor(kind);
+  if (existingAccessor) {
+    return;
+  }
   storage->setSynthesizedAccessor(kind, accessor);
 }
 
diff --git a/lib/Sema/TypeCheckStorage.cpp b/lib/Sema/TypeCheckStorage.cpp
index 89d46decbaf..afc486f5ea3 100644
--- a/lib/Sema/TypeCheckStorage.cpp
+++ b/lib/Sema/TypeCheckStorage.cpp
@@ -2359,12 +2359,15 @@ createCoroutineAccessorPrototype(AbstractStorageDecl *storage,
   // Coroutine accessors always return ().
   const Type retTy = TupleType::getEmpty(ctx);
 
-  auto *accessor = AccessorDecl::create(
+  auto *accessor = storage->getAccessor(kind);
+  if (!accessor) {
+    accessor = AccessorDecl::create(
       ctx, loc, /*AccessorKeywordLoc=*/SourceLoc(), kind, storage,
       /*StaticLoc=*/SourceLoc(), StaticSpellingKind::None,
       /*Async=*/false, /*AsyncLoc=*/SourceLoc(),
       /*Throws=*/false, /*ThrowsLoc=*/SourceLoc(), params, retTy, dc);
-  accessor->setSynthesized();
+    accessor->setSynthesized();
+  }
 
   if (isMutating)
     accessor->setSelfAccessKind(SelfAccessKind::Mutating);

@asl asl added triage needed This issue needs more specific labels and removed AutoDiff labels Aug 14, 2023
@tbkka
Copy link
Contributor

tbkka commented Aug 14, 2023

Can you simplify the reproducer further?

@fibrechannelscsi
Copy link
Contributor Author

I can get A.swift down to:

import Foundation; import Collections; import InternalPackage
public final class C<Graph: A>: LP {
    public var l: OrderedDictionary<String, L<Graph.Root>>
    public typealias Index = OrderedDictionary<String, L<Graph.Root>>.Index
    required init(from decoder: any Decoder) throws {fatalError("")}
}
public protocol A {associatedtype Root: AnyObject; var root: Root { get }}

and U.swift down to:

import Foundation; import OrderedCollections
public struct L<Root>{}
public protocol LP: Collection where Index == OrderedDictionary<Key, L<Root>>.Index, Element == OrderedDictionary<Key, L<Root>>.Element{
    associatedtype Root
    associatedtype Key: Hashable
    var l: OrderedDictionary<Key, L<Root>> { get set }
}
public extension LP {
    var startIndex: Index {self.l.elements.startIndex}
    var endIndex: Index {self.l.elements.endIndex}
    var indices: Range<Index> {self.l.elements.indices}
    func index(after index: Index) -> Index {fatalError()}
    subscript(index: Index) -> Element {self.l.elements[index]}
}
public struct BG<Root, Key: Hashable>: LP {
    public typealias Index = OrderedDictionary<Key, L<Root>>.Index
    public typealias Element = OrderedDictionary<Key, L<Root>>.Element
    public var l = OrderedDictionary<Key, L<Root>>()
}

Note that deleting all of struct BG causes the compilation to succeed.

@asl asl added SILGen Area → compiler: The SIL generation stage import Feature → declarations: `import` declarations labels Aug 14, 2023
@hborla hborla removed the triage needed This issue needs more specific labels label Apr 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. import Feature → declarations: `import` declarations SILGen Area → compiler: The SIL generation stage
Projects
None yet
Development

No branches or pull requests

5 participants