Move "unsafe" diagnostics from errors to warnings

DougGregor · DougGregor · commit 571859f294bd · 2024-08-19T14:34:00.000-07:00
Warnings fit better with the approach we're going for, and can be
escalated to errors by `-warnings-as-errors` for clients that need it.
diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def
@@ -7969,24 +7969,24 @@ NOTE(sending_function_result_with_sending_param_note, none,
 //------------------------------------------------------------------------------
 ERROR(unsafe_attr_disabled,none,
       "attribute requires '-enable-experimental-feature AllowUnsafeAttribute'", ())
-ERROR(override_safe_withunsafe,none,
+WARNING(override_safe_withunsafe,none,
       "override of safe %0 with unsafe %0", (DescriptiveDeclKind))
-ERROR(witness_unsafe,none,
+WARNING(witness_unsafe,none,
      "unsafe %0 %1 cannot satisfy safe requirement",
       (DescriptiveDeclKind, DeclName))
-ERROR(type_witness_unsafe,none,
+WARNING(type_witness_unsafe,none,
       "unsafe type %0 cannot satisfy safe associated type %1",
       (Type, DeclName))
-ERROR(unchecked_conformance_is_unsafe,none,
+WARNING(unchecked_conformance_is_unsafe,none,
       "@unchecked conformance involves unsafe code", ())
-ERROR(unowned_unsafe_is_unsafe,none,
+WARNING(unowned_unsafe_is_unsafe,none,
       "unowned(unsafe) involves unsafe code", ())
-ERROR(nonisolated_unsafe_is_unsafe,none,
+WARNING(nonisolated_unsafe_is_unsafe,none,
       "nonisolated(unsafe) involves unsafe code", ())
-ERROR(reference_to_unsafe_decl,none,
+WARNING(reference_to_unsafe_decl,none,
       "%select{reference|call}0 to unsafe %kindbase1",
       (bool, const ValueDecl *))
-ERROR(reference_to_unsafe_typed_decl,none,
+WARNING(reference_to_unsafe_typed_decl,none,
       "%select{reference|call}0 to %kindbase1 involves unsafe type %2",
       (bool, const ValueDecl *, Type))
 NOTE(unsafe_decl_here,none,
diff --git a/include/swift/Basic/Features.def b/include/swift/Basic/Features.def
@@ -405,8 +405,8 @@ EXPERIMENTAL_FEATURE(TrailingComma, false)
 /// Allow the @unsafe attribute.
 SUPPRESSIBLE_EXPERIMENTAL_FEATURE(AllowUnsafeAttribute, true)
 
-/// Disallow use of unsafe code.
-EXPERIMENTAL_FEATURE(DisallowUnsafe, true)
+/// Warn on use of unsafe constructs.
+EXPERIMENTAL_FEATURE(WarnUnsafe, true)
 
 #undef EXPERIMENTAL_FEATURE_EXCLUDED_FROM_MODULE_INTERFACE
 #undef EXPERIMENTAL_FEATURE
diff --git a/lib/AST/FeatureSet.cpp b/lib/AST/FeatureSet.cpp
@@ -230,7 +230,7 @@ static bool usesFeatureAllowUnsafeAttribute(Decl *decl) {
   return decl->getAttrs().hasAttribute<UnsafeAttr>();
 }
 
-UNINTERESTING_FEATURE(DisallowUnsafe)
+UNINTERESTING_FEATURE(WarnUnsafe)
 
 // ----------------------------------------------------------------------------
 // MARK: - FeatureSet
diff --git a/lib/Sema/AssociatedTypeInference.cpp b/lib/Sema/AssociatedTypeInference.cpp
@@ -346,7 +346,7 @@ static void recordTypeWitness(NormalProtocolConformance *conformance,
   }
 
   // If we're disallowing unsafe code, check for an unsafe type witness.
-  if (ctx.LangOpts.hasFeature(Feature::DisallowUnsafe) &&
+  if (ctx.LangOpts.hasFeature(Feature::WarnUnsafe) &&
       !assocType->isUnsafe() && type->isUnsafe()) {
     SourceLoc loc = typeDecl->getLoc();
     if (loc.isInvalid())
diff --git a/lib/Sema/TypeCheckAttr.cpp b/lib/Sema/TypeCheckAttr.cpp
@@ -4900,7 +4900,7 @@ Type TypeChecker::checkReferenceOwnershipAttr(VarDecl *var, Type type,
 
   // unowned(unsafe) is unsafe (duh).
   if (ownershipKind == ReferenceOwnership::Unmanaged &&
-      ctx.LangOpts.hasFeature(Feature::DisallowUnsafe)) {
+      ctx.LangOpts.hasFeature(Feature::WarnUnsafe)) {
     Diags.diagnose(attr->getLocation(), diag::unowned_unsafe_is_unsafe);
   }
 
@@ -7100,7 +7100,7 @@ void AttributeChecker::visitNonisolatedAttr(NonisolatedAttr *attr) {
 
   // nonisolated(unsafe) is unsafe, but only under strict concurrency.
   if (attr->isUnsafe() &&
-      Ctx.LangOpts.hasFeature(Feature::DisallowUnsafe) &&
+      Ctx.LangOpts.hasFeature(Feature::WarnUnsafe) &&
       Ctx.LangOpts.StrictConcurrencyLevel == StrictConcurrency::Complete)
     Ctx.Diags.diagnose(attr->getLocation(), diag::nonisolated_unsafe_is_unsafe);
 
diff --git a/lib/Sema/TypeCheckAvailability.cpp b/lib/Sema/TypeCheckAvailability.cpp
@@ -3886,21 +3886,20 @@ bool ExprAvailabilityWalker::diagnoseDeclRefAvailability(
   // If the declaration itself is "safe" but we don't disallow unsafe uses,
   // check whether it traffics in unsafe types.
   ASTContext &ctx = D->getASTContext();
-  if (ctx.LangOpts.hasFeature(Feature::DisallowUnsafe) && !D->isUnsafe()) {
+  if (ctx.LangOpts.hasFeature(Feature::WarnUnsafe) && !D->isUnsafe()) {
     auto type = D->getInterfaceType();
     if (auto subs = declRef.getSubstitutions())
       type = type.subst(subs);
     if (type->isUnsafe()) {
-      if (diagnoseUnsafeType(
-              ctx, R.Start, type,
-              [&](Type specificType) {
-                ctx.Diags.diagnose(
-                    R.Start, diag::reference_to_unsafe_typed_decl,
-                    call != nullptr && !isa<ParamDecl>(D), D,
-                    specificType);
-                D->diagnose(diag::decl_declared_here, D);
-              }))
-        return true;
+      diagnoseUnsafeType(
+          ctx, R.Start, type,
+          [&](Type specificType) {
+            ctx.Diags.diagnose(
+                R.Start, diag::reference_to_unsafe_typed_decl,
+                call != nullptr && !isa<ParamDecl>(D), D,
+                specificType);
+            D->diagnose(diag::decl_declared_here, D);
+          });
     }
   }
 
@@ -3966,21 +3965,20 @@ diagnoseDeclAsyncAvailability(const ValueDecl *D, SourceRange R,
 }
 
 /// Diagnose uses of unsafe declarations.
-static bool
+static void
 diagnoseDeclUnsafe(const ValueDecl *D, SourceRange R,
                    const Expr *call, const ExportContext &Where) {
   ASTContext &ctx = D->getASTContext();
-  if (!ctx.LangOpts.hasFeature(Feature::DisallowUnsafe))
-    return false;
+  if (!ctx.LangOpts.hasFeature(Feature::WarnUnsafe))
+    return;
 
   if (!D->isUnsafe())
-    return false;
+    return;
 
   SourceLoc diagLoc = call ? call->getLoc() : R.Start;
   ctx.Diags
     .diagnose(diagLoc, diag::reference_to_unsafe_decl, call != nullptr, D);
   D->diagnose(diag::decl_declared_here, D);
-  return true;
 }
 
 /// Diagnose uses of unavailable declarations. Returns true if a diagnostic
@@ -4019,8 +4017,7 @@ bool swift::diagnoseDeclAvailability(const ValueDecl *D, SourceRange R,
   if (diagnoseDeclAsyncAvailability(D, R, call, Where))
     return true;
 
-  if (diagnoseDeclUnsafe(D, R, call, Where))
-    return true;
+  diagnoseDeclUnsafe(D, R, call, Where);
 
   // Make sure not to diagnose an accessor's deprecation if we already
   // complained about the property/subscript.
diff --git a/lib/Sema/TypeCheckDeclOverride.cpp b/lib/Sema/TypeCheckDeclOverride.cpp
@@ -1733,7 +1733,7 @@ namespace  {
     void visitObjCAttr(ObjCAttr *attr) {}
 
     void visitUnsafeAttr(UnsafeAttr *attr) {
-      if (!Base->getASTContext().LangOpts.hasFeature(Feature::DisallowUnsafe))
+      if (!Base->getASTContext().LangOpts.hasFeature(Feature::WarnUnsafe))
         return;
 
       if (Override->isUnsafe() && !Base->isUnsafe()) {
diff --git a/lib/Sema/TypeCheckProtocol.cpp b/lib/Sema/TypeCheckProtocol.cpp
@@ -2430,7 +2430,7 @@ checkIndividualConformance(NormalProtocolConformance *conformance) {
 
   // @unchecked conformances are considered unsafe in strict concurrency mode.
   if (conformance->isUnchecked() &&
-      Context.LangOpts.hasFeature(Feature::DisallowUnsafe) &&
+      Context.LangOpts.hasFeature(Feature::WarnUnsafe) &&
       Context.LangOpts.StrictConcurrencyLevel == StrictConcurrency::Complete) {
     Context.Diags.diagnose(ComplainLoc, diag::unchecked_conformance_is_unsafe);
   }
@@ -5032,7 +5032,7 @@ void ConformanceChecker::resolveValueWitnesses() {
 
       // If we're disallowing unsafe code, check for an unsafe witness to a
       // safe requirement.
-      if (C.LangOpts.hasFeature(Feature::DisallowUnsafe) &&
+      if (C.LangOpts.hasFeature(Feature::WarnUnsafe) &&
           witness->isUnsafe() && !requirement->isUnsafe()) {
         witness->diagnose(diag::witness_unsafe,
                           witness->getDescriptiveKind(),
diff --git a/lib/Sema/TypeCheckType.cpp b/lib/Sema/TypeCheckType.cpp
@@ -6422,21 +6422,21 @@ Type ExplicitCaughtTypeRequest::evaluate(
   llvm_unreachable("Unhandled catch node");
 }
 
-bool swift::diagnoseUnsafeType(ASTContext &ctx, SourceLoc loc, Type type,
+void swift::diagnoseUnsafeType(ASTContext &ctx, SourceLoc loc, Type type,
                                llvm::function_ref<void(Type)> diagnose) {
-  if (!ctx.LangOpts.hasFeature(Feature::DisallowUnsafe))
-    return false;
+  if (!ctx.LangOpts.hasFeature(Feature::WarnUnsafe))
+    return;
 
   if (!type->isUnsafe())
-    return false;
+    return;
 
   // Look for a specific @unsafe nominal type.
   Type specificType;
   type.findIf([&specificType](Type type) {
     if (auto typeDecl = type->getAnyNominal()) {
       if (typeDecl->isUnsafe()) {
         specificType = type;
-        return true;
+        return false;
       }
     }
 
@@ -6450,6 +6450,4 @@ bool swift::diagnoseUnsafeType(ASTContext &ctx, SourceLoc loc, Type type,
       specificTypeDecl->diagnose(diag::unsafe_decl_here, specificTypeDecl);
     }
   }
-
-  return true;
 }
diff --git a/lib/Sema/TypeCheckType.h b/lib/Sema/TypeCheckType.h
@@ -688,10 +688,7 @@ bool diagnoseMissingOwnership(ParamSpecifier ownership,
 
 /// If the given type involves an unsafe type, diagnose it by calling the
 /// diagnose function with the most specific unsafe type that can be provided.
-///
-/// \returns \c true if the type was unsafe and we are diagnosing unsafe types
-/// here.
-bool diagnoseUnsafeType(ASTContext &ctx, SourceLoc loc, Type type,
+void diagnoseUnsafeType(ASTContext &ctx, SourceLoc loc, Type type,
                         llvm::function_ref<void(Type)> diagnose);
 
 } // end namespace swift
diff --git a/test/Unsafe/unsafe.swift b/test/Unsafe/unsafe.swift
@@ -1,7 +1,7 @@
 // RUN: %empty-directory(%t)
 // RUN: %target-swift-frontend -emit-module-path %t/unsafe_swift_decls.swiftmodule %S/Inputs/unsafe_swift_decls.swift -enable-experimental-feature AllowUnsafeAttribute
 
-// RUN: %target-typecheck-verify-swift -enable-experimental-feature AllowUnsafeAttribute -enable-experimental-feature DisallowUnsafe -I %t
+// RUN: %target-typecheck-verify-swift -enable-experimental-feature AllowUnsafeAttribute -enable-experimental-feature WarnUnsafe -I %t
 
 // Make sure everything compiles without error when unsafe code is allowed.
 // RUN: %target-swift-frontend -typecheck -enable-experimental-feature AllowUnsafeAttribute %s -I %t
@@ -17,7 +17,7 @@ protocol P {
 }
 
 struct XP: P {
-  @unsafe func f() { } // expected-error{{unsafe instance method 'f()' cannot satisfy safe requirement}}
+  @unsafe func f() { } // expected-warning{{unsafe instance method 'f()' cannot satisfy safe requirement}}
   @unsafe func g() { }
 }
 
@@ -29,7 +29,7 @@ protocol Ptrable2 {
   associatedtype Ptr // expected-note{{'Ptr' declared here}}
 }
 
-extension HasAPointerType: Ptrable2 { } // expected-error{{unsafe type 'HasAPointerType.Ptr' (aka 'PointerType') cannot satisfy safe associated type 'Ptr'}}
+extension HasAPointerType: Ptrable2 { } // expected-warning{{unsafe type 'HasAPointerType.Ptr' (aka 'PointerType') cannot satisfy safe associated type 'Ptr'}}
 
 // -----------------------------------------------------------------------
 // Overrides
@@ -49,7 +49,7 @@ class Sub: Super {
 // -----------------------------------------------------------------------
 struct SuperHolder {
   unowned var s1: Super
-  unowned(unsafe) var s2: Super // expected-error{{unowned(unsafe) involves unsafe code}}
+  unowned(unsafe) var s2: Super // expected-warning{{unowned(unsafe) involves unsafe code}}
 }
 
 // -----------------------------------------------------------------------
@@ -59,7 +59,7 @@ struct SuperHolder {
   func f() { } // expected-note{{'f()' declared here}}
 };
 
-class UnsafeSub: UnsafeSuper { } // expected-error{{reference to unsafe class 'UnsafeSuper'}}
+class UnsafeSub: UnsafeSuper { } // expected-warning{{reference to unsafe class 'UnsafeSuper'}}
 
 // -----------------------------------------------------------------------
 // Declaration references
@@ -68,14 +68,14 @@ class UnsafeSub: UnsafeSuper { } // expected-error{{reference to unsafe class 'U
 @unsafe var unsafeVar: Int = 0 // expected-note{{'unsafeVar' declared here}}
 
 func testMe(
-  _ pointer: PointerType, // expected-error{{reference to unsafe struct 'PointerType'}}
-  _ unsafeSuper: UnsafeSuper // expected-error{{reference to unsafe class 'UnsafeSuper'}}
+  _ pointer: PointerType, // expected-warning{{reference to unsafe struct 'PointerType'}}
+  _ unsafeSuper: UnsafeSuper // expected-warning{{reference to unsafe class 'UnsafeSuper'}}
   // expected-note@-1{{'unsafeSuper' declared here}}
 ) { 
-  unsafeF() // expected-error{{call to unsafe global function 'unsafeF'}}
-  _ = unsafeVar // expected-error{{reference to unsafe var 'unsafeVar'}}
-  unsafeSuper.f() // expected-error{{call to unsafe instance method 'f'}}
-  // expected-error@-1{{reference to parameter 'unsafeSuper' involves unsafe type 'UnsafeSuper'}}
+  unsafeF() // expected-warning{{call to unsafe global function 'unsafeF'}}
+  _ = unsafeVar // expected-warning{{reference to unsafe var 'unsafeVar'}}
+  unsafeSuper.f() // expected-warning{{call to unsafe instance method 'f'}}
+  // expected-warning@-1{{reference to parameter 'unsafeSuper' involves unsafe type 'UnsafeSuper'}}
 
-  _ = getPointers() // expected-error{{call to global function 'getPointers' involves unsafe type 'PointerType'}}
+  _ = getPointers() // expected-warning{{call to global function 'getPointers' involves unsafe type 'PointerType'}}
 }
diff --git a/test/Unsafe/unsafe_concurrency.swift b/test/Unsafe/unsafe_concurrency.swift
@@ -1,15 +1,15 @@
-// RUN: %target-typecheck-verify-swift -enable-experimental-feature AllowUnsafeAttribute -enable-experimental-feature DisallowUnsafe -enable-experimental-feature StrictConcurrency
+// RUN: %target-typecheck-verify-swift -enable-experimental-feature AllowUnsafeAttribute -enable-experimental-feature WarnUnsafe -enable-experimental-feature StrictConcurrency
 
 // REQUIRES: concurrency
 
-// expected-error@+1{{@unchecked conformance involves unsafe code}}
+// expected-warning@+1{{@unchecked conformance involves unsafe code}}
 class C: @unchecked Sendable {
   var counter: Int = 0
 }
 
 @available(SwiftStdlib 5.1, *)
 func f() async {
-  // expected-error@+1{{nonisolated(unsafe) involves unsafe code}}
+  // expected-warning@+1{{nonisolated(unsafe) involves unsafe code}}
   nonisolated(unsafe) var counter = 0
   Task.detached {
     counter += 1
diff --git a/test/Unsafe/unsafe_imports.swift b/test/Unsafe/unsafe_imports.swift
@@ -1,11 +1,11 @@
-// RUN: %target-typecheck-verify-swift -enable-experimental-feature AllowUnsafeAttribute -enable-experimental-feature DisallowUnsafe -I %S/Inputs
+// RUN: %target-typecheck-verify-swift -enable-experimental-feature AllowUnsafeAttribute -enable-experimental-feature WarnUnsafe -I %S/Inputs
 
 import unsafe_decls
 
-func testUnsafe(_ ut: UnsafeType) { // expected-error{{reference to unsafe struct 'UnsafeType'}}
-  unsafe_c_function() // expected-error{{call to unsafe global function 'unsafe_c_function'}}
+func testUnsafe(_ ut: UnsafeType) { // expected-warning{{reference to unsafe struct 'UnsafeType'}}
+  unsafe_c_function() // expected-warning{{call to unsafe global function 'unsafe_c_function'}}
 
   var array: [CInt] = [1, 2, 3, 4, 5]
   print_ints(&array, CInt(array.count))
-  // expected-error@-1{{call to global function 'print_ints' involves unsafe type 'UnsafeMutablePointer<Int32>'}}  
+  // expected-warning@-1{{call to global function 'print_ints' involves unsafe type 'UnsafeMutablePointer<Int32>'}}
 }
diff --git a/test/Unsafe/unsafe_stdlib.swift b/test/Unsafe/unsafe_stdlib.swift
@@ -1,16 +1,16 @@
-// RUN: %target-typecheck-verify-swift -enable-experimental-feature AllowUnsafeAttribute -enable-experimental-feature DisallowUnsafe
+// RUN: %target-typecheck-verify-swift -enable-experimental-feature WarnUnsafe
 
 // Make sure everything compiles without error when unsafe code is allowed.
-// RUN: %target-swift-frontend -typecheck -enable-experimental-feature AllowUnsafeAttribute %s
+// RUN: %target-swift-frontend -typecheck -enable-experimental-feature AllowUnsafeAttribute -warnings-as-errors %s
 
 func test(
-  x: OpaquePointer, // expected-error{{reference to unsafe struct 'OpaquePointer'}}
-  other: UnsafeMutablePointer<Int> // expected-error{{reference to unsafe generic struct 'UnsafeMutablePointer'}}
+  x: OpaquePointer, // expected-warning{{reference to unsafe struct 'OpaquePointer'}}
+  other: UnsafeMutablePointer<Int> // expected-warning{{reference to unsafe generic struct 'UnsafeMutablePointer'}}
 ) {
   var array = [1, 2, 3]
-  // expected-error@+1{{call to instance method 'withUnsafeBufferPointer' involves unsafe type 'UnsafeBufferPointer<Int>'}}
+  // expected-warning@+1{{call to instance method 'withUnsafeBufferPointer' involves unsafe type 'UnsafeBufferPointer<Int>'}}
   array.withUnsafeBufferPointer{ buffer in // expected-note{{'buffer' declared here}}
-    print(buffer) // expected-error{{reference to parameter 'buffer' involves unsafe type 'UnsafeBufferPointer<Int>'}}
+    print(buffer) // expected-warning{{reference to parameter 'buffer' involves unsafe type 'UnsafeBufferPointer<Int>'}}
   }
   array.append(4)
   _ = array

Original file line number	Diff line number	Diff line change
`@@ -230,7 +230,7 @@ static bool usesFeatureAllowUnsafeAttribute(Decl *decl) {`
`230`	`230`	`return decl->getAttrs().hasAttribute<UnsafeAttr>();`
`231`	`231`	`}`
`232`	`232`
`233`		`-UNINTERESTING_FEATURE(DisallowUnsafe)`
	`233`	`+UNINTERESTING_FEATURE(WarnUnsafe)`
`234`	`234`
`235`	`235`	`// ----------------------------------------------------------------------------`
`236`	`236`	`// MARK: - FeatureSet`
Original file line number	Diff line number	Diff line change
`@@ -346,7 +346,7 @@ static void recordTypeWitness(NormalProtocolConformance *conformance,`
`346`	`346`	`}`
`347`	`347`
`348`	`348`	`// If we're disallowing unsafe code, check for an unsafe type witness.`
`349`		`- if (ctx.LangOpts.hasFeature(Feature::DisallowUnsafe) &&`
	`349`	`+ if (ctx.LangOpts.hasFeature(Feature::WarnUnsafe) &&`
`350`	`350`	`!assocType->isUnsafe() && type->isUnsafe()) {`
`351`	`351`	`SourceLoc loc = typeDecl->getLoc();`
`352`	`352`	`if (loc.isInvalid())`
Original file line number	Diff line number	Diff line change
`@@ -6422,21 +6422,21 @@ Type ExplicitCaughtTypeRequest::evaluate(`
`6422`	`6422`	`llvm_unreachable("Unhandled catch node");`
`6423`	`6423`	`}`
`6424`	`6424`
`6425`		`-bool swift::diagnoseUnsafeType(ASTContext &ctx, SourceLoc loc, Type type,`
	`6425`	`+void swift::diagnoseUnsafeType(ASTContext &ctx, SourceLoc loc, Type type,`
`6426`	`6426`	`llvm::function_ref<void(Type)> diagnose) {`
`6427`		`- if (!ctx.LangOpts.hasFeature(Feature::DisallowUnsafe))`
`6428`		`- return false;`
	`6427`	`+ if (!ctx.LangOpts.hasFeature(Feature::WarnUnsafe))`
	`6428`	`+ return;`
`6429`	`6429`
`6430`	`6430`	`if (!type->isUnsafe())`
`6431`		`- return false;`
	`6431`	`+ return;`
`6432`	`6432`
`6433`	`6433`	`// Look for a specific @unsafe nominal type.`
`6434`	`6434`	`Type specificType;`
`6435`	`6435`	`type.findIf([&specificType](Type type) {`
`6436`	`6436`	`if (auto typeDecl = type->getAnyNominal()) {`
`6437`	`6437`	`if (typeDecl->isUnsafe()) {`
`6438`	`6438`	`specificType = type;`
`6439`		`- return true;`
	`6439`	`+ return false;`
`6440`	`6440`	`}`
`6441`	`6441`	`}`
`6442`	`6442`
`@@ -6450,6 +6450,4 @@ bool swift::diagnoseUnsafeType(ASTContext &ctx, SourceLoc loc, Type type,`
`6450`	`6450`	`specificTypeDecl->diagnose(diag::unsafe_decl_here, specificTypeDecl);`
`6451`	`6451`	`}`
`6452`	`6452`	`}`
`6453`		`-`
`6454`		`- return true;`
`6455`	`6453`	`}`