Skip to content

[MIR] Fix return value when computed properties conflict with given prop #109923

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
Sep 25, 2024

Conversation

gargaroff
Copy link
Contributor

This fixes a test failure when expensive checks are enabled. Use the correct return value when computing machine function properties resulted in an error (e.g. when conflicting with explicitly set values).

Without this, the machine verifier would crash even in the presence of parsing errors which should have gently terminated execution.

This fixes a test failure when expensive checks are enabled.
Use the correct return value when computing machine function properties
resulted in an error (e.g. when conflicting with explicitly set values).

Without this, the machine verifier would crash even in the presence of
parsing errors which should have gently terminated execution.
@gargaroff
Copy link
Contributor Author

Follow up to the failures coming from #108546. I realized that we returned false instead of true, leading the parser to believe that parsing was successful. With expensive checks enabled, this caused a crash coming from the newly added conflict test.

After using the correct return value, parsing is terminated gracefully with the error and the verifier doesn't crash anymore. This also required splitting up the conflict test as the other functions in the file wouldn't get parsed anymore.

Also noticed that initializeCallSiteInfo had the same issue, so changed the return value there as well.

@gargaroff gargaroff merged commit d853ade into llvm:main Sep 25, 2024
8 of 10 checks passed
@gargaroff gargaroff deleted the gargaroff/fix_return_value branch September 25, 2024 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants