Skip to content

Commit 4adf5f0

Browse files
authored
Merge pull request #19084 from rjmccall/coroutine-indices-writeback-conflict
2 parents 7059782 + dd77a20 commit 4adf5f0

File tree

6 files changed

+85
-9
lines changed

6 files changed

+85
-9
lines changed

lib/SILGen/ArgumentSource.cpp

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -473,3 +473,38 @@ bool ArgumentSource::isObviouslyEqual(const ArgumentSource &other) const {
473473
}
474474
llvm_unreachable("bad kind");
475475
}
476+
477+
PreparedArguments PreparedArguments::copyForDiagnostics() const {
478+
if (isNull())
479+
return PreparedArguments();
480+
481+
assert(isValid());
482+
PreparedArguments result(getFormalType(), isScalar());
483+
for (auto &arg : Arguments) {
484+
result.Arguments.push_back(arg.copyForDiagnostics());
485+
}
486+
return result;
487+
}
488+
489+
ArgumentSource ArgumentSource::copyForDiagnostics() const {
490+
switch (StoredKind) {
491+
case Kind::Invalid:
492+
return ArgumentSource();
493+
case Kind::LValue:
494+
// We have no way to copy an l-value for diagnostics.
495+
return {getKnownLValueLocation(), LValue()};
496+
case Kind::RValue:
497+
return {getKnownRValueLocation(), asKnownRValue().copyForDiagnostics()};
498+
case Kind::Expr:
499+
return asKnownExpr();
500+
case Kind::Tuple: {
501+
auto &tuple = Storage.get<TupleStorage>(StoredKind);
502+
SmallVector<ArgumentSource, 4> copiedElements;
503+
for (auto &elt : tuple.Elements) {
504+
copiedElements.push_back(elt.copyForDiagnostics());
505+
}
506+
return {tuple.Loc, tuple.SubstType, copiedElements};
507+
}
508+
}
509+
llvm_unreachable("bad kind");
510+
}

lib/SILGen/ArgumentSource.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,8 @@ class ArgumentSource {
317317

318318
bool isObviouslyEqual(const ArgumentSource &other) const;
319319

320+
ArgumentSource copyForDiagnostics() const;
321+
320322
void dump() const;
321323
void dump(raw_ostream &os, unsigned indent = 0) const;
322324

@@ -426,6 +428,8 @@ class PreparedArguments {
426428
PreparedArguments copy(SILGenFunction &SGF, SILLocation loc) const;
427429

428430
bool isObviouslyEqual(const PreparedArguments &other) const;
431+
432+
PreparedArguments copyForDiagnostics() const;
429433
};
430434

431435
} // end namespace Lowering

lib/SILGen/RValue.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -813,3 +813,13 @@ SILType RValue::getLoweredImplodedTupleType(SILGenFunction &SGF) const & {
813813
return loweredType.getAddressType();
814814
return loweredType.getObjectType();
815815
}
816+
817+
RValue RValue::copyForDiagnostics() const {
818+
assert(!isInSpecialState());
819+
assert(isComplete());
820+
RValue result(type);
821+
for (auto value : values)
822+
result.values.push_back(value);
823+
result.elementsToBeAdded = 0;
824+
return result;
825+
}

lib/SILGen/RValue.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -355,6 +355,8 @@ class RValue {
355355
/// Borrow all subvalues of the rvalue.
356356
RValue borrow(SILGenFunction &SGF, SILLocation loc) const &;
357357

358+
RValue copyForDiagnostics() const;
359+
358360
static bool areObviouslySameValue(SILValue lhs, SILValue rhs);
359361
bool isObviouslyEqual(const RValue &rhs) const;
360362

lib/SILGen/SILGenLValue.cpp

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1512,14 +1512,22 @@ namespace {
15121512

15131513
class EndApplyPseudoComponent : public WritebackPseudoComponent {
15141514
CleanupHandle EndApplyHandle;
1515-
Optional<AccessedStorage> Access;
1515+
AbstractStorageDecl *Storage;
1516+
bool IsSuper;
1517+
PreparedArguments PeekedIndices;
1518+
Expr *IndexExprForDiagnostics;
15161519
public:
15171520
EndApplyPseudoComponent(const LValueTypeData &typeData,
15181521
CleanupHandle endApplyHandle,
1519-
Optional<AccessedStorage> access)
1522+
AbstractStorageDecl *storage,
1523+
bool isSuper,
1524+
PreparedArguments &&peekedIndices,
1525+
Expr *indexExprForDiagnostics)
15201526
: WritebackPseudoComponent(typeData),
15211527
EndApplyHandle(endApplyHandle),
1522-
Access(access) {}
1528+
Storage(storage), IsSuper(isSuper),
1529+
PeekedIndices(std::move(peekedIndices)),
1530+
IndexExprForDiagnostics(indexExprForDiagnostics) {}
15231531

15241532
private:
15251533
void writeback(SILGenFunction &SGF, SILLocation loc,
@@ -1539,7 +1547,9 @@ namespace {
15391547
}
15401548

15411549
Optional<AccessedStorage> getAccessedStorage() const override {
1542-
return Access;
1550+
return AccessedStorage{Storage, IsSuper,
1551+
PeekedIndices.isNull() ? nullptr : &PeekedIndices,
1552+
IndexExprForDiagnostics};
15431553
}
15441554
};
15451555

@@ -1570,12 +1580,9 @@ namespace {
15701580

15711581
ManagedValue result;
15721582

1573-
LogicalPathComponent::AccessedStorage abstractStorage = {
1574-
Storage, IsSuper, /*indices*/ nullptr, IndexExprForDiagnostics
1575-
};
1576-
15771583
auto args =
15781584
std::move(*this).prepareAccessorArgs(SGF, loc, base, Accessor);
1585+
auto peekedIndices = args.Indices.copyForDiagnostics();
15791586
SmallVector<ManagedValue, 4> yields;
15801587
auto endApplyHandle =
15811588
SGF.emitCoroutineAccessor(
@@ -1585,7 +1592,9 @@ namespace {
15851592
// Push a writeback that ends the access.
15861593
std::unique_ptr<LogicalPathComponent>
15871594
component(new EndApplyPseudoComponent(getTypeData(), endApplyHandle,
1588-
abstractStorage));
1595+
Storage, IsSuper,
1596+
std::move(peekedIndices),
1597+
IndexExprForDiagnostics));
15891598
pushWriteback(SGF, loc, std::move(component), ManagedValue(),
15901599
MaterializedLValue());
15911600

test/SILGen/writeback_conflict_diagnostics.swift

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,3 +111,19 @@ func testMultiArrayWithoutAddressors(
111111
swap(&global_array_without_addressors[0][j], &array[j][i]) // ok
112112
}
113113

114+
// rdar://43802132
115+
struct ArrayWithReadModify<T> {
116+
init(value: T) { self.property = value }
117+
var property: T
118+
subscript(i: Int) -> T {
119+
_read { yield property }
120+
_modify { yield &property }
121+
}
122+
}
123+
124+
func testArrayWithReadModify<T>(array: ArrayWithReadModify<T>) {
125+
var copy = array
126+
swap(&copy[0], &copy[1])
127+
swap(&copy[0], // expected-note {{concurrent writeback occurred here}}
128+
&copy[0]) // expected-error {{inout writeback through subscript occurs in multiple arguments to call}}
129+
}

0 commit comments

Comments
 (0)