-
-
Notifications
You must be signed in to change notification settings - Fork 14.2k
attempt to fix unreachable code regression #149664
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r=me after fixing nit
bc43cdc to
7160ae5
Compare
This comment has been minimized.
This comment has been minimized.
7160ae5 to
2a2da78
Compare
|
@bors r=davidtwco |
Rollup of 5 pull requests Successful merges: - #144938 (Enable `outline-atomics` by default on more AArch64 platforms) - #146579 (Handle macro invocation in attribute during parse) - #149400 (unstable proc_macro tracked::* rename/restructure) - #149664 (attempt to fix unreachable code regression ) - #149806 (Mirror `ubuntu:24.04` on ghcr) Failed merges: - #149789 (Cleanup in the attribute parsers) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 5 pull requests Successful merges: - #144938 (Enable `outline-atomics` by default on more AArch64 platforms) - #146579 (Handle macro invocation in attribute during parse) - #149400 (unstable proc_macro tracked::* rename/restructure) - #149664 (attempt to fix unreachable code regression ) - #149806 (Mirror `ubuntu:24.04` on ghcr) Failed merges: - #149789 (Cleanup in the attribute parsers) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #149664 - Kivooeo:infallible-type-unreachable-code, r=davidtwco attempt to fix unreachable code regression For some reason it works, it checks function output type and suppress warning if type is uninhabited ~~This double negations in code breaks my mind actually~~ I'd love to revisit this part in future and try to find a proper solution maybe, but for now I feel like it's enough before release to fix the issue? I really wonder what team does think, especially `@cjgillot` and other people who are more confident in this part of compiler than I do I tried a lot of things here, it's only approach that pass all tests included new regression one fixes #149571 r? compiler
Rollup of 5 pull requests Successful merges: - rust-lang/rust#144938 (Enable `outline-atomics` by default on more AArch64 platforms) - rust-lang/rust#146579 (Handle macro invocation in attribute during parse) - rust-lang/rust#149400 (unstable proc_macro tracked::* rename/restructure) - rust-lang/rust#149664 (attempt to fix unreachable code regression ) - rust-lang/rust#149806 (Mirror `ubuntu:24.04` on ghcr) Failed merges: - rust-lang/rust#149789 (Cleanup in the attribute parsers) r? `@ghost` `@rustbot` modify labels: rollup
|
@rust-timer build 2fe9b51 Trying if this regressed perf. in #149818. |
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (2fe9b51): comparison URL. Overall result: ❌ regressions - please read the text belowBenchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf. Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @bors rollup=never Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary 1.4%, secondary 1.3%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 471.772s -> 476.618s (1.03%) |
|
It seems like this caused a perf. regression. Do you think something could be done to reduce it? |
|
Hmm, I think that I can try (not sure if it help or not) What will happen if I push changes to this merged branch? |
|
That's not really useful for anything, it should be a new PR :) |
For some reason it works, it checks function output type and suppress warning if type is uninhabited
This double negations in code breaks my mind actuallyI'd love to revisit this part in future and try to find a proper solution maybe, but for now I feel like it's enough before release to fix the issue? I really wonder what team does think, especially @cjgillot and other people who are more confident in this part of compiler than I do
I tried a lot of things here, it's only approach that pass all tests included new regression one
fixes #149571
r? compiler