Skip to content
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

Don't trust hasStackUsageOutput/hasOptOutput #6471

Merged
merged 1 commit into from May 12, 2024

Conversation

OfekShilon
Copy link
Member

... they can be true for compilations that failed and really have no stackUsageOutput/optOutput
This fixes #6468 alright, but I'm not sure about the followup questions:

(1) Is there really a point in keeping the boolean members hasStackUsageOutput/hasOptOutput?
(2) What about other has* members - hasPpOutput, hasAstOutput, hasIrOutput etc? I didn't give them similar treatment as I couldn't reproduce similar problems.

I tend to stick with this quick-n-dirty-ish solution but would appreciate opinions.

…or compilations that failed and really have no `stackUsageOutput`/`optOutput`
@github-actions github-actions bot added the ui label May 11, 2024
@RubenRBS
Copy link
Member

You're right that just checking if the response has the fields should be enough - Happy to merge as-is, but maybe a less breaking change for now until we investigate/improve that is to check for both variables? wdyt?

@OfekShilon
Copy link
Member Author

Thanks!
I looked into it again, and am now pretty much comfortable with removing the has*** members. But it has some technical type quirks, so I prefer to do it in a different PR - and will merge this for now.

@OfekShilon OfekShilon merged commit 8559ea5 into compiler-explorer:main May 12, 2024
9 checks passed
@mattgodbolt
Copy link
Member

This is rolling out now, thanks ofek

@OfekShilon OfekShilon deleted the Fix6468 branch May 15, 2024 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG]: opt-remarks/stack-usage panes can hide compilation error messages
3 participants