Merge pull request swiftlang#31886 from theblixguy/fix/SR-12821-5.3

[5.3] [TBDGen] Fix symbol mismatch for enum case constructors

Author: tkremenek

include/swift/SIL/SILDeclRef.h

@@ -21,6 +21,7 @@
 
 #include "swift/AST/ClangNode.h"
 #include "swift/AST/TypeAlignments.h"
+#include "swift/SIL/SILLinkage.h"
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/PointerUnion.h"
@@ -42,7 +43,6 @@ namespace swift {
   class ASTContext;
   class ClassDecl;
   class SILFunctionType;
-  enum class SILLinkage : unsigned char;
   enum IsSerialized_t : unsigned char;
   enum class SubclassScope : unsigned char;
   class SILModule;
@@ -413,6 +413,22 @@ inline llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, SILDeclRef C) {
   return OS;
 }
 
+// FIXME: This should not be necessary, but it looks like visibility rules for
+// extension members are slightly bogus, and so some protocol witness thunks
+// need to be public.
+//
+// We allow a 'public' member of an extension to witness a public
+// protocol requirement, even if the extended type is not public;
+// then SILGen gives the member private linkage, ignoring the more
+// visible access level it was given in the AST.
+inline bool
+fixmeWitnessHasLinkageThatNeedsToBePublic(SILDeclRef witness) {
+  auto witnessLinkage = witness.getLinkage(ForDefinition);
+  return !hasPublicVisibility(witnessLinkage)
+         && (!hasSharedVisibility(witnessLinkage)
+             || !witness.isSerialized());
+}
+
 } // end swift namespace
 
 namespace llvm {

include/swift/SIL/SILLinkage.h

@@ -265,20 +265,6 @@ inline SILLinkage effectiveLinkageForClassMember(SILLinkage linkage,
   return linkage;
 }
 
-// FIXME: This should not be necessary, but it looks like visibility rules for
-// extension members are slightly bogus, and so some protocol witness thunks
-// need to be public.
-//
-// We allow a 'public' member of an extension to witness a public
-// protocol requirement, even if the extended type is not public;
-// then SILGen gives the member private linkage, ignoring the more
-// visible access level it was given in the AST.
-inline bool
-fixmeWitnessHasLinkageThatNeedsToBePublic(SILLinkage witnessLinkage) {
-  return !hasPublicVisibility(witnessLinkage) &&
-         !hasSharedVisibility(witnessLinkage);
-}
-
 } // end swift namespace
 
 #endif

lib/SIL/IR/SILDeclRef.cpp

@@ -233,7 +233,7 @@ SILLinkage SILDeclRef::getLinkage(ForDefinition_t forDefinition) const {
 
   // Native function-local declarations have shared linkage.
   // FIXME: @objc declarations should be too, but we currently have no way
-  // of marking them "used" other than making them external. 
+  // of marking them "used" other than making them external.
   ValueDecl *d = getDecl();
   DeclContext *moduleContext = d->getDeclContext();
   while (!moduleContext->isModuleScopeContext()) {
@@ -335,6 +335,10 @@ SILLinkage SILDeclRef::getLinkage(ForDefinition_t forDefinition) const {
     }
   }
 
+  if (isEnumElement()) {
+    limit = Limit::OnDemand;
+  }
+
   auto effectiveAccess = d->getEffectiveAccess();
 
   // Private setter implementations for an internal storage declaration should

lib/SIL/Verifier/SILVerifier.cpp

@@ -687,6 +687,8 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
     }
     llvm::dbgs() << "In function:\n";
     F.print(llvm::dbgs());
+    llvm::dbgs() << "In module:\n";
+    F.getModule().print(llvm::dbgs());
 
     // We abort by default because we want to always crash in
     // the debugger.

lib/SILGen/SILGenType.cpp

