SIL: Serialize bodies of local functions inside @_transparent functions

A transparent function might be deserialized and inlined into a function
in another module, which would cause problems if the function referenced
local functions.

Previously we would force local functions to have public linkage instead,
which worked, but was not resilient if the body of the transparent
function changed in the module that contained it.

Add a library evolution test ensuring that such a change is resilient
now.

Part of https://bugs.swift.org/browse/SR-267.

Author: slavapestov

include/swift/SIL/SILDeclRef.h

@@ -264,6 +264,8 @@ struct SILDeclRef {
 
   /// \brief True if the function should be treated as transparent.
   bool isTransparent() const;
+  /// \brief True if the function should have its body serialized.
+  bool isFragile() const;
   /// \brief True if the function has noinline attribute.
   bool isNoinline() const;
   /// \brief True if the function has __always inline attribute.

lib/SIL/SILDeclRef.cpp

@@ -240,34 +240,6 @@ Optional<AnyFunctionRef> SILDeclRef::getAnyFunctionRef() const {
   return AnyFunctionRef(loc.get<AbstractClosureExpr*>());
 }
 
-static SILLinkage getLinkageForLocalContext(DeclContext *dc) {
-  auto isClangImported = [](AbstractFunctionDecl *fn) -> bool {
-    if (fn->hasClangNode())
-      return true;
-    if (auto func = dyn_cast<FuncDecl>(fn))
-      if (auto storage = func->getAccessorStorageDecl())
-        return storage->hasClangNode();
-    return false;
-  };
-
-  while (!dc->isModuleScopeContext()) {
-    // Local definitions in transparent contexts are forced public because
-    // external references to them can be exposed by mandatory inlining.
-    // For Clang-imported decls, though, the closure should get re-synthesized
-    // on use.
-    if (auto fn = dyn_cast<AbstractFunctionDecl>(dc))
-      if (fn->isTransparent() && !isClangImported(fn))
-        return SILLinkage::Public;
-    // Check that this local context is not itself in a local transparent
-    // context.
-    dc = dc->getParent();
-  }
-
-  // FIXME: Once we have access control at the AST level, we should not assume
-  // shared always, but rather base it off of the local decl context.
-  return SILLinkage::Shared;
-}
-
 bool SILDeclRef::isThunk() const {
   return isCurried || isForeignToNativeThunk() || isNativeToForeignThunk();
 }
@@ -300,6 +272,8 @@ bool SILDeclRef::isClangGenerated() const {
 
   auto clangNode = getDecl()->getClangNode().getAsDecl();
   if (auto nd = dyn_cast_or_null<clang::NamedDecl>(clangNode)) {
+    // ie, 'static inline' functions for which we must ask Clang to emit a body
+    // for explicitly
     if (!nd->isExternallyVisible())
       return true;
   }
@@ -308,25 +282,26 @@ bool SILDeclRef::isClangGenerated() const {
 }
 
 SILLinkage SILDeclRef::getLinkage(ForDefinition_t forDefinition) const {
-  // Anonymous functions have local linkage.
-  if (auto closure = getAbstractClosureExpr())
-    return getLinkageForLocalContext(closure->getParent());
+  // Anonymous functions have shared linkage.
+  // FIXME: This should really be the linkage of the parent function.
+  if (getAbstractClosureExpr())
+    return SILLinkage::Shared;
 
-  // Native function-local declarations have local linkage.
+  // 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. 
   ValueDecl *d = getDecl();
   DeclContext *moduleContext = d->getDeclContext();
   while (!moduleContext->isModuleScopeContext()) {
     if (!isForeign && moduleContext->isLocalContext())
-      return getLinkageForLocalContext(moduleContext);
+      return SILLinkage::Shared;
     moduleContext = moduleContext->getParent();
   }
 
   // Currying and calling convention thunks have shared linkage.
   if (isThunk())
-    // If a function declares a @_cdecl name, its native-to-foreign thunk is
-    // exported with the visibility of the function.
+    // If a function declares a @_cdecl name, its native-to-foreign thunk
+    // is exported with the visibility of the function.
     if (!isNativeToForeignThunk() || !d->getAttrs().hasAttribute<CDeclAttr>())
       return SILLinkage::Shared;
 
@@ -383,6 +358,36 @@ bool SILDeclRef::isTransparent() const {
   return hasDecl() ? getDecl()->isTransparent() : false;
 }
 
+/// \brief True if the function should have its body serialized.
+bool SILDeclRef::isFragile() const {
+  DeclContext *dc;
+  if (auto closure = getAbstractClosureExpr())
+    dc = closure->getLocalContext();
+  else
+    dc = getDecl()->getDeclContext();
+
+  while (!dc->isModuleScopeContext()) {
+    // Local definitions in transparent contexts are fragile because
+    // external references to them can be exposed by mandatory inlining.
+    if (auto fn = dyn_cast<AbstractFunctionDecl>(dc))
+      if (fn->isTransparent() &&
+          fn->getEffectiveAccess() == Accessibility::Public)
+        return true;
+    // Check that this local context is not itself in a local transparent
+    // context.
+    dc = dc->getParent();
+  }
+
+  // Externally-visible transparent functions are fragile.
+  if (hasDecl())
+    if (auto fn = dyn_cast<AbstractFunctionDecl>(getDecl()))
+      if (fn->isTransparent() &&
+          fn->getEffectiveAccess() == Accessibility::Public)
+        return true;
+
+  return false;
+}
+
 /// \brief True if the function has noinline attribute.
 bool SILDeclRef::isNoinline() const {
   if (!hasDecl())

lib/SIL/SILModule.cpp

@@ -328,16 +328,16 @@ SILFunction *SILModule::getOrCreateFunction(SILLocation loc,
     return fn;
   }
 
-  IsTransparent_t IsTrans = constant.isTransparent()?
-                              IsTransparent : IsNotTransparent;
-  IsFragile_t IsFrag = IsNotFragile;
-  if (IsTrans == IsTransparent && (linkage == SILLinkage::Public
-                                   || linkage == SILLinkage::PublicExternal)) {
-    IsFrag = IsFragile;
-  }
-
-  EffectsKind EK = constant.hasEffectsAttribute() ?
-  constant.getEffectsAttribute() : EffectsKind::Unspecified;
+  IsTransparent_t IsTrans = constant.isTransparent()
+                            ? IsTransparent
+                            : IsNotTransparent;
+  IsFragile_t IsFrag = constant.isFragile()
+                       ? IsFragile
+                       : IsNotFragile;
+
+  EffectsKind EK = constant.hasEffectsAttribute()
+                   ? constant.getEffectsAttribute()
+                   : EffectsKind::Unspecified;
 
   Inline_t inlineStrategy = InlineDefault;
   if (constant.isNoinline())

test/SILGen/c_materializeForSet_linkage.swift

@@ -16,11 +16,11 @@ extension NSReferencePoint: Pointable {}
 // Make sure synthesized materializeForSet and its callbacks have shared linkage
 // for properties imported from Clang
 
-// CHECK-LABEL: sil shared [transparent] @_TFVSC7NSPointm1xSf
-// CHECK-LABEL: sil shared [transparent] @_TFVSC7NSPointm1ySf
+// CHECK-LABEL: sil shared [transparent] [fragile] @_TFVSC7NSPointm1xSf
+// CHECK-LABEL: sil shared [transparent] [fragile] @_TFVSC7NSPointm1ySf
 
-// CHECK-LABEL: sil shared [transparent] @_TFCSo16NSReferencePointm1xSf
-// CHECK-LABEL: sil shared [transparent] @_TFCSo16NSReferencePointm1ySf
+// CHECK-LABEL: sil shared [transparent] [fragile] @_TFCSo16NSReferencePointm1xSf
+// CHECK-LABEL: sil shared [transparent] [fragile] @_TFCSo16NSReferencePointm1ySf
 
-// CHECK-LABEL: sil shared [transparent] @_TFFCSo16NSReferencePointm1xSfU_T_
-// CHECK-LABEL: sil shared [transparent] @_TFFCSo16NSReferencePointm1ySfU_T_
+// CHECK-LABEL: sil shared [transparent] [fragile] @_TFFCSo16NSReferencePointm1xSfU_T_
+// CHECK-LABEL: sil shared [transparent] [fragile] @_TFFCSo16NSReferencePointm1ySfU_T_

test/SILGen/cf.swift

@@ -59,9 +59,9 @@ protocol Impedance {
 extension CCImpedance: Impedance {}
 
 // CHECK-LABEL: sil hidden [transparent] [thunk] @_TTWVSC11CCImpedance2cf9ImpedanceS0_FS1_g4realwx9Component
-// CHECK-LABEL: sil shared [transparent] @_TFVSC11CCImpedanceg4realSd
+// CHECK-LABEL: sil shared [transparent] [fragile] @_TFVSC11CCImpedanceg4realSd
 // CHECK-LABEL: sil hidden [transparent] [thunk] @_TTWVSC11CCImpedance2cf9ImpedanceS0_FS1_g4imagwx9Component
-// CHECK-LABEL: sil shared [transparent] @_TFVSC11CCImpedanceg4imagSd
+// CHECK-LABEL: sil shared [transparent] [fragile] @_TFVSC11CCImpedanceg4imagSd
 
 class MyMagnetism : CCMagnetismModel {
   // CHECK-LABEL: sil hidden [thunk] @_TToFC2cf11MyMagnetism15getRefrigerator{{.*}} : $@convention(objc_method) (MyMagnetism) -> @autoreleased CCRefrigerator

test/SILGen/imported_struct_array_field.swift

@@ -1,6 +1,6 @@
 // RUN: %target-swift-frontend -emit-silgen -import-objc-header %S/Inputs/array_typedef.h %s | FileCheck %s
 
-// CHECK-LABEL: sil shared [transparent] @_TFVSC4NameC{{.*}} : $@convention(thin) (UInt8, UInt8, UInt8, UInt8, @thin Name.Type) -> Name
+// CHECK-LABEL: sil shared [transparent] [fragile] @_TFVSC4NameC{{.*}} : $@convention(thin) (UInt8, UInt8, UInt8, UInt8, @thin Name.Type) -> Name
 func useImportedArrayTypedefInit() -> Name {
   return Name(name: (0, 0, 0, 0))
 }

test/SILGen/nsmanaged-witness.swift

@@ -61,5 +61,5 @@ extension Foo: NativeIntProperty {}
 // TODO: We can't emit a vtable entry for materializeForSet for ObjC types.
 // CHECK-NOT: class_method {{.*}}Foo{{.*}}intProperty{{.*}}materializeForSet
 
-// CHECK-LABEL: sil shared [transparent] @_TFCSo3Foom11intPropertyVs5Int32
+// CHECK-LABEL: sil shared [transparent] [fragile] @_TFCSo3Foom11intPropertyVs5Int32
 

test/SILGen/transparent_attribute.swift

@@ -181,19 +181,19 @@ struct testVarDeclShortenedSyntax {
 }
 // CHECK: sil hidden [transparent] @_TF21transparent_attributeg22transparentOnGlobalVarSi
 
-// Local functions in transparent context have public linkage.
-@_transparent func foo() {
-  // CHECK-LABEL: sil @_TFF21transparent_attribute3fooFT_T_L_3barFT_T_ : $@convention(thin) () -> ()
+// Local functions in transparent context are fragile.
+@_transparent public func foo() {
+  // CHECK-LABEL: sil shared [fragile] @_TFF21transparent_attribute3fooFT_T_L_3barFT_T_ : $@convention(thin) () -> ()
   func bar() {}
   bar()
 
-  // CHECK-LABEL: sil @_TFF21transparent_attribute3fooFT_T_U_FT_T_ : $@convention(thin) () -> () {
+  // CHECK-LABEL: sil shared [fragile] @_TFF21transparent_attribute3fooFT_T_U_FT_T_ : $@convention(thin) () -> () {
   let f: () -> () = {}
   f()
 
-  // CHECK-LABEL: sil @_TFF21transparent_attribute3fooFT_T_L_3zimFT_T_ : $@convention(thin) () -> () {
+  // CHECK-LABEL: sil shared [fragile] @_TFF21transparent_attribute3fooFT_T_L_3zimFT_T_ : $@convention(thin) () -> () {
   func zim() {
-    // CHECK-LABEL: sil @_TFFF21transparent_attribute3fooFT_T_L_3zimFT_T_L_4zangFT_T_ : $@convention(thin) () -> () {
+    // CHECK-LABEL: sil shared [fragile] @_TFFF21transparent_attribute3fooFT_T_L_3zimFT_T_L_4zangFT_T_ : $@convention(thin) () -> () {
     func zang() {
     }
     zang()

validation-test/Evolution/Inputs/function_change_transparent_body.swift

@@ -7,3 +7,15 @@
 #endif
 }
 
+@_transparent public func getFunction(x: Int) -> Int -> Int {
+  // The mangled name and calling convention of the local function
+  // will change -- so we must serialize it and inline it into
+  // the calling module
+
+#if BEFORE
+  func id(y: Int) -> Int { return x * y }
+#else
+  func id(y: Int) -> Int { return y }
+#endif
+  return id
+}

validation-test/Evolution/test_function_change_transparent_body.swift

@@ -21,9 +21,19 @@ var ChangeTransparentBodyTest = TestSuite("ChangeTransparentBody")
 ChangeTransparentBodyTest.test("ChangeTransparentBody") {
 
 #if BEFORE
-  expectEqual(getBuildVersion(), 0)
+  expectEqual(0, getBuildVersion())
 #else
-  expectEqual(getBuildVersion(), 1)
+  expectEqual(1, getBuildVersion())
+#endif
+
+}
+
+ChangeTransparentBodyTest.test("ChangeTransparentClosure") {
+
+#if BEFORE
+  expectEqual(202, getFunction(2)(101))
+#else
+  expectEqual(101, getFunction(2)(101))
 #endif
 
 }