Implement an unsafe expression to cover uses of unsafe constructs

Introduce an `unsafe` expression akin to `try` and `await` that notes
that there are unsafe constructs in the expression to the right-hand
side. Extend the effects checker to also check for unsafety along with
throwing and async operations. This will result in diagnostics like
the following:

    10 |   func sum() -> Int {
    11 |     withUnsafeBufferPointer { buffer in
    12 |       let value = buffer[0]
       |                   |     `- note: reference to unsafe subscript 'subscript(_:)'
       |                   |- warning: expression uses unsafe constructs but is not marked with 'unsafe'
       |                   `- note: reference to parameter 'buffer' involves unsafe type 'UnsafeBufferPointer<Int>'
    13 |       tryWithP(X())
    14 |       return fastAdd(buffer.baseAddress, buffer.count)

These will come with a Fix-It that inserts `unsafe` into the proper
place. There's also a warning that appears when `unsafe` doesn't cover
any unsafe code, making it easier to clean up extraneous `unsafe`.

This approach requires that `@unsafe` be present on any declaration
that involves unsafe constructs within its signature. Outside of the
signature, the `unsafe` expression is used to identify unsafe code.

Author: DougGregor

include/swift/AST/ASTBridging.h

@@ -1505,6 +1505,11 @@ BridgedUnresolvedSpecializeExpr BridgedUnresolvedSpecializeExpr_createParsed(
     BridgedSourceLoc cLAngleLoc, BridgedArrayRef cArguments,
     BridgedSourceLoc cRAngleLoc);
 
+SWIFT_NAME("BridgedUnsafeExpr.createParsed(_:unsafeLoc:subExpr:)")
+BridgedUnsafeExpr BridgedUnsafeExpr_createParsed(BridgedASTContext cContext,
+                                                 BridgedSourceLoc cUnsafeLoc,
+                                                 BridgedExpr cSubExpr);
+
 SWIFT_NAME("BridgedInOutExpr.createParsed(_:loc:subExpr:)")
 BridgedInOutExpr BridgedInOutExpr_createParsed(BridgedASTContext cContext,
                                                BridgedSourceLoc cLoc,

include/swift/AST/DiagnosticsParse.def

@@ -1371,6 +1371,8 @@ ERROR(expected_expr_after_ternary_colon,none,
       "expected expression after '? ... :' in ternary expression", ())
 ERROR(expected_expr_after_await, none,
       "expected expression after 'await'", ())
+ERROR(expected_expr_after_unsafe, none,
+      "expected expression after 'unsafe'", ())
 ERROR(expected_expr_after_move, none,
       "expected expression after 'consume'", ())
 ERROR(expected_expr_after_copy, none,

include/swift/AST/DiagnosticsSema.def

@@ -4522,7 +4522,7 @@ WARNING(nan_comparison_both_nan, none,
        (StringRef, bool))
 
 // If you change this, also change enum TryKindForDiagnostics.
-#define TRY_KIND_SELECT(SUB) "%select{try|try!|try?|await}" #SUB
+#define TRY_KIND_SELECT(SUB) "%select{try|try!|try?|await|unsafe}" #SUB
 
 ERROR(try_rhs,none,
       "'" TRY_KIND_SELECT(0) "' cannot appear to the right of a "
@@ -8078,11 +8078,6 @@ NOTE(sending_function_result_with_sending_param_note, none,
 ERROR(unsafe_attr_disabled,none,
       "attribute requires '-enable-experimental-feature AllowUnsafeAttribute'", ())
 
-GROUPED_WARNING(decl_involves_unsafe,Unsafe,none,
-      "%kindbase0 involves unsafe code; "
-      "use %select{'@unsafe' to indicate that its use is not memory-safe|"
-      "'@safe(unchecked)' to assert that the code is memory-safe}1",
-      (const Decl *, bool))
 NOTE(note_reference_to_unsafe_decl,none,
       "%select{reference|call}0 to unsafe %kind1",
       (bool, const ValueDecl *))
@@ -8102,6 +8097,12 @@ NOTE(note_reference_exclusivity_unchecked,none,
 NOTE(note_use_of_unsafe_conformance_is_unsafe,none,
      "@unsafe conformance of %0 to %kind1 involves unsafe code",
      (Type, const ValueDecl *))
+
+GROUPED_WARNING(decl_signature_involves_unsafe,Unsafe,none,
+      "%kindbase0 has an interface that is not memory-safe; "
+      "use '@unsafe' to indicate that its use is unsafe",
+      (const Decl *))
+
 GROUPED_WARNING(conformance_involves_unsafe,Unsafe,none,
      "conformance of %0 to %kind1 involves unsafe code; use '@unsafe' to "
      "indicate that the conformance is not memory-safe",
@@ -8113,44 +8114,20 @@ NOTE(note_type_witness_unsafe,none,
       "unsafe type %0 cannot satisfy safe associated type %1",
       (Type, DeclName))
 
-GROUPED_WARNING(override_safe_withunsafe,Unsafe,none,
+GROUPED_WARNING(override_safe_with_unsafe,Unsafe,none,
       "override of safe %0 with unsafe %0", (DescriptiveDeclKind))
-GROUPED_WARNING(use_of_unsafe_conformance_is_unsafe,Unsafe,none,
-      "@unsafe conformance of %0 to %kind1 involves unsafe code",
-       (Type, const ValueDecl *))
-GROUPED_WARNING(reference_unowned_unsafe,Unsafe,none,
-      "reference to unowned(unsafe) %kind0 is unsafe", (const ValueDecl *))
-GROUPED_WARNING(reference_exclusivity_unchecked,Unsafe,none,
-      "reference to @exclusivity(unchecked) %kind0 is unsafe", (const ValueDecl *))
-
-GROUPED_WARNING(reference_to_nonisolated_unsafe,Unsafe,none,
-      "reference to nonisolated(unsafe) %kind0 is unsafe in concurrently-executing code",
-       (const ValueDecl *))
-GROUPED_WARNING(reference_to_unsafe_decl,Unsafe,none,
-      "%select{reference|call}0 to unsafe %kindbase1",
-      (bool, const ValueDecl *))
-GROUPED_WARNING(reference_to_unsafe_typed_decl,Unsafe,none,
-      "%select{reference|call}0 to %kindbase1 involves unsafe type %2",
-      (bool, const ValueDecl *, Type))
-GROUPED_WARNING(reference_to_unsafe_through_typealias,Unsafe,none,
-     "reference to %kind0 whose underlying type involves unsafe type %1",
-     (const ValueDecl *, Type))
+
 GROUPED_WARNING(preconcurrency_import_unsafe,Unsafe,none,
       "@preconcurrency import is not memory-safe because it can silently "
       "introduce data races; use '@safe(unchecked)' to assert that the "
       "code is memory-safe", ())
-NOTE(encapsulate_unsafe_in_enclosing_context,none,
-     "make %kindbase0 @safe(unchecked) to allow it to use unsafe constructs in its definition",
-     (const Decl *))
-NOTE(make_enclosing_context_unsafe,none,
-     "make %kindbase0 @unsafe to indicate that its use is not memory-safe",
-     (const Decl *))
+GROUPED_WARNING(unsafe_without_unsafe,Unsafe,none,
+      "expression uses unsafe constructs but is not marked with 'unsafe'", ())
+WARNING(no_unsafe_in_unsafe,none,
+        "no unsafe operations occur within 'unsafe' expression", ())
 NOTE(make_subclass_unsafe,none,
      "make class %0 @unsafe to allow unsafe overrides of safe superclass methods",
      (DeclName))
-NOTE(make_conforming_context_unsafe,none,
-     "make the enclosing %0 @unsafe to allow unsafe conformance to protocol %1",
-     (DescriptiveDeclKind, DeclName))
 
 //===----------------------------------------------------------------------===//
 // MARK: Value Generics

include/swift/AST/Effects.h

@@ -40,7 +40,8 @@ class ProtocolDecl;
 
 enum class EffectKind : uint8_t {
   Throws = 1 << 0,
-  Async  = 1 << 1
+  Async  = 1 << 1,
+  Unsafe = 1 << 2,
 };
 using PossibleEffects = OptionSet<EffectKind>;
 

include/swift/AST/Expr.h

@@ -2173,6 +2173,36 @@ class AwaitExpr final : public IdentityExpr {
   }
 };
 
+/// UnsafeExpr - An 'unsafe' surrounding an expression, marking that the
+/// expression contains uses of unsafe declarations.
+///
+/// getSemanticsProvidingExpr() looks through this because it doesn't
+/// provide the value and only very specific clients care where the
+/// 'unsafe' was written.
+class UnsafeExpr final : public IdentityExpr {
+  SourceLoc UnsafeLoc;
+public:
+  UnsafeExpr(SourceLoc unsafeLoc, Expr *sub, Type type = Type(),
+            bool implicit = false)
+    : IdentityExpr(ExprKind::Unsafe, sub, type, implicit),
+      UnsafeLoc(unsafeLoc) {
+  }
+
+  static UnsafeExpr *createImplicit(ASTContext &ctx, SourceLoc unsafeLoc, Expr *sub, Type type = Type()) {
+    return new (ctx) UnsafeExpr(unsafeLoc, sub, type, /*implicit=*/true);
+  }
+
+  SourceLoc getLoc() const { return UnsafeLoc; }
+
+  SourceLoc getUnsafeLoc() const { return UnsafeLoc; }
+  SourceLoc getStartLoc() const { return UnsafeLoc; }
+  SourceLoc getEndLoc() const { return getSubExpr()->getEndLoc(); }
+
+  static bool classof(const Expr *e) {
+    return e->getKind() == ExprKind::Unsafe;
+  }
+};
+
 /// ConsumeExpr - A 'consume' surrounding an lvalue expression marking the
 /// lvalue as needing to be moved.
 class ConsumeExpr final : public Expr {

include/swift/AST/ExprNodes.def

@@ -107,6 +107,7 @@ ABSTRACT_EXPR(Identity, Expr)
   EXPR(Paren, IdentityExpr)
   EXPR(DotSelf, IdentityExpr)
   EXPR(Await, IdentityExpr)
+  EXPR(Unsafe, IdentityExpr)
   EXPR(Borrow, IdentityExpr)
   EXPR(UnresolvedMemberChainResult, IdentityExpr)
   EXPR_RANGE(Identity, Paren, UnresolvedMemberChainResult)

include/swift/AST/SourceFileExtras.h

@@ -13,7 +13,6 @@
 #ifndef SWIFT_AST_SOURCEFILEEXTRAS_H
 #define SWIFT_AST_SOURCEFILEEXTRAS_H
 
-#include "swift/AST/UnsafeUse.h"
 #include "llvm/ADT/DenseMap.h"
 #include <vector>
 
@@ -24,11 +23,6 @@ class Decl;
 /// Extra information associated with a source file that is lazily created and
 /// stored in a separately-allocated side structure.
 struct SourceFileExtras {
-  /// Captures all of the unsafe uses associated with a given declaration.
-  ///
-  /// The declaration is the entity that can be annotated (e.g., with @unsafe)
-  /// to suppress all of the unsafe-related diagnostics listed here.
-  llvm::DenseMap<const Decl *, std::vector<UnsafeUse>> unsafeUses;
 };
 
 }

include/swift/AST/UnsafeUse.h

@@ -74,13 +74,11 @@ class UnsafeUse {
     struct {
       TypeBase *type;
       void *conformanceRef;
-      DeclContext *declContext;
       const void *location;
     } conformance;
 
     struct {
       const Decl *decl;
-      DeclContext *declContext;
       TypeBase *type;
       const void *location;
     } entity;
@@ -107,7 +105,6 @@ class UnsafeUse {
 
   static UnsafeUse forReference(
       Kind kind,
-      DeclContext *dc,
       const Decl *decl,
       Type type,
       SourceLoc location
@@ -121,7 +118,6 @@ class UnsafeUse {
 
     UnsafeUse result(kind);
     result.storage.entity.decl = decl;
-    result.storage.entity.declContext = dc;
     result.storage.entity.type = type.getPointer();
     result.storage.entity.location = location.getOpaquePointerValue();
     return result;
@@ -152,49 +148,41 @@ class UnsafeUse {
 
   static UnsafeUse forConformance(Type subjectType,
                                   ProtocolConformanceRef conformance,
-                                  SourceLoc location,
-                                  DeclContext *dc) {
+                                  SourceLoc location) {
     assert(subjectType);
     UnsafeUse result(UnsafeConformance);
     result.storage.conformance.type = subjectType.getPointer();
     result.storage.conformance.conformanceRef = conformance.getOpaqueValue();
-    result.storage.conformance.declContext = dc;
     result.storage.conformance.location = location.getOpaquePointerValue();
     return result;
   }
 
-  static UnsafeUse forUnownedUnsafe(const Decl *decl,
-                                    SourceLoc location,
-                                    DeclContext *dc) {
-    return forReference(UnownedUnsafe, dc, decl, Type(), location);
+  static UnsafeUse forUnownedUnsafe(const Decl *decl, SourceLoc location) {
+    return forReference(UnownedUnsafe, decl, Type(), location);
   }
 
   static UnsafeUse forExclusivityUnchecked(const Decl *decl,
-                                           SourceLoc location,
-                                           DeclContext *dc) {
-    return forReference(ExclusivityUnchecked, dc, decl, Type(), location);
+                                           SourceLoc location) {
+    return forReference(ExclusivityUnchecked, decl, Type(), location);
   }
 
   static UnsafeUse forNonisolatedUnsafe(const Decl *decl,
-                                        SourceLoc location,
-                                        DeclContext *dc) {
-    return forReference(NonisolatedUnsafe, dc, decl, Type(), location);
+                                        SourceLoc location) {
+    return forReference(NonisolatedUnsafe, decl, Type(), location);
   }
 
   static UnsafeUse forReferenceToUnsafe(const Decl *decl,
                                         bool isCall,
-                                        DeclContext *dc,
                                         Type type,
                                         SourceLoc location) {
-    return forReference(isCall ? CallToUnsafe: ReferenceToUnsafe, dc,
+    return forReference(isCall ? CallToUnsafe: ReferenceToUnsafe,
                         decl, type, location);
   }
 
   static UnsafeUse forReferenceToUnsafeThroughTypealias(const Decl *decl,
-                                        DeclContext *dc,
                                         Type type,
                                         SourceLoc location) {
-    return forReference(ReferenceToUnsafeThroughTypealias, dc,
+    return forReference(ReferenceToUnsafeThroughTypealias,
                         decl, type, location);
   }
 
@@ -269,32 +257,6 @@ class UnsafeUse {
     return storage.typeWitness.assocType;
   }
 
-  /// Retrieve the declaration context in which the reference occurs.
-  DeclContext *getDeclContext() const {
-    switch (getKind()) {
-    case UnownedUnsafe:
-    case ExclusivityUnchecked:
-    case NonisolatedUnsafe:
-    case ReferenceToUnsafe:
-    case ReferenceToUnsafeThroughTypealias:
-    case CallToUnsafe:
-      return storage.entity.declContext;
-
-    case Override:
-      return getDecl()->getDeclContext();
-
-    case Witness:
-    case TypeWitness:
-      return getConformance().getConcrete()->getDeclContext();
-
-    case UnsafeConformance:
-      return storage.conformance.declContext;
-
-    case PreconcurrencyImport:
-      return storage.importDecl->getDeclContext();
-    }
-  }
-
   /// Get the original declaration for an issue with a polymorphic
   /// implementation, e.g., an overridden declaration or a protocol
   /// requirement.

lib/AST/ASTDumper.cpp

@@ -2528,6 +2528,11 @@ class PrintExpr : public ExprVisitor<PrintExpr, void, StringRef>,
     printRec(E->getSubExpr());
     printFoot();
   }
+  void visitUnsafeExpr(UnsafeExpr *E, StringRef label) {
+    printCommon(E, "unsafe_expr", label);
+    printRec(E->getSubExpr());
+    printFoot();
+  }
   void visitConsumeExpr(ConsumeExpr *E, StringRef label) {
     printCommon(E, "consume_expr", label);
     printRec(E->getSubExpr());

lib/AST/ASTPrinter.cpp

@@ -4817,6 +4817,11 @@ void PrintAST::visitAwaitExpr(AwaitExpr *expr) {
   visit(expr->getSubExpr());
 }
 
+void PrintAST::visitUnsafeExpr(UnsafeExpr *expr) {
+  Printer << "unsafe ";
+  visit(expr->getSubExpr());
+}
+
 void PrintAST::visitConsumeExpr(ConsumeExpr *expr) {
   Printer << "consume ";
   visit(expr->getSubExpr());