[Strict memory safety] Provide argument-specific diagnostics for calls

Similar to what we do for 'throws' checking, perform argument-specific
checking for unsafe call arguments. This provides more detailed failures:

```
example.swift:18:3: warning: expression uses unsafe constructs but is not
marked with 'unsafe' [#StrictMemorySafety]
16 |   x.f(a: 0, b: 17, c: nil)
17 |
18 |   x.f(a: 0, b: 17, c: &i)
   |   |                   `- note: argument 'c' in call to instance
method 'f' has unsafe type 'UnsafePointer<Int>?'
   |   `- warning: expression uses unsafe constructs but is not marked
with 'unsafe' [#StrictMemorySafety]
19 |   unsafeF()
20 | }
```

It also means that we won't complain for `nil` or `Optional.none`
arguments passed to unsafe types, which eliminates some false
positives, and won't complain about unsafe result types when there is
a call---because we'd still get complaints later about the
actually-unsafe bit, which is using those results.

Fixes rdar://149629670.
This commit is contained in:
Doug Gregor
2025-04-25 11:54:18 -07:00
parent 0b264e2d55
commit ee9487b86f
11 changed files with 365 additions and 110 deletions

View File

@@ -8341,6 +8341,15 @@ NOTE(note_reference_to_unsafe_decl,none,
NOTE(note_reference_to_unsafe_typed_decl,none,
"%select{reference|call}0 to %kind1 involves unsafe type %2",
(bool, const ValueDecl *, Type))
NOTE(note_unsafe_call_decl_argument_named,none,
"argument %1 in call to %kindbase0 has unsafe type %2",
(const ValueDecl *, Identifier, Type))
NOTE(note_unsafe_call_decl_argument_indexed,none,
"argument #%1 in call to %kindbase0 has unsafe type %2",
(const ValueDecl *, unsigned, Type))
NOTE(note_unsafe_call_argument_indexed,none,
"argument #%0 in call has unsafe type %1",
(unsigned, Type))
NOTE(note_reference_to_unsafe_through_typealias,none,
"reference to %kind0 whose underlying type involves unsafe type %1",
(const ValueDecl *, Type))

View File

@@ -53,6 +53,8 @@ public:
ReferenceToUnsafeThroughTypealias,
/// A call to an unsafe declaration.
CallToUnsafe,
/// An unsafe argument in a call.
CallArgument,
/// A @preconcurrency import.
PreconcurrencyImport,
/// A use of withoutActuallyEscaping that lacks enforcement that the
@@ -91,6 +93,15 @@ private:
MakeTemporarilyEscapableExpr *temporarilyEscaping;
struct {
Expr *call;
const Decl *calleeDecl;
TypeBase *paramType;
const void *argumentName;
unsigned argumentIndex;
Expr *argument;
} callArgument;
const ImportDecl *importDecl;
} storage;
@@ -201,6 +212,19 @@ public:
decl, type, location);
}
static UnsafeUse forCallArgument(
Expr *call, const Decl *calleeDecl, Type paramType,
Identifier argumentName, unsigned argumentIndex, Expr *argument) {
UnsafeUse result(CallArgument);
result.storage.callArgument.call = call;
result.storage.callArgument.calleeDecl = calleeDecl;
result.storage.callArgument.paramType = paramType.getPointer();
result.storage.callArgument.argumentName = argumentName.getAsOpaquePointer();
result.storage.callArgument.argumentIndex = argumentIndex;
result.storage.callArgument.argument = argument;
return result;
}
static UnsafeUse forTemporarilyEscaping(MakeTemporarilyEscapableExpr *expr) {
UnsafeUse result(TemporarilyEscaping);
result.storage.temporarilyEscaping = expr;
@@ -242,6 +266,9 @@ public:
return SourceLoc(
llvm::SMLoc::getFromPointer((const char *)storage.entity.location));
case CallArgument:
return storage.callArgument.call->getLoc();
case TemporarilyEscaping:
return storage.temporarilyEscaping->getLoc();
@@ -257,6 +284,7 @@ public:
case Witness:
case TemporarilyEscaping:
case PreconcurrencyImport:
case CallArgument:
// Cannot replace location.
return;
@@ -298,6 +326,9 @@ public:
case CallToUnsafe:
return storage.entity.decl;
case CallArgument:
return storage.callArgument.calleeDecl;
case UnsafeConformance:
case TemporarilyEscaping:
return nullptr;
@@ -330,6 +361,7 @@ public:
case ReferenceToUnsafeThroughTypealias:
case ReferenceToUnsafeStorage:
case CallToUnsafe:
case CallArgument:
case UnsafeConformance:
case PreconcurrencyImport:
case TemporarilyEscaping:
@@ -360,6 +392,9 @@ public:
case CallToUnsafe:
return storage.entity.type;
case CallArgument:
return storage.callArgument.paramType;
case TemporarilyEscaping:
return storage.temporarilyEscaping->getOpaqueValue()->getType();
}
@@ -386,11 +421,24 @@ public:
case ReferenceToUnsafeStorage:
case ReferenceToUnsafeThroughTypealias:
case CallToUnsafe:
case CallArgument:
case TemporarilyEscaping:
case PreconcurrencyImport:
return ProtocolConformanceRef::forInvalid();
}
}
/// Get information about the call argument.
///
/// Produces the argument name, argument index, and argument expression for
/// a unsafe use describing a call argument.
std::tuple<Identifier, unsigned, Expr *> getCallArgument() const {
assert(getKind() == CallArgument);
return std::make_tuple(
Identifier::getFromOpaquePointer(storage.callArgument.argumentName),
storage.callArgument.argumentIndex,
storage.callArgument.argument);
}
};
} // end namespace swift

