Skip to content

Fix MemoryLifetimeVerifier for enums with empty/non-uniform element types #39173

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

Closed
wants to merge 1 commit into from

Conversation

meg-gupta
Copy link
Contributor

@meg-gupta meg-gupta commented Sep 4, 2021

MemoryLifetimeVerifier currently does not do further analysis when it sees
an empty element type or a non-uniform element type.
But this behaviour is not the same when such element types only have
an inject_enum_addr without an init_enum_data_addr.
This PR makes this behaviour consistent.

MemoryLifetimeVerifier currently does not do further analysis when it sees
an empty element type or a mismatched element type.
But this behaviour is not the same when such element types only have
an inject_enum_addr without an init_enum_data_addr.
This PR makes this behaviour consistent.
@meg-gupta meg-gupta requested a review from eeckstein September 4, 2021 03:37
@meg-gupta
Copy link
Contributor Author

@swift-ci test

@meg-gupta meg-gupta changed the title Fix MemoryLifetimeVerifier for enums with empty/mismatched element types Fix MemoryLifetimeVerifier for enums with empty/non-uniform element types Sep 4, 2021
Copy link
Contributor

@eeckstein eeckstein left a comment

Choose a reason for hiding this comment

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

That's not the best way to fix this.
It would be better to model the "triviality" as something which is propagated in the data flow.
It would replace isEnumTrivialAt (which is only an insufficient workaround).

@eeckstein
Copy link
Contributor

Actually, I was wrong. The best way to fix this is to handle "empty" payloads like no-payload cases: #39200

br bb3

bb3:
%12 = load [take] %0 : $*Result<Int, Error>
Copy link
Contributor

@eeckstein eeckstein Sep 8, 2021

Choose a reason for hiding this comment

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

I think this should be considered as illegal SIL. This load [take] is loading an uninitialized value (the Int-case coming from bb1). It will not crash, because the only thing which is done is to destroy the value. But still, loading an uninitialized value is not something we should be allowing in SIL.

In my PR (#39200) I have put this test case into memory_lifetime_failures.sil

@meg-gupta
Copy link
Contributor Author

#39200 handles this

@meg-gupta meg-gupta closed this Sep 8, 2021
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