Skip to content

Commit 5714a59

Browse files
authored
Merge pull request swiftlang#35165 from gottesmm/pr-ad9a97a586d0760867436cf845d17da4412bc75b
[sil] Eliminate a confusing optional method result by changing type dependent operands to have OwnershipKind::None instead of returning Optional::None from Operand::getOperandOwnership().
2 parents 6ca6a72 + 368f8ac commit 5714a59

File tree

7 files changed

+27
-45
lines changed

7 files changed

+27
-45
lines changed

include/swift/SIL/OwnershipUtils.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ class ForwardingOperand {
8686
OwnershipConstraint getOwnershipConstraint() const {
8787
// We use a force unwrap since a ForwardingOperand should always have an
8888
// ownership constraint.
89-
return *use->getOwnershipConstraint();
89+
return use->getOwnershipConstraint();
9090
}
9191
ValueOwnershipKind getOwnershipKind() const;
9292
void setOwnershipKind(ValueOwnershipKind newKind) const;

include/swift/SIL/SILValue.h

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -855,21 +855,15 @@ class Operand {
855855
/// Return which operand this is in the operand list of the using instruction.
856856
unsigned getOperandNumber() const;
857857

858-
/// Return the use ownership of this operand. Returns none if the operand is a
859-
/// type dependent operand.
858+
/// Return the use ownership of this operand.
860859
///
861860
/// NOTE: This is implemented in OperandOwnership.cpp.
862-
Optional<OperandOwnership> getOperandOwnership() const;
861+
OperandOwnership getOperandOwnership() const;
863862

864863
/// Return the ownership constraint that restricts what types of values this
865-
/// Operand can contain. Returns none if the operand is a type dependent
866-
/// operand.
867-
Optional<OwnershipConstraint> getOwnershipConstraint() const {
868-
auto operandOwnership = getOperandOwnership();
869-
if (!operandOwnership) {
870-
return None;
871-
}
872-
return operandOwnership->getOwnershipConstraint();
864+
/// Operand can contain.
865+
OwnershipConstraint getOwnershipConstraint() const {
866+
return getOperandOwnership().getOwnershipConstraint();
873867
}
874868

875869
/// Returns true if changing the operand to use a value with the given

lib/SIL/IR/OperandOwnership.cpp

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -775,9 +775,15 @@ OperandOwnership OperandOwnershipClassifier::visitBuiltinInst(BuiltinInst *bi) {
775775
// Top Level Entrypoint
776776
//===----------------------------------------------------------------------===//
777777

778-
Optional<OperandOwnership> Operand::getOperandOwnership() const {
778+
OperandOwnership Operand::getOperandOwnership() const {
779+
// We consider type dependent uses to be instantaneous uses.
780+
//
781+
// NOTE: We could instead try to exclude type dependent uses from our system,
782+
// but that adds a bunch of Optionals and unnecessary types. This doesn't hurt
783+
// anything and allows us to eliminate Optionals and thus confusion in between
784+
// Optional::None and OwnershipKind::None.
779785
if (isTypeDependent())
780-
return None;
786+
return OperandOwnership::InstantaneousUse;
781787

782788
// If we do not have ownership enabled, just return any. This ensures that we
783789
// do not have any consuming uses and everything from an ownership perspective

lib/SIL/IR/SILValue.cpp

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -343,14 +343,11 @@ static bool canAcceptUnownedValue(OperandOwnership operandOwnership) {
343343

344344
bool Operand::canAcceptKind(ValueOwnershipKind kind) const {
345345
auto operandOwnership = getOperandOwnership();
346-
if (!operandOwnership) {
347-
return false;
348-
}
349-
auto constraint = operandOwnership->getOwnershipConstraint();
346+
auto constraint = operandOwnership.getOwnershipConstraint();
350347
if (constraint.satisfiesConstraint(kind)) {
351348
// Constraints aren't precise enough to enforce Unowned value uses.
352349
if (kind == OwnershipKind::Unowned) {
353-
return canAcceptUnownedValue(operandOwnership.getValue());
350+
return canAcceptUnownedValue(operandOwnership);
354351
}
355352
return true;
356353
}
@@ -365,13 +362,8 @@ bool Operand::satisfiesConstraints() const {
365362
bool Operand::isLifetimeEnding() const {
366363
auto constraint = getOwnershipConstraint();
367364

368-
// If we got back Optional::None, then our operand is for a type dependent
369-
// operand. So return false.
370-
if (!constraint)
371-
return false;
372-
373365
// If our use lifetime constraint is NonLifetimeEnding, just return false.
374-
if (!constraint->isLifetimeEnding())
366+
if (!constraint.isLifetimeEnding())
375367
return false;
376368

377369
// Otherwise, we may have a lifetime ending use. We consider two cases here:

lib/SIL/Utils/OwnershipUtils.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -877,7 +877,7 @@ Optional<ForwardingOperand> ForwardingOperand::get(Operand *use) {
877877
return None;
878878
}
879879
#ifndef NDEBUG
880-
switch (use->getOperandOwnership().getValue()) {
880+
switch (use->getOperandOwnership()) {
881881
case OperandOwnership::None:
882882
case OperandOwnership::ForwardingUnowned:
883883
case OperandOwnership::ForwardingConsume:

lib/SIL/Verifier/SILOwnershipVerifier.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ bool SILValueOwnershipChecker::isCompatibleDefUse(
188188
return true;
189189
}
190190

191-
auto constraint = *op->getOwnershipConstraint();
191+
auto constraint = op->getOwnershipConstraint();
192192
errorBuilder.handleMalformedSIL([&]() {
193193
llvm::errs() << "Have operand with incompatible ownership?!\n"
194194
<< "Value: " << op->get() << "User: " << *user
@@ -326,8 +326,7 @@ bool SILValueOwnershipChecker::gatherUsers(
326326
// Example: A guaranteed parameter of a co-routine.
327327

328328
// Now check if we have a non guaranteed forwarding inst...
329-
if (op->getOperandOwnership().getValue()
330-
!= OperandOwnership::ForwardingBorrow) {
329+
if (op->getOperandOwnership() != OperandOwnership::ForwardingBorrow) {
331330
// First check if we are visiting an operand that is a consuming use...
332331
if (op->isLifetimeEnding()) {
333332
// If its underlying value is our original value, then this is a true
@@ -744,7 +743,7 @@ void SILInstruction::verifyOperandOwnership() const {
744743
if (op.satisfiesConstraints())
745744
continue;
746745

747-
auto constraint = *op.getOwnershipConstraint();
746+
auto constraint = op.getOwnershipConstraint();
748747
SILValue opValue = op.get();
749748
auto valueOwnershipKind = opValue.getOwnershipKind();
750749
errorBuilder->handleMalformedSIL([&] {

lib/SIL/Verifier/SILVerifier.cpp

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1127,21 +1127,12 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
11271127
// type dependent operand then we should have a constraint of
11281128
// OwnershipKind::Any, UseLifetimeConstraint::NonLifetimeEnding.
11291129
if (!I->getFunction()->hasOwnership()) {
1130-
if (operand.isTypeDependent()) {
1131-
require(
1132-
!operand.getOwnershipConstraint(),
1133-
"Non Optional::None constraint for a type dependent operand?!");
1134-
} else {
1135-
auto constraint = operand.getOwnershipConstraint();
1136-
require(constraint.hasValue(),
1137-
"All non-type dependent operands must have a "
1138-
"non-Optional::None constraint?!");
1139-
require(constraint->getPreferredKind() == OwnershipKind::Any &&
1140-
constraint->getLifetimeConstraint() ==
1141-
UseLifetimeConstraint::NonLifetimeEnding,
1142-
"In non-ossa all non-type dependent operands must have a "
1143-
"constraint of Any, NonLifetimeEnding");
1144-
}
1130+
auto constraint = operand.getOwnershipConstraint();
1131+
require(constraint.getPreferredKind() == OwnershipKind::Any &&
1132+
constraint.getLifetimeConstraint() ==
1133+
UseLifetimeConstraint::NonLifetimeEnding,
1134+
"In non-ossa all non-type dependent operands must have a "
1135+
"constraint of Any, NonLifetimeEnding");
11451136
}
11461137
}
11471138

0 commit comments

Comments
 (0)