Skip to content

Conversation

@eeckstein
Copy link
Contributor

Bail if there is any kind of user which is not handled in this transformation.

Bail if there is any kind of user which is not handled in this transformation.
@eeckstein eeckstein requested a review from gottesmm April 23, 2020 13:56
@eeckstein
Copy link
Contributor Author

@swift-ci smoke test

@eeckstein
Copy link
Contributor Author

@swift-ci smoke test linux

1 similar comment
@eeckstein
Copy link
Contributor Author

@swift-ci smoke test linux

case SILInstructionKind::MarkDependenceInst:
continue;
default:
return {std::next(si->getIterator()), false};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to think about this a little more, but can you change this to crash in asserts builds? I think that might be a nice way to square the circle of not crashing in non-asserts builds but also getting noise when we do not update all of the relevant code. Creating implicit dependencies in the code like this, makes me nervous.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, that doesn't make sense.
Why should there be an assert for a case which totally legal to happen and we know how to reproduce it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah ok. I understand what you are doing now. LGTM.

@eeckstein eeckstein merged commit b68c729 into swiftlang:master Apr 24, 2020
@eeckstein eeckstein deleted the fix-temprvalueopt branch April 24, 2020 06:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants