Skip to content

[silgen] Assert that rvalues always have consistent ownership and cle… #11195

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions lib/SILGen/RValue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,7 @@ RValue::RValue(ArrayRef<ManagedValue> values, CanType type)
return;
}

verifyConsistentOwnership();
}

RValue RValue::withPreExplodedElements(ArrayRef<ManagedValue> values,
Expand All @@ -450,6 +451,7 @@ RValue::RValue(SILGenFunction &SGF, SILLocation l, CanType formalType,

ExplodeTupleValue(values, SGF, l).visit(formalType, v);
assert(values.size() == getRValueSize(type));
verifyConsistentOwnership();
}

RValue::RValue(SILGenFunction &SGF, Expr *expr, ManagedValue v)
Expand All @@ -464,6 +466,7 @@ RValue::RValue(SILGenFunction &SGF, Expr *expr, ManagedValue v)
assert(v && "creating r-value with consumed value");
ExplodeTupleValue(values, SGF, expr).visit(type, v);
assert(values.size() == getRValueSize(type));
verifyConsistentOwnership();
}

RValue::RValue(CanType type)
Expand All @@ -485,6 +488,7 @@ void RValue::addElement(RValue &&element) & {
element.makeUsed();

assert(!isComplete() || values.size() == getRValueSize(type));
verifyConsistentOwnership();
}

void RValue::addElement(SILGenFunction &SGF, ManagedValue element,
Expand All @@ -498,6 +502,7 @@ void RValue::addElement(SILGenFunction &SGF, ManagedValue element,
ExplodeTupleValue(values, SGF, l).visit(formalType, element);

assert(!isComplete() || values.size() == getRValueSize(type));
verifyConsistentOwnership();
}

SILValue RValue::forwardAsSingleValue(SILGenFunction &SGF, SILLocation l) && {
Expand Down Expand Up @@ -753,3 +758,29 @@ void RValue::dump(raw_ostream &OS, unsigned indent) const {
value.dump(OS, indent + 2);
}
}

void RValue::verifyConsistentOwnership() const {
// This is a no-op in non-assert builds.
#ifndef NDEBUG
auto result = Optional<ValueOwnershipKind>(ValueOwnershipKind::Any);
Optional<bool> sameHaveCleanups;
for (ManagedValue v : values) {
ValueOwnershipKind kind = v.getOwnershipKind();
if (kind == ValueOwnershipKind::Trivial)
continue;

// Merge together whether or not the RValue has cleanups.
if (!sameHaveCleanups.hasValue()) {
sameHaveCleanups = v.hasCleanup();
} else {
assert(*sameHaveCleanups == v.hasCleanup());
}

// This variable is here so that if the assert below fires, the current
// reduction value is still available.
auto newResult = result.getValue().merge(kind);
assert(newResult.hasValue());
result = newResult;
}
#endif
}
11 changes: 10 additions & 1 deletion lib/SILGen/RValue.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,9 @@ class RValue {
RValue(std::vector<ManagedValue> &&values, CanType type,
unsigned elementsToBeAdded)
: values(std::move(values)), type(type),
elementsToBeAdded(elementsToBeAdded) {}
elementsToBeAdded(elementsToBeAdded) {
verifyConsistentOwnership();
}

/// Construct an RValue from a pre-exploded set of ManagedValues.
///
Expand Down Expand Up @@ -278,6 +280,13 @@ class RValue {

void dump() const;
void dump(raw_ostream &OS, unsigned indent = 0) const;

private:
/// Assert that all non-trivial ManagedValues in this RValue either all have a
/// cleanup or all do not have a cleanup.
///
/// *NOTE* This is a no-op in non-assert builds.
void verifyConsistentOwnership() const;
};

} // end namespace Lowering
Expand Down