Conversation
2a24467 to
06ca7b7
Compare
|
Ah, but validation complains about... Since promoteds are constants and constants may not point to statics. |
|
Meanwhile, let's see if this affects perf. |
|
Awaiting bors try build completion. |
|
⌛ Trying commit 06ca7b7 with merge d24e527f2368bc3de48bd2932d21d8ced60d3318... |
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
☀️ Try build successful - checks-actions |
|
Queued d24e527f2368bc3de48bd2932d21d8ced60d3318 with parent 59aaa2a, future comparison URL. @rustbot label: +S-waiting-on-perf |
ah lol, I guess we need to differentiate between promoteds in statics and promoteds elsewhere? |
|
I guess promoted in consts should not contain refs to statics, yeah... |
|
Finished benchmarking try commit (d24e527f2368bc3de48bd2932d21d8ced60d3318): comparison url. Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up. @bors rollup=never |
|
Looks like there's a perf hit of up to 4.5% for some examples. |
|
I fixed the breakage, tests should pass now (the ones I ran locally did). What remains is deciding if we are okay with the perf hit. |
|
So... I'm ok with this perf hit, though I'm wondering if we should somehow "turn it off" and just keep it as a sort of debug assertion that we aren't doing anything bad with promotion. I mean... this should never actually cause a validation failure afaict. |
|
This would be true if we did not promote |
|
Riiight, those. So let's do this now and consider moving to debug assertions once we know the result of the crater run @bors r+ |
|
📌 Commit 97cae9c has been approved by |
|
☀️ Test successful - checks-actions |
Turn on const-value validation for promoteds. This is made possible now that #67534 is resolved.
I don't think this is a breaking change. We don't promote any unsafe operation any more (since #77526 landed). We do promote
const fncalls under some circumstances (inconst/staticinitializers), but union field access and similar operations are not allowed inconst fn. So now is a perfect time to add this check. :Dr? @oli-obk
Fixes #67465