Skip to content

[semantic-sil] Use SILGenFunction::emitManagedBufferWithCleanup instead of entering in a random cleanup. #6792

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 2 commits into from
Jan 13, 2017

Conversation

gottesmm
Copy link
Contributor

[semantic-sil] Use SILGenFunction::emitManagedBufferWithCleanup instead of entering in a random cleanup.

This is a small cleanup where we are creating a cleanup for an uninitialized
buffer that will be verified as initialized later by DI and then creating a +0
unmanaged ManagedValue for the value.

This violates the "norms" of using ManagedValue in SILGen which is that one
should use the SILGenFunction::emitManaged* methods for creating/propagating
cleanups around.

…ad of entering in a random cleanup.

This is a small cleanup where we are creating a cleanup for an uninitialized
buffer that will be verified as initialized later by DI and then creating a +0
unmanaged ManagedValue for the value.

This violates the "norms" of using ManagedValue in SILGen which is that one
should use the SILGenFunction::emitManaged* methods for creating/propagating
cleanups around.
@gottesmm gottesmm requested a review from jckarter January 13, 2017 20:41
@gottesmm
Copy link
Contributor Author

Hey Joe. I am pretty sure this is correct, but I want to get another set of eyes to look at this. The only reason I can think that this would be incorrect is if in general we do not propagate the cleanups for uninitialized things that DI will handle.

@gottesmm
Copy link
Contributor Author

@swift-ci Please test

@gottesmm
Copy link
Contributor Author

@jckarter Sorry, should have @ed you.

@gottesmm
Copy link
Contributor Author

@ ed you.

@jckarter
Copy link
Contributor

Seems reasonable. It definitely looks fishy the way it is.

Copy link
Contributor

@jckarter jckarter left a comment

Choose a reason for hiding this comment

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

LGTM

@gottesmm gottesmm merged commit 9586a23 into swiftlang:master Jan 13, 2017
@gottesmm gottesmm deleted the small_silgen_fixup branch January 13, 2017 22:46
@gottesmm
Copy link
Contributor Author

Thanks Joe!

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