Merge pull request #84975 from jckarter/cant-discard-in-inlinable-code

Sema: `discard self` should not be allowed in inlinable methods of non-frozen types.

Author: jckarter

include/swift/AST/DeclContext.h

@@ -220,6 +220,9 @@ struct FragileFunctionKind {
   friend bool operator==(FragileFunctionKind lhs, FragileFunctionKind rhs) {
     return lhs.kind == rhs.kind;
   }
+  friend bool operator!=(FragileFunctionKind lhs, FragileFunctionKind rhs) {
+    return lhs.kind != rhs.kind;
+  }
 
   /// Casts to `unsigned` for diagnostic %selects.
   unsigned getSelector() { return static_cast<unsigned>(kind); }

include/swift/AST/DiagnosticsSema.def

@@ -5343,12 +5343,27 @@ GROUPED_ERROR(opaque_type_unsupported_availability,OpaqueTypeInference,none,
 //------------------------------------------------------------------------------
 // MARK: Discard Statement
 //------------------------------------------------------------------------------
+#define FRAGILE_FUNC_KIND \
+  "%select{a '@_transparent' function|" \
+  "an '@inlinable' function|" \
+  "an '@_alwaysEmitIntoClient' function|" \
+  "a default argument value|" \
+  "a property initializer in a '@frozen' type|" \
+  "a '@backDeployed' function|" \
+  "an embedded function not marked '@_neverEmitIntoClient'}"
+
 ERROR(discard_wrong_context_decl,none,
       "'discard' statement cannot appear in %kindonly0",
       (const Decl *))
 ERROR(discard_no_deinit,none,
       "'discard' has no effect for type %0 unless it has a deinitializer",
       (Type))
+ERROR(discard_in_inlinable_method,none,
+      "'discard' statement cannot be used in " FRAGILE_FUNC_KIND "0 inside of type %1, which is not '@frozen'",
+      (unsigned, Type))
+WARNING(discard_in_inlinable_method_warning,none,
+        "'discard' statement cannot be used in " FRAGILE_FUNC_KIND "0 inside of type %1, which is not '@frozen'; this will become an error",
+        (unsigned, Type))
 ERROR(discard_wrong_context_closure,none,
       "'discard' statement cannot appear in closure",
       ())
@@ -7306,15 +7321,6 @@ WARNING(inlinable_implies_usable_from_inline,none,
 ERROR(usable_from_inline_attr_in_protocol,none,
       "'@usableFromInline' attribute cannot be used in protocols", ())
 
-#define FRAGILE_FUNC_KIND \
-  "%select{a '@_transparent' function|" \
-  "an '@inlinable' function|" \
-  "an '@_alwaysEmitIntoClient' function|" \
-  "a default argument value|" \
-  "a property initializer in a '@frozen' type|" \
-  "a '@backDeployed' function|" \
-  "an embedded function not marked '@_neverEmitIntoClient'}"
-
 ERROR(local_type_in_inlinable_function,
       none, "type %0 cannot be nested inside " FRAGILE_FUNC_KIND "1",
       (Identifier, unsigned))

lib/Sema/TypeCheckStmt.cpp

@@ -25,6 +25,7 @@
 #include "swift/AST/ASTScope.h"
 #include "swift/AST/ASTVisitor.h"
 #include "swift/AST/ASTWalker.h"
+#include "swift/AST/Attr.h"
 #include "swift/AST/ConformanceLookup.h"
 #include "swift/AST/DiagnosticSuppression.h"
 #include "swift/AST/DiagnosticsSema.h"
@@ -1325,6 +1326,22 @@ class StmtChecker : public StmtVisitor<StmtChecker, Stmt*> {
                            nominalType)
             .fixItRemove(DS->getSourceRange());
         diagnosed = true;
+      // if the type is public and not frozen, then the method must not be
+      // inlinable.
+      } else if (auto fragileKind = fn->getFragileFunctionKind();
+                 !nominalDecl->getAttrs().hasAttribute<FrozenAttr>()
+                 && fragileKind != FragileFunctionKind{FragileFunctionKind::None}) {
+        ctx.Diags.diagnose(DS->getDiscardLoc(),
+                           // Code in ABI stable SDKs has already used the `@inlinable`
+                           // attribute on functions using `discard self`.
+                           // Phase this in as a warning until those APIs
+                           // can be updated.
+                           fragileKind == FragileFunctionKind{FragileFunctionKind::Inlinable}
+                             ? diag::discard_in_inlinable_method_warning
+                             : diag::discard_in_inlinable_method,
+                           fragileKind.getSelector(), nominalType)
+            .fixItRemove(DS->getSourceRange());
+        diagnosed = true;
       } else {
         // Set the contextual type for the sub-expression before we typecheck.
         contextualInfo = {nominalType, CTP_DiscardStmt};

test/ModuleInterface/discard_interface.swift

@@ -8,6 +8,7 @@
 // CHECK: @_alwaysEmitIntoClient public consuming func AEIC_discard() { discard self }
 // CHECK: @inlinable public consuming func inlinable_discard() { discard self }
 
+@frozen
 public struct MoveOnlyStruct: ~Copyable {
   let x = 0
 

test/Sema/discard_resilient_inlinable.swift

@@ -0,0 +1,69 @@
+// RUN: %target-typecheck-verify-swift
+
+public struct NotFrozen: ~Copyable {
+	deinit {}
+
+	public consuming func notInlinable() {
+		discard self
+	}
+
+	@inlinable
+	public consuming func inlinable() {
+		// expected-warning @+1 {{'discard' statement cannot be used in an '@inlinable' function inside of type 'NotFrozen', which is not '@frozen'}}
+		discard self
+	}
+
+	@_alwaysEmitIntoClient
+	public consuming func aeic() {
+		// expected-error @+1 {{'discard' statement cannot be used in an '@_alwaysEmitIntoClient' function inside of type 'NotFrozen', which is not '@frozen'}}
+		discard self
+	}
+
+	@_transparent
+	public consuming func transparent() {
+		// expected-error @+1 {{'discard' statement cannot be used in a '@_transparent' function inside of type 'NotFrozen', which is not '@frozen'}}
+		discard self
+	}
+}
+
+@frozen
+public struct Frozen: ~Copyable {
+	deinit {}
+
+	public consuming func notInlinable() {
+		discard self
+	}
+
+	@inlinable
+	public consuming func inlinable() {
+		discard self
+	}
+}
+
+@usableFromInline
+internal struct NotFrozenUFI: ~Copyable {
+	deinit {}
+
+	public consuming func notInlinable() {
+		discard self
+	}
+
+	@inlinable
+	public consuming func inlinable() {
+		// expected-warning @+1 {{'discard' statement cannot be used in an '@inlinable' function inside of type 'NotFrozenUFI', which is not '@frozen'}}
+		discard self
+	}
+
+	@_alwaysEmitIntoClient
+	public consuming func aeic() {
+		// expected-error @+1 {{'discard' statement cannot be used in an '@_alwaysEmitIntoClient' function inside of type 'NotFrozenUFI', which is not '@frozen'}}
+		discard self
+	}
+
+	@_transparent
+	public consuming func transparent() {
+		// expected-error @+1 {{'discard' statement cannot be used in a '@_transparent' function inside of type 'NotFrozenUFI', which is not '@frozen'}}
+		discard self
+	}
+}
+