Skip to content

Commit ef4523c

Browse files
gottesmmgottesmm
committed
[move-only] If we diagnose an invalid escaping capture, turn off noncopyable checking in the closure.
The reason why I am doing this is that currently SILGen knows when emitting said closure that we are going to emit an error (that it does not have enough information to emit itself since it doesn't know the caller), so doesn't emit move checking markers. The result is that the move checker will not eliminate any copies in the closure and thus will emit a "copy of noncopyable type found" error and tell the user to file a bug. This just suppresses that. The tests for this go with the next commit.
1 parent 39a2863 commit ef4523c

File tree

6 files changed

+45
-15
lines changed

6 files changed

+45
-15
lines changed

‎include/swift/AST/SemanticAttrs.def

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,8 +127,21 @@ SEMANTICS_ATTR(LIFETIMEMANAGEMENT_COPY, "lifetimemanagement.copy")
127127

128128
SEMANTICS_ATTR(NO_PERFORMANCE_ANALYSIS, "no_performance_analysis")
129129

130-
// A flag used to turn off moveonly diagnostics on functions that allocbox to
131-
// stack specialized.
130+
// A flag used to turn off moveonly diagnostics on a function due to an earlier
131+
// pass that ran.
132+
//
133+
// DISCUSSION: This is used in a few situations:
134+
//
135+
// 1. When allocbox to stack specializes a function, we do not want to emit move
136+
// errors twice, once on the specialized and once on the original
137+
// function. Instead, we put this on the original function.
138+
//
139+
// 2. If we emit a diagnose invalid escaping captures error due to an inout
140+
// being escaped into an escaping closure, we do not want to emit move errors in
141+
// the closure. This is because SILGen today assumes that we will error in such
142+
// cases and thus does not emit markers in said function for the inout. This
143+
// then causes us to emit spurious "found a copy of a noncopyable value" errors
144+
// that may cause the user to think there is a bug in the compiler.
132145
SEMANTICS_ATTR(NO_MOVEONLY_DIAGNOSTICS, "sil.optimizer.moveonly.diagnostic.ignore")
133146

134147
#undef SEMANTICS_ATTR

‎lib/SILOptimizer/Mandatory/DiagnoseInvalidEscapingCaptures.cpp

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include "swift/AST/Expr.h"
2121
#include "swift/AST/Types.h"
2222
#include "swift/SIL/ApplySite.h"
23+
#include "swift/AST/SemanticAttrs.h"
2324
#include "swift/SIL/InstructionUtils.h"
2425
#include "swift/SIL/SILArgument.h"
2526
#include "swift/SIL/SILBasicBlock.h"
@@ -396,18 +397,24 @@ static void checkPartialApply(ASTContext &Context, DeclContext *DC,
396397
}
397398
}
398399
}
400+
401+
bool emittedError = false;
402+
399403
// First, diagnose the inout captures, if any.
400404
for (auto inoutCapture : inoutCaptures) {
401405
Optional<Identifier> paramName = None;
402406
if (isUseOfSelfInInitializer(inoutCapture)) {
407+
emittedError = true;
403408
diagnose(Context, PAI->getLoc(), diag::escaping_mutable_self_capture,
404409
functionKind);
405410
} else {
406411
auto *param = getParamDeclFromOperand(inoutCapture->get());
407-
if (param->isSelfParameter())
412+
if (param->isSelfParameter()) {
413+
emittedError = true;
408414
diagnose(Context, PAI->getLoc(), diag::escaping_mutable_self_capture,
409415
functionKind);
410-
else {
416+
} else {
417+
emittedError = true;
411418
paramName = param->getName();
412419
diagnose(Context, PAI->getLoc(), diag::escaping_inout_capture,
413420
functionKind, param->getName());
@@ -416,31 +423,46 @@ static void checkPartialApply(ASTContext &Context, DeclContext *DC,
416423
}
417424
}
418425
if (functionKind != EscapingAutoClosure) {
426+
emittedError = true;
419427
diagnoseCaptureLoc(Context, DC, PAI, inoutCapture);
420428
continue;
421429
}
422430
// For an autoclosure capture, present a way to fix the problem.
423-
if (paramName)
431+
if (paramName) {
432+
emittedError = true;
424433
diagnose(Context, PAI->getLoc(), diag::copy_inout_captured_by_autoclosure,
425434
paramName.value());
426-
else
435+
} else {
436+
emittedError = true;
427437
diagnose(Context, PAI->getLoc(), diag::copy_self_captured_by_autoclosure);
438+
}
428439
}
429440

430441
// Finally, diagnose captures of values with noescape type.
431442
for (auto noEscapeCapture : noEscapeCaptures) {
432443
if (auto *param = getParamDeclFromOperand(noEscapeCapture->get())) {
444+
emittedError = true;
433445
diagnose(Context, PAI->getLoc(), diag::escaping_noescape_param_capture,
434446
functionKind, param->getName());
435447
diagnose(Context, param->getLoc(), diag::noescape_param_defined_here,
436448
param->getName());
437449
} else {
450+
emittedError = true;
438451
diagnose(Context, PAI->getLoc(), diag::escaping_noescape_var_capture,
439452
functionKind);
440453
}
441454

442455
diagnoseCaptureLoc(Context, DC, PAI, noEscapeCapture);
443456
}
457+
458+
// If we emitted an error, mark the closure function as not being suitable for
459+
// noncopyable diagnostics. The user can fix the issue and then recompile.
460+
if (emittedError) {
461+
if (auto *f = apply.getCalleeFunction()) {
462+
auto s = semantics::NO_MOVEONLY_DIAGNOSTICS;
463+
f->addSemanticsAttr(s);
464+
}
465+
}
444466
}
445467

446468
// Enforce exclusivity restrictions on recursive uses of non-escaping closures.
@@ -539,7 +561,7 @@ static void checkEscapingCaptures(SILFunction *F) {
539561
if (F->empty())
540562
return;
541563

542-
auto &Context =F->getASTContext();
564+
auto &Context = F->getASTContext();
543565
auto *DC = F->getDeclContext();
544566

545567
for (auto &BB : *F) {

‎lib/SILOptimizer/Mandatory/MoveOnlyChecker.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ class MoveOnlyCheckerPass : public SILFunctionTransform {
157157
assert(fn->getModule().getStage() == SILStage::Raw &&
158158
"Should only run on Raw SIL");
159159

160-
// If allocbox to stack told use to not emit diagnostics for this function,
160+
// If an earlier pass told use to not emit diagnostics for this function,
161161
// clean up any copies, invalidate the analysis, and return early.
162162
if (getFunction()->hasSemanticsAttr(semantics::NO_MOVEONLY_DIAGNOSTICS)) {
163163
if (cleanupNonCopyableCopiesAfterEmittingDiagnostic(getFunction()))

‎test/SILOptimizer/moveonly_addresschecker_diagnostics.swift

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1925,7 +1925,6 @@ public func closureCaptureClassArgUseAfterConsume(_ x2: inout Klass) {
19251925
let f = { // expected-note {{consuming use here}}
19261926
// expected-error @-1 {{escaping closure captures 'inout' parameter 'x2'}}
19271927
borrowVal(x2) // expected-note {{captured here}}
1928-
// expected-error @-1 {{copy of noncopyable typed value. This is a compiler bug. Please file a bug with a small example of the bug}}
19291928
consumeVal(x2) // expected-note {{captured here}}
19301929
consumeVal(x2) // expected-note {{captured here}}
19311930
}
@@ -2107,9 +2106,8 @@ public func closureAndClosureCaptureClassArgUseAfterConsume(_ x2: inout Klass) {
21072106
let g = { // expected-error {{escaping closure captures 'inout' parameter 'x2'}}
21082107
// expected-note @-1 {{captured indirectly by this call}}
21092108
borrowVal(x2)
2110-
// expected-error @-1 {{copy of noncopyable typed value. This is a compiler bug. Please file a bug with a small example of the bug}}
2109+
// expected-note @-1 {{captured here}}
21112110
// expected-note @-2 {{captured here}}
2112-
// expected-note @-3 {{captured here}}
21132111
consumeVal(x2)
21142112
// expected-note @-1 {{captured here}}
21152113
// expected-note @-2 {{captured here}}

‎test/SILOptimizer/moveonly_objectchecker_diagnostics.swift

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2157,7 +2157,6 @@ public func closureCaptureClassUseAfterConsume2(_ x2: inout Klass) {
21572157
let f = { // expected-error {{escaping closure captures 'inout' parameter 'x2'}}
21582158
// expected-note @-1 {{consuming use here}}
21592159
borrowVal(x2) // expected-note {{captured here}}
2160-
// expected-error @-1 {{copy of noncopyable typed value. This is a compiler bug. Please file a bug with a small example of the bug}}
21612160
consumeVal(x2) // expected-note {{captured here}}
21622161
consumeVal(x2) // expected-note {{captured here}}
21632162
}

‎test/SILOptimizer/moveonly_trivial_addresschecker_diagnostics.swift

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1237,7 +1237,6 @@ public func closureCaptureClassArgUseAfterConsume(_ x2: inout NonTrivialStruct)
12371237
let f = { // expected-note {{consuming use here}}
12381238
// expected-error @-1 {{escaping closure captures 'inout' parameter 'x2'}}
12391239
borrowVal(x2) // expected-note {{captured here}}
1240-
// expected-error @-1 {{copy of noncopyable typed value. This is a compiler bug. Please file a bug with a small example of the bug}}
12411240
consumeVal(x2) // expected-note {{captured here}}
12421241
consumeVal(x2) // expected-note {{captured here}}
12431242
}
@@ -1419,9 +1418,8 @@ public func closureAndClosureCaptureClassArgUseAfterConsume(_ x2: inout NonTrivi
14191418
let g = { // expected-error {{escaping closure captures 'inout' parameter 'x2'}}
14201419
// expected-note @-1 {{captured indirectly by this call}}
14211420
borrowVal(x2)
1422-
// expected-error @-1 {{copy of noncopyable typed value. This is a compiler bug. Please file a bug with a small example of the bug}}
1421+
// expected-note @-1 {{captured here}}
14231422
// expected-note @-2 {{captured here}}
1424-
// expected-note @-3 {{captured here}}
14251423
consumeVal(x2)
14261424
// expected-note @-1 {{captured here}}
14271425
// expected-note @-2 {{captured here}}

0 commit comments

Comments
 (0)