Skip to content

[definite-init] Treat destroys of mark_uninitialized [var] container to be a load + destroy. #16844

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 1 commit into from
May 30, 2018

Conversation

gottesmm
Copy link
Contributor

This ensures that DI creates dealloc_box in cases where the box is uninitialized
conditionally, i.e.:

func foo() {
var k: Klass

if condition() { return } // <== We need to use a dealloc_box on k.

k = Klass()
}

rdar://40332620


I also deleted some dead code, so please look at just the last commit. I am going to squash at the end.

SILBasicBlock::iterator InsertPt) {
assert(ABI == Release->getOperand(0));
SILBuilderWithScope B(Release);
B.setInsertionPoint(InsertPt);
Copy link
Member

Choose a reason for hiding this comment

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

Here you're updating the insertion point but not the debug scope. Is this intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just followed the model in other parts of the file. If this is wrong then the other places are wrong as well.

@gottesmm
Copy link
Contributor Author

I am going to get rid of the dead code commits in another PR to make this easier to review.

@gottesmm
Copy link
Contributor Author

@swift-ci smoke test

2 similar comments
@gottesmm
Copy link
Contributor Author

@swift-ci smoke test

@gottesmm
Copy link
Contributor Author

@swift-ci smoke test

@gottesmm gottesmm force-pushed the definite-init-fix branch 2 times, most recently from b634507 to 6708e99 Compare May 29, 2018 02:55
@gottesmm
Copy link
Contributor Author

@swift-ci smoke test and merge

4 similar comments
@gottesmm
Copy link
Contributor Author

@swift-ci smoke test and merge

@gottesmm
Copy link
Contributor Author

@swift-ci smoke test and merge

@gottesmm
Copy link
Contributor Author

@swift-ci smoke test and merge

@gottesmm
Copy link
Contributor Author

@swift-ci smoke test and merge

@gottesmm gottesmm force-pushed the definite-init-fix branch from 6708e99 to 4f83e63 Compare May 30, 2018 19:39
@gottesmm
Copy link
Contributor Author

I put in a special case for the lldb failures... = /.

@gottesmm
Copy link
Contributor Author

@swift-ci smoke test and merge

2 similar comments
@gottesmm
Copy link
Contributor Author

@swift-ci smoke test and merge

@gottesmm
Copy link
Contributor Author

@swift-ci smoke test and merge

…to be a load + destroy.

This ensures that DI creates dealloc_box in cases where the box is uninitialized
conditionally.

In the process, I also discovered that we were missing a test case for DI being
used by LLDB. Long term we shouldn't support that code pattern in the general
case, but for now we at least need a test case for it.

rdar://40332620
@gottesmm gottesmm force-pushed the definite-init-fix branch from 4f83e63 to 8886368 Compare May 30, 2018 21:26
@gottesmm
Copy link
Contributor Author

@swift-ci smoke test and merge

2 similar comments
@gottesmm
Copy link
Contributor Author

@swift-ci smoke test and merge

@gottesmm
Copy link
Contributor Author

@swift-ci smoke test and merge

@swift-ci swift-ci merged commit 9aca727 into swiftlang:master May 30, 2018
@gottesmm gottesmm deleted the definite-init-fix branch May 30, 2018 22:23
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.

3 participants