Skip to content

Commit c998bbc

Browse files
authored
Merge pull request #75351 from gottesmm/rdar127318392
[region-isolation] Emit an error when we assign a non-disconnected value into a sending result.
2 parents 3b6c21d + 1086264 commit c998bbc

16 files changed

+796
-183
lines changed

include/swift/AST/DiagnosticsSIL.def

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -972,6 +972,9 @@ ERROR(regionbasedisolation_arg_passed_to_strongly_transferred_param, none,
972972
ERROR(regionbasedisolation_named_transfer_yields_race, none,
973973
"sending %0 risks causing data races",
974974
(Identifier))
975+
NOTE(regionbasedisolation_type_is_non_sendable, none,
976+
"%0 is a non-Sendable type",
977+
(Type))
975978

976979
ERROR(regionbasedisolation_inout_sending_cannot_be_actor_isolated, none,
977980
"'inout sending' parameter %0 cannot be %1at end of function",
@@ -980,6 +983,20 @@ NOTE(regionbasedisolation_inout_sending_cannot_be_actor_isolated_note, none,
980983
"%1%0 risks causing races in between %1uses and caller uses since caller assumes value is not actor isolated",
981984
(Identifier, StringRef))
982985

986+
ERROR(regionbasedisolation_out_sending_cannot_be_actor_isolated_type, none,
987+
"returning a %1 %0 value as a 'sending' result risks causing data races",
988+
(Type, StringRef))
989+
NOTE(regionbasedisolation_out_sending_cannot_be_actor_isolated_note_type, none,
990+
"returning a %1 %0 value risks causing races since the caller assumes the value can be safely sent to other isolation domains",
991+
(Type, StringRef))
992+
993+
ERROR(regionbasedisolation_out_sending_cannot_be_actor_isolated_named, none,
994+
"returning %1 %0 as a 'sending' result risks causing data races",
995+
(Identifier, StringRef))
996+
NOTE(regionbasedisolation_out_sending_cannot_be_actor_isolated_note_named, none,
997+
"returning %1 %0 risks causing data races since the caller assumes that %0 can be safely sent to other isolation domains",
998+
(Identifier, StringRef))
999+
9831000
NOTE(regionbasedisolation_named_info_transfer_yields_race, none,
9841001
"sending %1%0 to %2 callee risks causing data races between %2 and local %3 uses",
9851002
(Identifier, StringRef, ActorIsolation, ActorIsolation))

include/swift/SIL/InstWrappers.h

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,13 @@ class SelectEnumOperation {
221221
return sei->getCase(i);
222222
return value.get<SelectEnumAddrInst *>()->getCase(i);
223223
}
224+
225+
std::pair<EnumElementDecl *, Operand *> getCaseOperand(unsigned i) const {
226+
if (auto *sei = value.dyn_cast<SelectEnumInst *>())
227+
return sei->getCaseOperand(i);
228+
return value.get<SelectEnumAddrInst *>()->getCaseOperand(i);
229+
}
230+
224231
/// Return the value that will be used as the result for the specified enum
225232
/// case.
226233
SILValue getCaseResult(EnumElementDecl *D) {
@@ -229,6 +236,12 @@ class SelectEnumOperation {
229236
return value.get<SelectEnumAddrInst *>()->getCaseResult(D);
230237
}
231238

239+
Operand *getCaseResultOperand(EnumElementDecl *D) {
240+
if (auto *sei = value.dyn_cast<SelectEnumInst *>())
241+
return sei->getCaseResultOperand(D);
242+
return value.get<SelectEnumAddrInst *>()->getCaseResultOperand(D);
243+
}
244+
232245
/// If the default refers to exactly one case decl, return it.
233246
NullablePtr<EnumElementDecl> getUniqueCaseForDefault();
234247

@@ -243,6 +256,12 @@ class SelectEnumOperation {
243256
return sei->getDefaultResult();
244257
return value.get<SelectEnumAddrInst *>()->getDefaultResult();
245258
}
259+
260+
Operand *getDefaultResultOperand() const {
261+
if (auto *sei = value.dyn_cast<SelectEnumInst *>())
262+
return sei->getDefaultResultOperand();
263+
return value.get<SelectEnumAddrInst *>()->getDefaultResultOperand();
264+
}
246265
};
247266

248267
class ForwardingOperation {

include/swift/SIL/SILInstruction.h

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7067,6 +7067,12 @@ class SelectEnumInstBase : public BaseTy {
70677067
getAllOperands()[i+1].get());
70687068
}
70697069

7070+
std::pair<EnumElementDecl *, Operand *> getCaseOperand(unsigned i) const {
7071+
auto *self = const_cast<SelectEnumInstBase *>(this);
7072+
return std::make_pair(getEnumElementDeclStorage()[i],
7073+
&self->getAllOperands()[i + 1]);
7074+
}
7075+
70707076
/// Return the value that will be used as the result for the specified enum
70717077
/// case.
70727078
SILValue getCaseResult(EnumElementDecl *D) {
@@ -7079,6 +7085,18 @@ class SelectEnumInstBase : public BaseTy {
70797085
return getDefaultResult();
70807086
}
70817087

7088+
Operand *getCaseResultOperand(EnumElementDecl *D) {
7089+
for (unsigned i = 0, e = getNumCases(); i != e; ++i) {
7090+
auto Entry = getCaseOperand(i);
7091+
if (Entry.first == D)
7092+
return Entry.second;
7093+
}
7094+
7095+
// select_enum is required to be fully covered, so return the default if we
7096+
// didn't find anything.
7097+
return getDefaultResultOperand();
7098+
}
7099+
70827100
bool hasDefault() const {
70837101
return sharedUInt8().SelectEnumInstBase.hasDefault;
70847102
}
@@ -7088,6 +7106,12 @@ class SelectEnumInstBase : public BaseTy {
70887106
return getAllOperands().back().get();
70897107
}
70907108

7109+
Operand *getDefaultResultOperand() const {
7110+
assert(hasDefault() && "doesn't have a default");
7111+
auto *self = const_cast<SelectEnumInstBase *>(this);
7112+
return &self->getAllOperands().back();
7113+
}
7114+
70917115
unsigned getNumCases() const {
70927116
return getAllOperands().size() - 1 - hasDefault();
70937117
}

include/swift/SILOptimizer/Analysis/RegionAnalysis.h

Lines changed: 4 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,6 @@ class BlockPartitionState {
114114

115115
class TrackableValue;
116116
class TrackableValueState;
117-
class RepresentativeValue;
118117

119118
enum class TrackableValueFlag {
120119
/// Base value that says a value is uniquely represented and is
@@ -208,53 +207,6 @@ class regionanalysisimpl::TrackableValueState {
208207
}
209208
};
210209

211-
/// The representative value of the equivalence class that makes up a tracked
212-
/// value.
213-
///
214-
/// We use a wrapper struct here so that we can inject "fake" actor isolated
215-
/// values into the regions of values that become merged into an actor by
216-
/// calling a function without a non-sendable result.
217-
class regionanalysisimpl::RepresentativeValue {
218-
friend llvm::DenseMapInfo<RepresentativeValue>;
219-
220-
using InnerType = PointerUnion<SILValue, SILInstruction *>;
221-
222-
/// If this is set to a SILValue then it is the actual represented value. If
223-
/// it is set to a SILInstruction, then this is a "fake" representative value
224-
/// used to inject actor isolatedness. The instruction stored is the
225-
/// instruction that introduced the actor isolated-ness.
226-
InnerType value;
227-
228-
public:
229-
RepresentativeValue() : value() {}
230-
RepresentativeValue(SILValue value) : value(value) {}
231-
RepresentativeValue(SILInstruction *actorRegionInst)
232-
: value(actorRegionInst) {}
233-
234-
operator bool() const { return bool(value); }
235-
236-
void print(llvm::raw_ostream &os) const {
237-
if (auto *inst = value.dyn_cast<SILInstruction *>()) {
238-
os << "ActorRegionIntroducingInst: " << *inst;
239-
return;
240-
}
241-
242-
os << *value.get<SILValue>();
243-
}
244-
245-
SILValue getValue() const { return value.get<SILValue>(); }
246-
SILValue maybeGetValue() const { return value.dyn_cast<SILValue>(); }
247-
bool hasRegionIntroducingInst() const { return value.is<SILInstruction *>(); }
248-
SILInstruction *getActorRegionIntroducingInst() const {
249-
return value.get<SILInstruction *>();
250-
}
251-
252-
SWIFT_DEBUG_DUMP { print(llvm::dbgs()); }
253-
254-
private:
255-
RepresentativeValue(InnerType value) : value(value) {}
256-
};
257-
258210
/// A tuple consisting of a base value and its value state.
259211
///
260212
/// DISCUSSION: We are computing regions among equivalence classes of values
@@ -336,7 +288,6 @@ class RegionAnalysisValueMap {
336288
using Region = PartitionPrimitives::Region;
337289
using TrackableValue = regionanalysisimpl::TrackableValue;
338290
using TrackableValueState = regionanalysisimpl::TrackableValueState;
339-
using RepresentativeValue = regionanalysisimpl::RepresentativeValue;
340291

341292
private:
342293
/// A map from the representative of an equivalence class of values to their
@@ -365,6 +316,10 @@ class RegionAnalysisValueMap {
365316
/// value" returns an empty SILValue.
366317
SILValue maybeGetRepresentative(Element trackableValueID) const;
367318

319+
/// Returns the value for this instruction if it isn't a fake "represenative
320+
/// value" to inject actor isolatedness. Asserts in such a case.
321+
RepresentativeValue getRepresentativeValue(Element trackableValueID) const;
322+
368323
/// Returns the fake "representative value" for this element if it
369324
/// exists. Returns nullptr otherwise.
370325
SILInstruction *maybeGetActorIntroducingInst(Element trackableValueID) const;
@@ -576,37 +531,4 @@ class RegionAnalysis final
576531

577532
} // namespace swift
578533

579-
namespace llvm {
580-
581-
inline llvm::raw_ostream &
582-
operator<<(llvm::raw_ostream &os,
583-
const swift::regionanalysisimpl::RepresentativeValue &value) {
584-
value.print(os);
585-
return os;
586-
}
587-
588-
template <>
589-
struct DenseMapInfo<swift::regionanalysisimpl::RepresentativeValue> {
590-
using RepresentativeValue = swift::regionanalysisimpl::RepresentativeValue;
591-
using InnerType = RepresentativeValue::InnerType;
592-
using InnerDenseMapInfo = DenseMapInfo<InnerType>;
593-
594-
static RepresentativeValue getEmptyKey() {
595-
return RepresentativeValue(InnerDenseMapInfo::getEmptyKey());
596-
}
597-
static RepresentativeValue getTombstoneKey() {
598-
return RepresentativeValue(InnerDenseMapInfo::getTombstoneKey());
599-
}
600-
601-
static unsigned getHashValue(RepresentativeValue value) {
602-
return InnerDenseMapInfo::getHashValue(value.value);
603-
}
604-
605-
static bool isEqual(RepresentativeValue LHS, RepresentativeValue RHS) {
606-
return InnerDenseMapInfo::isEqual(LHS.value, RHS.value);
607-
}
608-
};
609-
610-
} // namespace llvm
611-
612534
#endif

0 commit comments

Comments
 (0)