Fix ICE: When Trying to check visibility of a #[type_const], check RHS instead.#151085
Fix ICE: When Trying to check visibility of a #[type_const], check RHS instead.#151085rust-bors[bot] merged 1 commit intorust-lang:mainfrom
Conversation
|
@BoxyUwU Any changes I need to make? |
|
@rustbot author |
|
Reminder, once the PR becomes ready for a review, use |
|
@rustbot ready, if it looks all good. I can then flatten and rebase it with main. |
|
You'll need to update your PR description too as the TooGeneric change is no longer present. Can you change the title to something a bit more informative (similar to the comment in the test). There's also no need to link the fixed issue in the title as it's part of the PR description |
@BoxyUwU Fair enough I have updated the description, and title. =) |
…S instead. We want to evaluate the rhs of a type_const. Also added an early return/guard in eval_in_interpreter which is used in functions like `eval_to_allocation_raw_provider` Lastly add a debug assert to `thir_body()` if we have gotten there with a type_const something as gone wrong. Get rid of a call to is_type_const() and instead use a match arm. Change this is_type_const() check to a debug_assert!() Change to use an if else statment instead. Update type_const-pub.rs Fix formatting. Noticed that this is the same check as is_type_const() centralize it. Add test case for pub type_const.
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
@BoxyUwU Squashed and rebased onto main. |
|
@bors r+ rollup |
This comment has been minimized.
This comment has been minimized.
What is this?This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.Comparing 844f131 (parent) -> 9f6cd6d (this PR) Test differencesShow 4 test diffsStage 1
Stage 2
Additionally, 2 doctest diffs were found. These are ignored, as they are noisy. Job group index
Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard 9f6cd6defbd7ef13f6777aa8e43b14d69f0a830a --output-dir test-dashboardAnd then open Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
|
Finished benchmarking commit (9f6cd6d): comparison URL. Overall result: ❌✅ regressions and improvements - no action needed@rustbot label: -perf-regression 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 -0.8%, secondary 1.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 2.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary -0.1%, secondary -0.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 472.032s -> 472.543s (0.11%) |
This PR fixes #150956 for min_const_generic_args #132980.
The first part of this PR checks in
reachable.rsif we have a type const usevisit_const_item_rhs(init);instead of callingconst_eval_poly_to_alloc()The second part is I noticed that if
const_eval_poly_to_alloc()returns aErrorHandled::TooGenericthen switches tovisit_const_item_rhs(). So further upconst_eval_poly_to_alloc()call order it eventual gets toeval_in_interpreter(). So I added a debug_assert ineval_in_interpreter()if we happen to try and run CTFE on atype_const.The other bit is just a test case, and some duplicated code I noticed.
While the PR does get rid of the ICE for a type_const with
pubvisibility. If I try using the const though it will ICE with the following:I suspect it is a similar issue to inherent associated consts. Since if I apply the same fix I had here:
#150799 (comment)
I then can use the const internally and in external crates without any issues.
Although, did not include that fix since if it is a similar issue it would need to be addressed elsewhere.
r? @BoxyUwU
@rustbot label +F-associated_const_equality +F-min_generic_const_args