Hm, it looks like we set the access at the Decl level, but don't copy over an attribute for it. Maybe SIL printing should set PrintAccess in PrintOptions?

Comment by Bartosz Polaczyk (JIRA)

Indeed! Enabling PrintAccess defines __derived_struct_equals as public, but assertion still fails:

((Qualifier != StoreOwnershipQualifier::Unqualified) || !getFunction().hasQualifiedOwnership() && "Unqualified inst in qualified function"), function createStore, file /Users/buildnode/jenkins/workspace/oss-swift-package-osx/swift/include/swift/SIL/SILBuilder.h, line 630.

Maybe I will try to dive into this issue...

Comment by Bartosz Polaczyk (JIRA)

Mentioned assertion failed because I missed -assume-parsing-unqualified-ownership-sil. My mistake.

Problem statement:
Equatable declaration is created in DerivedConformanceEquatableHashable::deriveEquatable_eq, which copies formal access from a type (together with a versioned attribute) to the `==` function.
As @belkadan pointed out, current implementation of a SILPrinter does not explicitly ask to print access types of AST declarations (Options.PrintAccess is false by default), so for declarations without access control attribute, their access is not printed in a SIL. Technically we could copy access attributes while derived-conforming to a protocol, rather than copy formal access and version only, but copying formal access only is almost as old as DerivedConformanceRawRepresentable so maybe it is better to not modify it...

Fix suggestion
Change suggested by @belkadan - .PrintAccess = true - emits formalAccesses to the SIL and SILGen, so it could solve all kinds of problems related with missing accesses modifier (like non-public var hashValue in the example above) but this seems to not be enough. For some reason SIL generated by

 private enum P {case a}

expects fileprivate access of __derived_enum_equals and hashValue (contrary to generated private attribute):

private enum P {
  case a
  @_implements(Equatable, ==(_:_:)) private static func __derived_enum_equals(_ a: P, _ b: P) -> Bool
  private var hashValue: Int { get }
}

bin/swift -frontend -c -primary-file /Users/bartosz.polaczyk/Desktop/si.sil -target x86_64-apple-darwin17.4.0 -enable-objc-interop -color-diagnostics -module-name si -o /var/folders/3v/2ymlw0cd53q9k8qsr1vg45gh0000gn/T/si-49e136.o -assume-parsing-unqualified-ownership-sil
/Users/bartosz.polaczyk/Desktop/si.sil:9:57: error: method '__derived_enum_equals' must be as accessible as its enclosing type because it matches a requirement in protocol 'Equatable'
  @_implements(Equatable, ==(_:_:)) private static func __derived_enum_equals(_ a: P, _ b: P) -> Bool
                                    ~~~~~~~             ^
                                    fileprivate
/Users/bartosz.polaczyk/Desktop/si.sil:10:15: error: property 'hashValue' must be as accessible as its enclosing type because it matches a requirement in protocol 'Hashable'
  private var hashValue: Int { get }
  ~~~~~~~     ^
  fileprivate

  

Now I am wondering if we really need to emit implicit AST types and declarations in a SILPrinter? I observed that even for .sil enum missing __derived_enum_equals, we still visit MultiConformanceChecker to derive Equatable.

BTW
@_versioned attribute is still quite misty for me as a "public-hidden" declaration, but shouldn't DerivedConformanceCodable also copy versioned attribute together with formal access? In other words, shouldn't 10bd85c also include Codable/CodingKey conformance?

@_versioned attribute is still quite misty for me as a "public-hidden" declaration, but shouldn't DerivedConformanceCodable also copy versioned attribute together with formal access? In other words, shouldn't 10bd85c also include Codable/CodingKey conformance?

Hm, probably yes. It's certainly possible for an inlinable function to rely on an internal-but-exposed-in-ABI type being Codable, and that would lead to it mentioning those methods directly.

graydon (JIRA User), @xwu, this also seems like a problem for __derived_enum_equals. Now we've put the wrong name in the ABI.

Comment by Graydon Hoare (JIRA)

@belkadan I .. am afraid I don't really get what you're saying here (nor do I understand much of the thread leading here). Is there a bug in copyFormalAccess?

No, I'm worried about __derived_enum_equals being a public symbol. What if someone devirtualizes the == call? You'd never be able to replace it with a custom == function.

Comment by Graydon Hoare (JIRA)

Hm, is this only a module-boundary thing? I.e. would it be fixed by keeping the symbol internal? It's only there to help with incremental mode (and I think some stdlib uses I don't quite understand).

Comment by Bartosz Polaczyk (JIRA)

I cannot proceed to fix that bug. I can only confirm that build swift-5.0-DEVELOPMENT-SNAPSHOT-2019-02-20-a still include the same SIL signature of __derived_enum_equals