View File

@@ -2699,6 +2699,7 @@ diagnoseDeclUnsafe(ConcreteDeclRef declRef, SourceRange R,
SourceLoc diagLoc = call ? call->getLoc() : R.Start;
enumerateUnsafeUses(declRef, diagLoc, call != nullptr,
/*skipTypeCheck=*/false,
[&](UnsafeUse unsafeUse) {
unsafeUses->push_back(unsafeUse);
return false;

View File

@@ -335,7 +335,7 @@ private:
PolymorphicEffectKind RethrowsKind = PolymorphicEffectKind::None;
PolymorphicEffectKind ReasyncKind = PolymorphicEffectKind::None;
SubstitutionMap Substitutions;
bool AppliedSelf = false;
SelfApplyExpr *AppliedSelf = nullptr;
public:
explicit AbstractFunction(Kind kind, Expr *fn)
@@ -344,7 +344,7 @@ public:
}
explicit AbstractFunction(AbstractFunctionDecl *fn, SubstitutionMap subs,
bool appliedSelf)
SelfApplyExpr *appliedSelf)
: TheKind(Kind::Function),
RethrowsKind(fn->getPolymorphicEffectKind(EffectKind::Throws)),
ReasyncKind(fn->getPolymorphicEffectKind(EffectKind::Async)),
@@ -370,7 +370,15 @@ public:
switch (kind) {
case EffectKind::Throws: return RethrowsKind;
case EffectKind::Async: return ReasyncKind;
case EffectKind::Unsafe: return swift::PolymorphicEffectKind::None;
case EffectKind::Unsafe:
switch (getExplicitSafety()) {
case ExplicitSafety::Safe:
return PolymorphicEffectKind::None;
case ExplicitSafety::Unsafe:
return PolymorphicEffectKind::Always;
case ExplicitSafety::Unspecified:
return PolymorphicEffectKind::ByClosure;
}
}
llvm_unreachable("Bad effect kind");
}
@@ -413,6 +421,22 @@ public:
}
}
Type getOrigParamSelfType() const {
switch (getKind()) {
case Kind::Opaque:
case Kind::Closure:
case Kind::Parameter:
return Type();
case Kind::Function:
if (getFunction()->hasImplicitSelfDecl() && AppliedSelf) {
return getFunction()->getImplicitSelfDecl()->getInterfaceType();
}
return Type();
}
}
bool isAutoClosure() const {
if (getKind() == Kind::Closure)
return isa<AutoClosureExpr>(getClosure());
@@ -436,20 +460,37 @@ public:
return TheExpr;
}
SelfApplyExpr *getAppliedSelf() const {
return AppliedSelf;
}
SubstitutionMap getSubstitutions() const {
return Substitutions;
}
static AbstractFunction getAppliedFn(ApplyExpr *apply) {
ExplicitSafety getExplicitSafety() const {
switch (getKind()) {
case Kind::Closure:
case Kind::Opaque:
case Kind::Parameter:
return ExplicitSafety::Unspecified;
case Kind::Function:
return getFunction()->getExplicitSafety();
}
}
static AbstractFunction getAppliedFn(ApplyExpr *apply,
Expr **calleeFn = nullptr) {
Expr *fn = apply->getFn()->getValueProvidingExpr();
bool appliedSelf = false;
SelfApplyExpr *appliedSelf = nullptr;
if (auto *selfCall = dyn_cast<SelfApplyExpr>(fn)) {
appliedSelf = selfCall;
fn = selfCall->getFn()->getValueProvidingExpr();
appliedSelf = true;
}
return decomposeFunction(fn, appliedSelf);
return decomposeFunction(fn, appliedSelf, calleeFn);
}
bool isPreconcurrency() const {
@@ -468,7 +509,9 @@ public:
}
}
static AbstractFunction decomposeFunction(Expr *fn, bool appliedSelf) {
static AbstractFunction decomposeFunction(Expr *fn,
SelfApplyExpr *appliedSelf,
Expr **calleeFn) {
assert(fn->getValueProvidingExpr() == fn);
while (true) {
@@ -500,7 +543,11 @@ public:
break;
}
}
// Tell our caller about the callee, if they asked for it.
if (calleeFn)
*calleeFn = fn;
// Constructor delegation.
if (auto otherCtorDeclRef = dyn_cast<OtherConstructorDeclRefExpr>(fn)) {
return AbstractFunction(otherCtorDeclRef->getDecl(),
@@ -616,10 +663,12 @@ public:
/*isImplicitlyAsync=*/false,
/*isImplicitlyThrows=*/false);
} else if (auto *LE = dyn_cast<LiteralExpr>(E)) {
recurse = asImpl().checkDeclRef(LE, LE->getInitializer(), LE->getLoc(),
/*isEvaluated=*/false,
/*isImplicitlyAsync=*/false,
/*isImplicitlyThrows=*/false);
if (LE->getInitializer()) {
recurse = asImpl().checkDeclRef(LE, LE->getInitializer(), LE->getLoc(),
/*isEvaluated=*/false,
/*isImplicitlyAsync=*/false,
/*isImplicitlyThrows=*/false);
}
} else if (auto *CE = dyn_cast<CollectionExpr>(E)) {
recurse = asImpl().checkDeclRef(CE, CE->getInitializer(), CE->getLoc(),
/*isEvaluated=*/false,
@@ -1128,6 +1177,7 @@ public:
static Classification
forDeclRef(ConcreteDeclRef declRef, ConditionalEffectKind conditionalKind,
PotentialEffectReason reason, SourceLoc loc,
bool skipTypeCheck,
std::optional<EffectKind> onlyEffect = std::nullopt) {
ASTContext &ctx = declRef.getDecl()->getASTContext();
@@ -1139,7 +1189,8 @@ public:
// If we're tracking "unsafe" effects, compute them here.
if (considerUnsafe) {
enumerateUnsafeUses(declRef, loc, /*isCall=*/false, [&](UnsafeUse use) {
enumerateUnsafeUses(declRef, loc, /*isCall=*/false, skipTypeCheck,
[&](UnsafeUse use) {
result.recordUnsafeUse(use);
return false;
});
@@ -1492,7 +1543,8 @@ public:
if (auto getter = getEffectfulGetOnlyAccessor(member)) {
reason = getKindOfEffectfulProp(member);
classification = Classification::forDeclRef(
getter, ConditionalEffectKind::Always, reason, E->getLoc());
getter, ConditionalEffectKind::Always, reason, E->getLoc(),
assumedSafeArguments && assumedSafeArguments->contains(E));
} else if (isa<SubscriptExpr>(E) || isa<DynamicSubscriptExpr>(E)) {
reason = PotentialEffectReason::forSubscriptAccess();
}
@@ -1509,6 +1561,7 @@ public:
classification.merge(
Classification::forDeclRef(
member, ConditionalEffectKind::Always, reason, E->getLoc(),
assumedSafeArguments && assumedSafeArguments->contains(E),
EffectKind::Unsafe));
}
@@ -1535,7 +1588,8 @@ public:
Classification classifyDeclRef(ConcreteDeclRef declRef, SourceLoc loc,
bool isImplicitlyAsync,
bool isImplicitlyThrows) {
bool isImplicitlyThrows,
bool ignoreUnsafeType) {
if (!declRef)
return Classification::forInvalidCode();
@@ -1545,12 +1599,14 @@ public:
if (auto getter = getEffectfulGetOnlyAccessor(declRef)) {
reason = getKindOfEffectfulProp(declRef);
classification = Classification::forDeclRef(
getter, ConditionalEffectKind::Always, reason, loc);
getter, ConditionalEffectKind::Always, reason, loc,
ignoreUnsafeType);
} else if (isa<VarDecl>(declRef.getDecl())) {
// Handle async let.
reason = PotentialEffectReason::forAsyncLet();
classification = Classification::forDeclRef(
declRef, ConditionalEffectKind::Always, reason, loc);
declRef, ConditionalEffectKind::Always, reason, loc,
ignoreUnsafeType);
}
classification.setDowngradeToWarning(
@@ -1565,18 +1621,12 @@ public:
classification.merge(
Classification::forDeclRef(
declRef, ConditionalEffectKind::Always, reason, loc,
EffectKind::Unsafe));
ignoreUnsafeType, EffectKind::Unsafe));
}
return classification;
}
Classification classifyDeclRef(DeclRefExpr *E) {
return classifyDeclRef(
E->getDeclRef(), E->getLoc(), E->isImplicitlyAsync().has_value(),
E->isImplicitlyThrows());
}
Classification classifyThrow(ThrowStmt *S) {
Expr *thrownValue = S->getSubExpr();
if (!thrownValue)
@@ -1610,22 +1660,13 @@ public:
assert(!E->isImplicitlyAsync());
}
// If this is calling a @safe function, treat local variables as being
// safe.
if (assumedSafeArguments) {
if (auto calleeDecl = E->getCalledValue()) {
if (calleeDecl->getExplicitSafety() == ExplicitSafety::Safe) {
markArgumentsSafe(E->getArgs(), *assumedSafeArguments);
}
}
}
auto type = E->getFn()->getType();
if (!type) return Classification::forInvalidCode();
auto fnType = type->getAs<AnyFunctionType>();
if (!fnType) return Classification::forInvalidCode();
auto fnRef = AbstractFunction::getAppliedFn(E);
Expr *calleeFn = nullptr;
auto fnRef = AbstractFunction::getAppliedFn(E, &calleeFn);
auto substitutions = fnRef.getSubstitutions();
const bool hasAnyConformances =
llvm::any_of(substitutions.getConformances(),
@@ -1633,6 +1674,29 @@ public:
auto *requirement = conformance.getProtocol();
return !requirement->getInvertibleProtocolKind();
});
bool hasUnspecifiedSafety =
fnRef.getExplicitSafety() == ExplicitSafety::Unspecified;
if (assumedSafeArguments) {
// When calling a function that has no explicitly-written @safe or
// @unsafe, we diagnose the safety of the arguments---not the function
// itself.
if (fnRef.getExplicitSafety() == ExplicitSafety::Unspecified) {
if (calleeFn)
assumedSafeArguments->insert(calleeFn);
if (auto appliedSelf = fnRef.getAppliedSelf())
assumedSafeArguments->insert(appliedSelf);
}
// If this is calling a @safe function, treat local variables as being
// safe.
if (fnRef.getExplicitSafety() == ExplicitSafety::Safe) {
markArgumentsSafe(E->getArgs(), *assumedSafeArguments);
}
}
ASTContext &ctx = type->getASTContext();
// If the function doesn't have any effects or conformances, we're done
// here.
@@ -1640,12 +1704,12 @@ public:
!E->implicitlyThrows() &&
!fnType->isAsync() &&
!E->isImplicitlyAsync() &&
!hasAnyConformances) {
!hasAnyConformances &&
(fnRef.getExplicitSafety() == ExplicitSafety::Safe ||
!ctx.LangOpts.hasFeature(Feature::StrictMemorySafety))) {
return Classification();
}
ASTContext &ctx = type->getASTContext();
// Decompose the application.
auto *args = E->getArgs();
@@ -1667,7 +1731,8 @@ public:
(fnRef.isPreconcurrency() || isContextPreconcurrency()));
auto classifyApplyEffect = [&](EffectKind kind) {
if (!fnType->hasEffect(kind) &&
if (kind != EffectKind::Unsafe &&
!fnType->hasEffect(kind) &&
!(kind == EffectKind::Async && E->isImplicitlyAsync()) &&
!(kind == EffectKind::Throws && E->implicitlyThrows())) {
return;
@@ -1700,6 +1765,22 @@ public:
if (polyKind == PolymorphicEffectKind::ByConformance ||
polyKind == PolymorphicEffectKind::AsyncSequenceRethrows) {
LLVM_FALLTHROUGH;
} else if (kind == EffectKind::Unsafe) {
// For explicitly @unsafe functions, the entire call is considered
// unsafe.
AbstractFunctionDecl *calleeFn =
fnRef.getKind() == AbstractFunction::Function
? fnRef.getFunction()
: nullptr;
result.merge(
Classification::forUnsafe(
{
UnsafeUse::forReferenceToUnsafe(
calleeFn, true, fnRef.getType(), E->getLoc())
}
));
return;
} else if (RethrowsDC &&
fnRef.getKind() == AbstractFunction::Function &&
isRethrowLikeTypedThrows(fnRef.getFunction())) {
@@ -1763,10 +1844,17 @@ public:
return;
}
AbstractFunctionDecl *calleeFn =
fnRef.getKind() == AbstractFunction::Function
? fnRef.getFunction()
: nullptr;
for (unsigned i = 0, e = args->size(); i < e; ++i) {
Type origParamType = fnRef.getOrigParamInterfaceType(i);
auto argClassification = classifyArgument(
args->getExpr(i), origParamType, fnRef.getSubstitutions(), kind);
E, calleeFn, args->getLabel(i), i,
args->getExpr(i), origParamType, fnRef.getSubstitutions(),
kind);
// Rethrows is untyped, so adjust the thrown error type.
if (kind == EffectKind::Throws &&
@@ -1777,6 +1865,17 @@ public:
result.merge(argClassification);
}
// The only effect on "self" is unsafe, so check that.
if (kind == EffectKind::Unsafe) {
if (auto appliedSelf = fnRef.getAppliedSelf()) {
Type origParamType = fnRef.getOrigParamSelfType();
auto selfClassification = classifyArgument(
E, calleeFn, ctx.Id_self, 0, appliedSelf->getBase(),
origParamType, fnRef.getSubstitutions(), kind);
result.merge(selfClassification);
}
}
return;
}
@@ -1818,6 +1917,14 @@ public:
classifyApplyEffect(EffectKind::Throws);
classifyApplyEffect(EffectKind::Async);
// If the safety of the callee is unspecified, check the safety of the
// arguments specifically.
if (hasUnspecifiedSafety &&
ctx.LangOpts.hasFeature(Feature::StrictMemorySafety) &&
!(assumedSafeArguments && assumedSafeArguments->contains(E))) {
classifyApplyEffect(EffectKind::Unsafe);
}
return result;
}
@@ -2006,7 +2113,8 @@ private:
conditional = ConditionalEffectKind::Conditional;
return Classification::forDeclRef(
ConcreteDeclRef(fn, subs), conditional, reason, fn->getLoc())
ConcreteDeclRef(fn, subs), conditional, reason, fn->getLoc(),
/*ignoreUnsafeType=*/false)
.onlyEffect(kind);
}
@@ -2099,7 +2207,8 @@ private:
if (isEvaluated) {
classification.merge(
Self.classifyDeclRef(
declRef, loc, isImplicitlyAsync, isImplicitlyThrows
declRef, loc, isImplicitlyAsync, isImplicitlyThrows,
/*ignoreUnsafeType=*/true
).onlyThrowing());
}
@@ -2348,10 +2457,10 @@ private:
bool isEvaluated,
bool isImplicitlyAsync,
bool isImplicitlyThrows) {
if (!assumedSafeArguments.contains(expr)) {
classification.merge(
Self.classifyDeclRef(declRef, loc, false, false).onlyUnsafe());
}
classification.merge(
Self.classifyDeclRef(declRef, loc, false, false,
assumedSafeArguments.contains(expr))
.onlyUnsafe());
return ShouldRecurse;
}
@@ -2382,23 +2491,18 @@ private:
}
ShouldRecurse_t checkWithSubstitutionMap(Expr *E, SubstitutionMap subs) {
if (!assumedSafeArguments.contains(E)) {
classification.merge(
Classification::forSubstitutionMap(subs, E->getLoc())
.onlyUnsafe());
}
classification.merge(
Classification::forSubstitutionMap(subs, E->getLoc())
.onlyUnsafe());
return ShouldRecurse;
}
ShouldRecurse_t checkWithConformances(
Expr *E,
ArrayRef<ProtocolConformanceRef> conformances) {
if (!assumedSafeArguments.contains(E)) {
classification.merge(
Classification::forConformances(conformances, E->getLoc())
.onlyUnsafe());
}
classification.merge(
Classification::forConformances(conformances, E->getLoc())
.onlyUnsafe());
return ShouldRecurse;
}
@@ -2481,19 +2585,65 @@ private:
return result;
}
/// Determine whether the given expression represents "nil" or
/// Optional<T>.none.
static bool isNilOrNone(Expr *arg) {
// If this argument is `nil` literal, it doesn't cause the call to throw.
if (isa<NilLiteralExpr>(arg)) {
return !arg->getType()->getOptionalObjectType().isNull();
}
// Neither does 'Optional<T>.none'.
if (auto *DSCE = dyn_cast<DotSyntaxCallExpr>(arg)) {
if (auto *DE = dyn_cast<DeclRefExpr>(DSCE->getFn())) {
auto &ctx = DE->getType()->getASTContext();
return DE->getDecl() == ctx.getOptionalNoneDecl();
}
return false;
}
return false;
}
/// Classify an argument being passed to a rethrows/reasync function.
Classification classifyArgument(
Expr *arg, Type paramType, SubstitutionMap subs, EffectKind kind) {
ApplyExpr *call, const Decl* calleeDecl, Identifier argName,
unsigned argIndex, Expr *arg, Type paramType, SubstitutionMap subs,
EffectKind kind) {
// Check for an unsafe argument.
if (kind == EffectKind::Unsafe) {
if (arg->getType()->isUnsafe()) {
// Dig out the underlying argument for special case checks.
Expr *underlyingArg = arg;
if (auto *defaultArg = dyn_cast<DefaultArgumentExpr>(arg)) {
if (defaultArg->isCallerSide()) {
underlyingArg = defaultArg->getCallerSideDefaultExpr();
}
}
// nil / Optional<T>.none values cannot introduce safety issues because
// they don't reference anything.
if (isNilOrNone(underlyingArg))
return Classification();
return Classification::forUnsafe({
UnsafeUse::forCallArgument(call, calleeDecl, paramType, argName,
argIndex, arg)
});
}
return Classification();
}
arg = arg->getValueProvidingExpr();
if (auto *defaultArg = dyn_cast<DefaultArgumentExpr>(arg)) {
// Special-case a 'nil' default argument, which is known not to throw.
if (defaultArg->isCallerSide()) {
auto *callerSideArg = defaultArg->getCallerSideDefaultExpr();
if (isa<NilLiteralExpr>(callerSideArg)) {
if (callerSideArg->getType()->getOptionalObjectType())
return Classification();
}
if (isNilOrNone(callerSideArg))
return Classification();
}
return classifyArgumentByType(arg->getType(), subs,
@@ -2502,26 +2652,17 @@ private:
kind);
}
// If this argument is `nil` literal, it doesn't cause the call to throw.
if (isa<NilLiteralExpr>(arg)) {
if (arg->getType()->getOptionalObjectType())
return Classification();
}
// Neither does 'Optional<T>.none'.
if (auto *DSCE = dyn_cast<DotSyntaxCallExpr>(arg)) {
if (auto *DE = dyn_cast<DeclRefExpr>(DSCE->getFn())) {
auto &ctx = paramType->getASTContext();
if (DE->getDecl() == ctx.getOptionalNoneDecl())
return Classification();
}
}
// If this argument is `nil` literal or Optional<T>.none, it does not
// have any effects.
if (isNilOrNone(arg))
return Classification();
// If the parameter was structurally a tuple, try to look through the
// various tuple operations.
if (auto paramTupleType = dyn_cast<TupleType>(paramType.getPointer())) {
if (auto tuple = dyn_cast<TupleExpr>(arg)) {
return classifyTupleArgument(tuple, paramTupleType, subs, kind);
return classifyTupleArgument(call, calleeDecl, argName, argIndex,
tuple, paramTupleType, subs, kind);
}
if (paramTupleType->getNumElements() != 1) {
@@ -2555,7 +2696,7 @@ private:
// Decompose the function reference, then consider the type
// of the decomposed function.
AbstractFunction fn = AbstractFunction::decomposeFunction(
arg, /*appliedSelf=*/false);
arg, /*appliedSelf=*/nullptr, /*calleeFn=*/nullptr);
// If it doesn't have function type, we must have invalid code.
Type argType = fn.getType();
@@ -2574,7 +2715,11 @@ private:
}
/// Classify an argument to a rethrows/reasync function that's a tuple literal.
Classification classifyTupleArgument(TupleExpr *tuple,
Classification classifyTupleArgument(ApplyExpr *call,
const Decl* calleeDecl,
Identifier argName,
unsigned argIndex,
TupleExpr *tuple,
TupleType *paramTupleType,
SubstitutionMap subs,
EffectKind kind) {
@@ -2583,7 +2728,8 @@ private:
Classification result;
for (unsigned i : indices(tuple->getElements())) {
result.merge(classifyArgument(tuple->getElement(i),
result.merge(classifyArgument(call, calleeDecl, argName, argIndex,
tuple->getElement(i),
paramTupleType->getElementType(i),
subs, kind));
}
@@ -3833,11 +3979,6 @@ private:
auto classification = Classification::forSubstitutionMap(
subs, E->getLoc());
// If this expression is covered as a safe argument, drop the unsafe
// classification.
if (assumedSafeArguments.contains(E))
classification = classification.withoutUnsafe();
checkEffectSite(E, /*requiresTry=*/false, classification);
return ShouldRecurse;
}
@@ -3848,11 +3989,6 @@ private:
auto classification = Classification::forConformances(
conformances, E->getLoc());
// If this expression is covered as a safe argument, drop the unsafe
// classification.
if (assumedSafeArguments.contains(E))
classification = classification.withoutUnsafe();
checkEffectSite(E, /*requiresTry=*/false, classification);
return ShouldRecurse;
}
@@ -4040,15 +4176,12 @@ private:
bool isImplicitlyAsync,
bool isImplicitlyThrows) {
if (auto classification = getApplyClassifier().classifyDeclRef(
declRef, loc, isImplicitlyAsync, isImplicitlyThrows)) {
declRef, loc, isImplicitlyAsync, isImplicitlyThrows,
assumedSafeArguments.contains(expr))) {
// If we aren't evaluating the reference, we only care about 'unsafe'.
if (!isEvaluated)
classification = classification.onlyUnsafe();
// If we're treating this expression as a safe argument, drop 'unsafe'.
if (assumedSafeArguments.contains(expr))
classification = classification.withoutUnsafe();
auto throwDest = checkEffectSite(
expr, classification.hasThrows(), classification);
@@ -4407,7 +4540,8 @@ private:
// Unsafety in the next/nextElement call is covered by an "unsafe" effect.
if (classification.hasUnsafe()) {
// If there is no such effect, complain.
if (S->getUnsafeLoc().isInvalid()) {
if (S->getUnsafeLoc().isInvalid() &&
Ctx.LangOpts.hasFeature(Feature::StrictMemorySafety)) {
auto insertionLoc = S->getPattern()->getStartLoc();
Ctx.Diags.diagnose(S->getForLoc(), diag::for_unsafe_without_unsafe)
.fixItInsert(insertionLoc, "unsafe ");
@@ -4649,10 +4783,12 @@ private:
void diagnoseUncoveredUnsafeSite(
const Expr *anchor, ArrayRef<UnsafeUse> unsafeUses) {
if (!Ctx.LangOpts.hasFeature(Feature::StrictMemorySafety))
return;
const auto &[loc, insertText] = getFixItForUncoveredSite(anchor, "unsafe");
Ctx.Diags.diagnose(anchor->getStartLoc(), diag::unsafe_without_unsafe)
.fixItInsert(loc, insertText)
.highlight(anchor->getSourceRange());
.fixItInsert(loc, insertText);
for (const auto &unsafeUse : unsafeUses) {
diagnoseUnsafeUse(unsafeUse);
}

View File

@@ -151,6 +151,40 @@ void swift::diagnoseUnsafeUse(const UnsafeUse &use) {
return;
}
case UnsafeUse::CallArgument: {
auto [argumentName, argumentIndex, argument] = use.getCallArgument();
Type paramType = use.getType();
ASTContext &ctx = paramType->getASTContext();
SourceLoc loc = argument->getLoc();
if (loc.isInvalid())
loc = use.getLocation();
if (auto calleeDecl = dyn_cast_or_null<ValueDecl>(use.getDecl())) {
if (argumentName.empty()) {
ctx.Diags.diagnose(
loc,
diag::note_unsafe_call_decl_argument_indexed,
calleeDecl, argumentIndex, paramType)
.highlight(argument->getSourceRange());
} else {
ctx.Diags.diagnose(
loc,
diag::note_unsafe_call_decl_argument_named,
calleeDecl, argumentName, paramType)
.highlight(argument->getSourceRange());
}
} else {
ctx.Diags.diagnose(
loc,
diag::note_unsafe_call_argument_indexed,
argumentIndex, paramType)
.highlight(argument->getSourceRange());
}
return;
}
case UnsafeUse::TemporarilyEscaping: {
Type type = use.getType();
ASTContext &ctx = type->getASTContext();
@@ -177,6 +211,7 @@ static bool isReferenceToNonisolatedUnsafe(ValueDecl *decl) {
bool swift::enumerateUnsafeUses(ConcreteDeclRef declRef,
SourceLoc loc,
bool isCall,
bool skipTypeCheck,
llvm::function_ref<bool(UnsafeUse)> fn) {
// If the declaration is explicitly unsafe, note that.
auto decl = declRef.getDecl();
@@ -203,7 +238,7 @@ bool swift::enumerateUnsafeUses(ConcreteDeclRef declRef,
// If the type of this declaration involves unsafe types, diagnose that.
ASTContext &ctx = decl->getASTContext();
auto subs = declRef.getSubstitutions();
{
if (!skipTypeCheck) {
auto type = decl->getInterfaceType();
if (subs) {
if (auto *genericFnType = type->getAs<GenericFunctionType>())
@@ -343,7 +378,8 @@ bool swift::enumerateUnsafeUses(SubstitutionMap subs,
bool swift::isUnsafe(ConcreteDeclRef declRef) {
return enumerateUnsafeUses(
declRef, SourceLoc(), /*isCall=*/false, [&](UnsafeUse) {
declRef, SourceLoc(), /*isCall=*/false, /*skipTypeCheck=*/false,
[&](UnsafeUse) {
return true;
});
}

View File

@@ -34,6 +34,7 @@ void diagnoseUnsafeUse(const UnsafeUse &use);
bool enumerateUnsafeUses(ConcreteDeclRef declRef,
SourceLoc loc,
bool isCall,
bool skipTypeCheck,
llvm::function_ref<bool(UnsafeUse)> fn);
/// Enumerate all of the unsafe uses that occur within this array of protocol

View File

@@ -38,7 +38,7 @@ class NotSafeSubclass: NotSafe {
ns.stillUnsafe() // expected-warning{{expression uses unsafe constructs but is not marked with 'unsafe'}}{{documentation-file=strict-memory-safety}}
// expected-note@-1{{reference to parameter 'ns' involves unsafe type 'NotSafe'}}
// expected-note@-2{{reference to instance method 'stillUnsafe()' involves unsafe type 'NotSafe'}}
// expected-note@-2{{argument 'self' in call to instance method 'stillUnsafe' has unsafe type 'NotSafe'}}
}
@safe func testImpliedSafetySubclass(ns: NotSafeSubclass) {

View File

@@ -11,7 +11,9 @@ func iAmImpliedUnsafe() -> UnsafeType? { nil }
@unsafe
func labeledUnsafe(_: UnsafeType) {
unsafe iAmUnsafe()
let _ = unsafe iAmImpliedUnsafe()
let _ = iAmImpliedUnsafe() // okay, since the unsafety is captured in the result type
// expected-warning@+1{{expression uses unsafe constructs but is not marked with 'unsafe'}}
let _ = iAmImpliedUnsafe // expected-note{{reference to global function 'iAmImpliedUnsafe()' involves unsafe type 'UnsafeType'}}
}

View File

@@ -49,7 +49,7 @@ extension ConformsToMultiP: MultiP {
// expected-note@-1{{unsafe type 'UnsafeSuper' cannot satisfy safe associated type 'Ptr'}}
@unsafe func f() -> UnsafeSuper {
.init() // expected-warning{{expression uses unsafe constructs but is not marked with 'unsafe'}}
// expected-note@-1{{reference to initializer 'init()' involves unsafe type 'UnsafeSuper'}}
// expected-note@-1{{argument 'self' in call to initializer 'init' has unsafe type 'UnsafeSuper'}}
}
}
@@ -141,11 +141,13 @@ func testRHS(b: Bool, x: Int) {
}
// -----------------------------------------------------------------------
// Declaration references
// Calls and declaration references
// -----------------------------------------------------------------------
@unsafe func unsafeF() { }
@unsafe var unsafeVar: Int = 0
func acceptUnsafeArgument(_: UnsafePointer<Int>?) { }
func acceptUnsafeNamedArgument(arg: UnsafePointer<Int>?) { }
func testMe(
_ pointer: PointerType,
@@ -156,11 +158,28 @@ func testMe(
// expected-warning@+1{{expression uses unsafe constructs but is not marked with 'unsafe'}}{{7-7=unsafe }}
_ = unsafeVar // expected-note{{reference to unsafe var 'unsafeVar'}}
// expected-warning@+1{{expression uses unsafe constructs but is not marked with 'unsafe'}}{{3-3=unsafe }}
unsafeSuper.f() // expected-note{{reference to instance method 'f()' involves unsafe type 'UnsafeSuper'}}
unsafeSuper.f() // expected-note{{argument 'self' in call to instance method 'f' has unsafe type 'UnsafeSuper'}}
// expected-note@-1{{reference to parameter 'unsafeSuper' involves unsafe type 'UnsafeSuper'}}
_ = getPointers() // unsafe return is fine
// expected-warning@+1{{expression uses unsafe constructs but is not marked with 'unsafe'}}{{7-7=unsafe }}
_ = getPointers() // expected-note{{reference to global function 'getPointers()' involves unsafe type 'PointerType'}}
_ = getPointers // expected-note{{reference to global function 'getPointers()' involves unsafe type 'PointerType'}}
var i = 17
// expected-warning@+1{{expression uses unsafe constructs but is not marked with 'unsafe'}}
acceptUnsafeArgument(&i) // expected-note{{argument #0 in call to global function 'acceptUnsafeArgument' has unsafe type 'UnsafePointer<Int>?'}}
// expected-warning@+1{{expression uses unsafe constructs but is not marked with 'unsafe'}}
acceptUnsafeNamedArgument(arg: &i) // expected-note{{argument 'arg' in call to global function 'acceptUnsafeNamedArgument' has unsafe type 'UnsafePointer<Int>?'}}
// expected-warning@+1{{expression uses unsafe constructs but is not marked with 'unsafe'}}
let fn: (UnsafePointer<Int>?) -> Void = acceptUnsafeArgument // expected-note{{reference to global function 'acceptUnsafeArgument' involves unsafe type 'UnsafePointer<Int>'}}
// expected-warning@+1{{expression uses unsafe constructs but is not marked with 'unsafe'}}
fn(&i) // expected-note{{argument #0 in call has unsafe type 'UnsafePointer<Int>?'}}
// Nil isn't unsafe
acceptUnsafeArgument(nil)
acceptUnsafeNamedArgument(arg: nil)
fn(nil)
}
// -----------------------------------------------------------------------

View File

@@ -13,7 +13,10 @@ func testUnsafe(_ ut: UnsafeType) {
var array: [CInt] = [1, 2, 3, 4, 5]
// expected-warning@+1{{expression uses unsafe constructs but is not marked with 'unsafe'}}{{3-3=unsafe }}
print_ints(&array, CInt(array.count))
// expected-note@-1{{reference to global function 'print_ints' involves unsafe type 'UnsafeMutablePointer<Int32>'}}
// expected-note@-1{{argument #0 in call to global function 'print_ints' has unsafe type 'UnsafeMutablePointer<Int32>?'}}
// expected-warning@+1{{expression uses unsafe constructs but is not marked with 'unsafe'}}{{7-7=unsafe }}
_ = print_ints // expected-note{{reference to global function 'print_ints' involves unsafe type 'UnsafeMutablePointer<Int32>'}}
}
// Reference a typealias that isn't itself @unsafe, but refers to an unsafe
@@ -25,7 +28,7 @@ func testUnsafeThroughAlias(_ ut: UnsafeTypeAlias) {
func callThroughAlias(ut: UnsafeTypeAlias) {
// expected-warning@+1{{expression uses unsafe constructs but is not marked with 'unsafe'}}
testUnsafeThroughAlias(ut) // expected-note{{reference to global function 'testUnsafeThroughAlias' involves unsafe type 'UnsafeTypeAlias' (aka 'PointerType')}}
testUnsafeThroughAlias(ut) // expected-note{{argument #0 in call to global function 'testUnsafeThroughAlias' has unsafe type 'UnsafeTypeAlias' (aka 'PointerType')}}
// expected-note@-1{{reference to parameter 'ut' involves unsafe type 'UnsafeTypeAlias' (aka 'PointerType')}}
}

View File

@@ -9,7 +9,7 @@ func test(
) {
var array = [1, 2, 3]
// expected-warning@+2{{expression uses unsafe constructs but is not marked with 'unsafe'}}{{3-3=unsafe }}
// expected-note@+1{{reference to instance method 'withUnsafeBufferPointer' involves unsafe type 'UnsafeBufferPointer<Int>'}}
// expected-note@+1{{argument #0 in call to instance method 'withUnsafeBufferPointer' has unsafe type '(UnsafeBufferPointer<Element>) throws(E) -> R'}}
array.withUnsafeBufferPointer{ buffer in
// expected-warning@+1{{expression uses unsafe constructs but is not marked with 'unsafe'}}{{5-5=unsafe }}
print(buffer) // expected-note{{reference to parameter 'buffer' involves unsafe type 'UnsafeBufferPointer<Int>'}}