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.
This commit is contained in:
Doug Gregor
2025-01-10 09:14:07 -08:00
parent befa36146b
commit 8bb5bbedbc
42 changed files with 821 additions and 573 deletions

View File

@@ -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,

View File

@@ -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,

View File

@@ -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

View File

@@ -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>;

View File

@@ -2173,6 +2173,36 @@ public:
}
};
/// 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 {

View File

@@ -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)

View File

@@ -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;
};
}

View File

@@ -74,13 +74,11 @@ private:
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 @@ private:
static UnsafeUse forReference(
Kind kind,
DeclContext *dc,
const Decl *decl,
Type type,
SourceLoc location
@@ -121,7 +118,6 @@ private:
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 @@ public:
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 @@ public:
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.