check types of const param defaults #139646
Conversation
rustbot
added
S-waiting-on-review
lcnr
changed the title
lcnr
added
T-types
|
This PR causes This feel like a clear bugfix to me and I don't expect any fallout, so I didn't run crater here. Any breakage will be caught by the beta crater run and this is trivial to revert if necessary. @rfcbot fcp merge |
|
Team member @lcnr has proposed to merge this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
rfcbot
added
proposed-final-comment-period
|
This makes sense to me, unlike "normal" cases where some possible instantiation of the generic parameter could result in things being wf, it is not possible for any well formed argument to I feel somewhat 🤔 about not cratering this, as a general policy i don't think its great to say "we can just look at the results of the beta crater run" as it's putting more work on the release team. I imagine a crater run would finish before the FCP 10 day period ends so I don't think there's any harm in just starting a try run and doing a crater run afterwards? I'm fine to not crater this though if noone else on the team thinks its worthwhile |
|
@bors try |
check types of const param defaults fixes rust-lang#139643 by checking that the type of a const parameter default matches the type of the parameter as long as both types are fully concrete r? `@BoxyUwU`
|
☀️ Try build successful - checks-actions |
|
waiting with crater run for top-1000 in #133502 |
|
@craterbot check |
|
👌 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
craterbot
added
S-waiting-on-crater
|
🚧 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
|
🎉 Experiment
|
craterbot
added
S-waiting-on-review
rfcbot
added
final-comment-period
|
🔔 This is now entering its final comment period, as per the review above. 🔔 |
|
All the crater errors are spurrious. There's one spurrious fail -> ICE regression but I can't tell if that was ICEing before. Regardless it depends on a whole host of unstable features (including gce) so I think its safe to say there's no breakage from this 👍 |
BoxyUwU
removed
the
S-waiting-on-review
|
or a revision without the feature gates enabled 🤔 yeah, just something that makes it more clear this affects stuff without feature gates. though if you think it doesn't really matter and the current test is fine then that's also fine with me |
rfcbot
added
finished-final-comment-period
|
The final comment period, with a disposition to merge, as per the review above, is now complete. As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed. This will be merged soon. |
rust-cloud-vms
bot
force-pushed
the
default-is-fully-concrete
branch
from
3e80f96 to
8cd12bf
Compare
|
@rustbot ready |
rustbot
added
S-waiting-on-review
|
@bors r+ thanks gamer |
bors
added
S-waiting-on-bors
…BoxyUwU check types of const param defaults fixes rust-lang#139643 by checking that the type of a const parameter default matches the type of the parameter as long as both types are fully concrete r? `@BoxyUwU`
…xyUwU check types of const param defaults fixes rust-lang#139643 by checking that the type of a const parameter default matches the type of the parameter as long as both types are fully concrete r? `@BoxyUwU`
…BoxyUwU check types of const param defaults fixes rust-lang#139643 by checking that the type of a const parameter default matches the type of the parameter as long as both types are fully concrete r? `@BoxyUwU`
|
@bors retry put this in a rollup five minutes too late |
Rollup of 7 pull requests Successful merges: - rust-lang#137439 (Stabilise `std::ffi::c_str`) - rust-lang#138737 (uefi: Update r-efi) - rust-lang#139646 (check types of const param defaults) - rust-lang#140220 (Fix detection of main function if there are expressions around it) - rust-lang#140291 (Correctly display stdout and stderr in case a doctest is failing) - rust-lang#140297 (Update example to use CStr::to_string_lossy) - rust-lang#140330 (Clarified bootstrap optimization "true" argument) r? `@ghost` `@rustbot` modify labels: rollup
|
☀️ Test successful - checks-actions |
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 43e62a7 (parent) -> 496145b (this PR) Test differencesShow 2 test diffsStage 1
Stage 2
Job group index
Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard 496145b9cc023aef4bb1f16c0964a53d0da36c88 --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 (496145b): comparison URL. Overall result: ❌ regressions - no action needed@rustbot label: -perf-regression Instruction countThis is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.
Max RSS (memory usage)Results (primary -0.7%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResults (primary -0.9%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 764.554s -> 764.349s (-0.03%) |
fixes #139643 by checking that the type of a const parameter default matches the type of the parameter as long as both types are fully concrete
r? @BoxyUwU