Skip to content

Consume: warn about no-op consumes to be fixed #75449

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -7749,6 +7749,8 @@ ERROR(consume_expression_needed_for_cast,none,
"implicit conversion to %0 is consuming", (Type))
NOTE(add_consume_to_silence,none,
"add 'consume' to make consumption explicit", ())
WARNING(consume_of_bitwisecopyable_noop,none,
"'consume' applied to bitwise-copyable type %0 has no effect", (Type))
ERROR(consume_expression_not_passed_lvalue,none,
"'consume' can only be applied to a local binding ('let', 'var', or parameter)", ())
ERROR(consume_expression_partial_copyable,none,
Expand Down
50 changes: 50 additions & 0 deletions lib/Sema/MiscDiagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,56 @@ static void diagSyntacticUseRestrictions(const Expr *E, const DeclContext *DC,
consumeExpr->getSubExpr());
for (auto &diag : diags)
diag.emit(Ctx);

// As of now, SE-366 is not correctly implemented (rdar://102780553),
// so warn about certain consume's being no-ops today that will no longer
// be a no-op in the future once we fix this.
if (auto ty = consumeExpr->getType()) {
bool shouldWarn = true;

// Look through any load.
auto *expr = consumeExpr->getSubExpr();
if (auto *load = dyn_cast<LoadExpr>(expr))
expr = load->getSubExpr();

// Don't warn if explicit ownership was provided on a parameter.
// Those seem to be checked just fine in SIL.
if (auto *declRef = dyn_cast<DeclRefExpr>(expr)) {
if (auto *decl = declRef->getDecl()) {
if (auto *paramDecl = dyn_cast<ParamDecl>(decl)) {
switch (paramDecl->getSpecifier()) {
case ParamSpecifier::InOut:
case ParamSpecifier::Borrowing:
case ParamSpecifier::Consuming:
case ParamSpecifier::ImplicitlyCopyableConsuming:
shouldWarn = false;
break;
case ParamSpecifier::Default:
case ParamSpecifier::LegacyShared:
case ParamSpecifier::LegacyOwned:
break; // warn
}
}
}
}

// Only warn about obviously concrete BitwiseCopyable types, since we
// know those won't get checked for consumption.
if (diags.empty() &&
shouldWarn &&
!ty->hasError() &&
!ty->hasTypeParameter() &&
!ty->hasUnboundGenericType() &&
!ty->hasArchetype()) {
auto bitCopy = Ctx.getProtocol(KnownProtocolKind::BitwiseCopyable);
if (DC->getParentModule()->checkConformance(ty, bitCopy)) {
Ctx.Diags.diagnose(consumeExpr->getLoc(),
diag::consume_of_bitwisecopyable_noop, ty)
.fixItRemoveChars(consumeExpr->getStartLoc(),
consumeExpr->getSubExpr()->getStartLoc());
}
}
}
}

