Skip to content

Commit 7059fea

Browse files
committed
[move-function] Make sure that we error if the user re-initializes a var via its fields instead of all at once.
In the future we may be able to handle this case, but for now lets just error so that users are not confused.
1 parent b0bebcd commit 7059fea

File tree

3 files changed

+79
-0
lines changed

3 files changed

+79
-0
lines changed

lib/SILOptimizer/Mandatory/MoveKillsCopyableAddressesChecker.cpp

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -917,12 +917,26 @@ bool GatherClosureUseVisitor::visitUse(Operand *op, AccessUseType useTy) {
917917
return true;
918918

919919
if (memInstMustInitialize(op)) {
920+
if (stripAccessMarkers(op->get()) != useState.address) {
921+
LLVM_DEBUG(llvm::dbgs()
922+
<< "!!! Error! Found init use not on base address: "
923+
<< *op->getUser());
924+
return false;
925+
}
926+
920927
LLVM_DEBUG(llvm::dbgs() << "ClosureUse: Found init: " << *op->getUser());
921928
useState.inits.insert(op->getUser());
922929
return true;
923930
}
924931

925932
if (memInstMustReinitialize(op)) {
933+
if (stripAccessMarkers(op->get()) != useState.address) {
934+
LLVM_DEBUG(llvm::dbgs()
935+
<< "!!! Error! Found reinit use not on base address: "
936+
<< *op->getUser());
937+
return false;
938+
}
939+
926940
LLVM_DEBUG(llvm::dbgs() << "ClosureUse: Found reinit: " << *op->getUser());
927941
useState.insertReinit(op->getUser());
928942
return true;
@@ -1279,12 +1293,26 @@ bool GatherLexicalLifetimeUseVisitor::visitUse(Operand *op,
12791293
}
12801294

12811295
if (memInstMustInitialize(op)) {
1296+
if (stripAccessMarkers(op->get()) != useState.address) {
1297+
LLVM_DEBUG(llvm::dbgs()
1298+
<< "!!! Error! Found init use not on base address: "
1299+
<< *op->getUser());
1300+
return false;
1301+
}
1302+
12821303
LLVM_DEBUG(llvm::dbgs() << "Found init: " << *op->getUser());
12831304
useState.inits.insert(op->getUser());
12841305
return true;
12851306
}
12861307

12871308
if (memInstMustReinitialize(op)) {
1309+
if (stripAccessMarkers(op->get()) != useState.address) {
1310+
LLVM_DEBUG(llvm::dbgs()
1311+
<< "!!! Error! Found reinit use not on base address: "
1312+
<< *op->getUser());
1313+
return false;
1314+
}
1315+
12881316
LLVM_DEBUG(llvm::dbgs() << "Found reinit: " << *op->getUser());
12891317
useState.insertReinit(op->getUser());
12901318
return true;
@@ -1308,6 +1336,14 @@ bool GatherLexicalLifetimeUseVisitor::visitUse(Operand *op,
13081336
// are going to try and extend move checking into the partial apply using
13091337
// cloning to eliminate destroys or reinits.
13101338
if (auto fas = FullApplySite::isa(op->getUser())) {
1339+
if (stripAccessMarkers(op->get()) != useState.address) {
1340+
LLVM_DEBUG(
1341+
llvm::dbgs()
1342+
<< "!!! Error! Found consuming closure use not on base address: "
1343+
<< *op->getUser());
1344+
return false;
1345+
}
1346+
13111347
if (fas.getArgumentOperandConvention(*op) ==
13121348
SILArgumentConvention::Indirect_InoutAliasable) {
13131349
// If we don't find the function, we can't handle this, so bail.

test/SILOptimizer/move_function_kills_copyable_addressonly_vars.swift

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -668,3 +668,25 @@ func multipleCapture2<T : P>(_ k: T) -> () {
668668
print("foo bar")
669669
}
670670

671+
//////////////////////
672+
// Reinit in pieces //
673+
//////////////////////
674+
675+
// These tests exercise the diagnostic to see how we error if we re-initialize a
676+
// var in pieces. Eventually we should teach either this diagnostic pass how to
677+
// handle this or teach DI how to combine the initializations into one large
678+
// reinit.
679+
struct ProtPair<T : P> {
680+
var lhs: T
681+
var rhs: T
682+
}
683+
684+
func reinitInPieces1<T : P>(_ k: ProtPair<T>) {
685+
let selfType = type(of: k.lhs)
686+
var k2 = k
687+
k2 = k
688+
689+
let _ = _move(k2) // expected-error {{_move applied to value that the compiler does not support checking}}
690+
k2.lhs = selfType.getP()
691+
k2.rhs = selfType.getP()
692+
}

test/SILOptimizer/move_function_kills_copyable_loadable_vars.swift

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -710,3 +710,24 @@ func multipleCapture2(_ k: Klass) -> () {
710710
print("foo bar")
711711
}
712712

713+
//////////////////////
714+
// Reinit in pieces //
715+
//////////////////////
716+
717+
// These tests exercise the diagnostic to see how we error if we re-initialize a
718+
// var in pieces. Eventually we should teach either this diagnostic pass how to
719+
// handle this or teach DI how to combine the initializations into one large
720+
// reinit.
721+
struct KlassPair {
722+
var lhs: Klass
723+
var rhs: Klass
724+
}
725+
726+
func reinitInPieces1(_ k: KlassPair) {
727+
var k2 = k
728+
k2 = k
729+
730+
let _ = _move(k2) // expected-error {{_move applied to value that the compiler does not support checking}}
731+
k2.lhs = Klass()
732+
k2.rhs = Klass()
733+
}

0 commit comments

Comments
 (0)