Skip to content

Commit eb19ee7

Browse files
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 4b8621e commit eb19ee7

File tree

3 files changed

+43
-8
lines changed

3 files changed

+43
-8
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()))

0 commit comments

Comments
 (0)