@@ -567,7 +567,7 @@ class SILGenConformance : public SILGenWitnessTable<SILGenConformance> {
     auto witnessLinkage = witnessRef.getLinkage(ForDefinition);
     auto witnessSerialized = Serialized;
     if (witnessSerialized &&
-        fixmeWitnessHasLinkageThatNeedsToBePublic(witnessLinkage)) {
+        fixmeWitnessHasLinkageThatNeedsToBePublic(witnessRef)) {
       witnessLinkage = SILLinkage::Public;
       witnessSerialized = IsNotSerialized;
     } else {

lib/TBDGen/TBDGen.cpp

@@ -473,10 +473,11 @@ void TBDGenVisitor::addConformances(DeclContext *DC) {
         rootConformance);
     auto addSymbolIfNecessary = [&](ValueDecl *requirementDecl,
                                     ValueDecl *witnessDecl) {
-      auto witnessLinkage = SILDeclRef(witnessDecl).getLinkage(ForDefinition);
+      auto witnessRef = SILDeclRef(witnessDecl);
+      auto witnessLinkage = witnessRef.getLinkage(ForDefinition);
       if (conformanceIsFixed &&
           (isa<SelfProtocolConformance>(rootConformance) ||
-           fixmeWitnessHasLinkageThatNeedsToBePublic(witnessLinkage))) {
+           fixmeWitnessHasLinkageThatNeedsToBePublic(witnessRef))) {
         Mangle::ASTMangler Mangler;
         addSymbol(
             Mangler.mangleWitnessThunk(rootConformance, requirementDecl));
@@ -496,7 +497,8 @@ void TBDGenVisitor::addConformances(DeclContext *DC) {
             addSymbolIfNecessary(reqtAccessor, witnessAccessor);
           });
         } else if (isa<EnumElementDecl>(witnessDecl)) {
-          addSymbolIfNecessary(valueReq, witnessDecl);
+          auto getter = storage->getSynthesizedAccessor(AccessorKind::Get);
+          addSymbolIfNecessary(getter, witnessDecl);
         }
       }
     });
@@ -1016,13 +1018,12 @@ void TBDGenVisitor::visitProtocolDecl(ProtocolDecl *PD) {
 
 void TBDGenVisitor::visitEnumDecl(EnumDecl *ED) {
   visitNominalTypeDecl(ED);
-
-  if (!ED->isResilient())
-    return;
 }
 
 void TBDGenVisitor::visitEnumElementDecl(EnumElementDecl *EED) {
-  addSymbol(LinkEntity::forEnumCase(EED));
+  if (EED->getParentEnum()->isResilient())
+    addSymbol(LinkEntity::forEnumCase(EED));
+
   if (auto *PL = EED->getParameterList())
     visitDefaultArguments(EED, PL);
 }

test/SILGen/internal_protocol_refines_public_protocol_with_public_default_implementation.swift

@@ -0,0 +1,15 @@
+// RUN: %target-swift-emit-silgen -verify %s
+
+public protocol A {
+    @_borrowed
+    subscript() -> Int { get }
+}
+
+protocol B: A { }
+
+extension B {
+    public subscript() -> Int { return 0 }
+}
+
+public struct S: B {
+}

test/SILGen/protocol_enum_witness.swift

@@ -26,7 +26,7 @@ enum AnotherBar: AnotherFoo {
 // CHECK-NEXT: return [[TUPLE]] : $()
 // CHECK-END: }
 
-// CHECK-LABEL: sil hidden [transparent] [ossa] @$s21protocol_enum_witness3BarO6buttonyA2CmF : $@convention(method) (@thin Bar.Type) -> Bar {
+// CHECK-LABEL: sil shared [transparent] [ossa] @$s21protocol_enum_witness3BarO6buttonyA2CmF : $@convention(method) (@thin Bar.Type) -> Bar {
 // CHECK: bb0({{%.*}} : $@thin Bar.Type):
 // CHECK-NEXT: [[CASE:%.*]] = enum $Bar, #Bar.button!enumelt
 // CHECK-NEXT: return [[CASE]] : $Bar

test/TBD/enum_case_protocol_witness.swift

@@ -0,0 +1,23 @@
+// REQUIRES: VENDOR=apple
+// RUN: %target-swift-frontend -emit-ir -enable-testing -o/dev/null -module-name test -validate-tbd-against-ir=all %s
+// RUN: %target-swift-frontend -emit-ir -o/dev/null -module-name test -validate-tbd-against-ir=all %s
+
+protocol ProtoInternal {
+  static var bar1: Self { get }
+  static func bar2(arg: Int) -> Self
+}
+
+enum EnumInternal: ProtoInternal {
+  case bar1
+  case bar2(arg: Int)
+}
+
+public protocol ProtoPublic {
+  static var bar3: Self { get }
+  static func bar4(arg: Int) -> Self
+}
+
+public enum EnumPublic: ProtoPublic {
+  case bar3
+  case bar4(arg: Int)
+}