void checkCopyExpr(CopyExpr *copyExpr) {
Expand Down
2 changes: 1 addition & 1 deletion test/Parse/move_expr.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

var global: Int = 5
func testGlobal() {
let _ = consume global
let _ = consume global // expected-warning {{'consume' applied to bitwise-copyable type 'Int' has no effect}}
}

func testLet() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -725,7 +725,14 @@ func consumeInitdArray() {
func isNegative(_ c: consuming Int) -> Bool { return c < 0 }
func consumeInt() {
var g = 0 // expected-warning{{variable 'g' was never mutated; consider changing to 'let' constant}}
isNegative(consume g) // expected-warning{{result of call to 'isNegative' is unused}}
// expected-error@-1 {{'g' used after consume}}

_ = isNegative(consume g) // expected-note {{consumed here}}
// expected-warning@-1 {{'consume' applied to bitwise-copyable type 'Int' has no effect}}

_ = isNegative(consume g) // expected-note {{used here}}
// expected-error@-1 {{'consume' applied to value that the compiler does not support. This is a compiler bug. Please file a bug with a small example of the bug}}
// expected-warning@-2 {{'consume' applied to bitwise-copyable type 'Int' has no effect}}
}

//////////////////////
Expand Down
60 changes: 60 additions & 0 deletions test/Sema/consume_operator_noop_warning.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
// RUN: %target-swift-emit-sil %s -verify -sil-verify-all

struct Point {
let x: Float
let y: Float
}

struct ConditionallyBC<T> {
var t: T
}
extension ConditionallyBC: BitwiseCopyable where T: BitwiseCopyable {}

func test<T, BCG: BitwiseCopyable>(_ t: T, // expected-error {{'t' is borrowed and cannot be consumed}}
_ bcg: BCG, // expected-error {{'bcg' is borrowed and cannot be consumed}}
_ cbcg_generic: ConditionallyBC<BCG>, // expected-error {{'cbcg_generic' is borrowed and cannot be consumed}}
_ maybeBCG: BCG?, // expected-error {{'maybeBCG' is borrowed and cannot be consumed}}
_ maybeT: T?, // expected-error {{'maybeT' is borrowed and cannot be consumed}}
_ anyBC: any BitwiseCopyable, // expected-error {{'anyBC' is borrowed and cannot be consumed}}
_ x: Int,
_ point: Point,
_ cbcg_concrete: ConditionallyBC<Int>,
_ maybeFloat: Float?) {
_ = consume t // expected-note {{consumed here}}
_ = consume bcg // expected-note {{consumed here}}
_ = consume cbcg_generic // expected-note {{consumed here}}
_ = consume maybeBCG // expected-note {{consumed here}}
_ = consume maybeT // expected-note {{consumed here}}
_ = consume anyBC // expected-note {{consumed here}}

_ = consume x // expected-warning {{'consume' applied to bitwise-copyable type 'Int' has no effect}}{{7-15=}}
_ = consume point // expected-warning {{'consume' applied to bitwise-copyable type 'Point' has no effect}}{{7-15=}}
_ = consume cbcg_concrete // expected-warning {{'consume' applied to bitwise-copyable type 'ConditionallyBC<Int>' has no effect}}{{7-16=}}
_ = consume maybeFloat // expected-warning {{'consume' applied to bitwise-copyable type 'Float?' has no effect}}{{8-16=}}
}

func proofOfUseAfterConsume() -> Int {
let someInt = 10
let y = consume someInt // expected-warning {{'consume' applied to bitwise-copyable type 'Int' has no effect}}
print(y)
return someInt // undiagnosed use-after-consume
}

func moreProofs(_ share: __shared Int,
_ own: __owned Int,
_ snd: sending Int, // expected-error {{'snd' used after consume}}
_ ino: inout Int, // expected-error {{'ino' used after consume}}
_ brw: borrowing Int, // expected-error {{'brw' is borrowed and cannot be consumed}}
_ csm: consuming Int // expected-error {{'csm' consumed more than once}}
) {
_ = consume share // expected-warning {{'consume' applied to bitwise-copyable type 'Int' has no effect}}
_ = consume own // expected-warning {{'consume' applied to bitwise-copyable type 'Int' has no effect}}
let _ = (share, own)

_ = consume ino // expected-note {{consumed}}
_ = consume brw // expected-note {{consumed}}
_ = consume csm // expected-note {{consumed}}
_ = consume csm // expected-note {{consumed}}
_ = consume snd // expected-note {{consumed}}
_ = snd // expected-note {{used}}
} // expected-note {{used here}}
6 changes: 3 additions & 3 deletions test/Sema/move_expr.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ class Klass {

var global: Int = 5
func testGlobal() {
let _ = consume global
let _ = consume global // expected-warning {{'consume' applied to bitwise-copyable type 'Int' has no effect}}
}

func testLet() {
Expand All @@ -23,14 +23,14 @@ func testVar() {
func testExprFailureLet() {
let t = 5
// Next line is parsed as move(t) + t
let _ = consume t + t
let _ = consume t + t // expected-warning {{'consume' applied to bitwise-copyable type 'Int' has no effect}}
}

func testExprFailureVar() {
var t = 5
t = 5
// Next line is parsed as move(t) + t
let _ = consume t + t
let _ = consume t + t // expected-warning {{'consume' applied to bitwise-copyable type 'Int' has no effect}}
}

func letAddressOnly<T>(_ v: T) {
Expand Down