Const check/promotion cleanup and sanity assertion#71198
Const check/promotion cleanup and sanity assertion#71198bors merged 7 commits intorust-lang:masterfrom
Conversation
rust-highfive
added
the
S-waiting-on-review
There was a problem hiding this comment.
idk, I just copied the test
There was a problem hiding this comment.
Copied from where?
Also I thought borrowck=migrate is on its way out of the compiler, so I am surprised to see new tests for it (Cc @rust-lang/wg-compiler-nll)
There was a problem hiding this comment.
There was a problem hiding this comment.
Okay so the header is the same but the rest of the test is very different.
I still have no clue why this is testing what, and what it has to do with the rest of the PR.
There was a problem hiding this comment.
So the commit title is "Add test for passing promotion but failing compilation ".
But I don't entirely understand what that is for, doesn't that happen all the time? The delay_span_bug is for failing promotion but passing compilation, i.e., the exact opposite.
There was a problem hiding this comment.
This test was originally for explicit promotion, so maybe it doesn't make sense anymore
There was a problem hiding this comment.
What kind of strange hack is this?
There was a problem hiding this comment.
I found the read_only! macro definition but it seems undocumented.
I am calling it a hack because the macro looks like it could just be a method instead, so I am probably missing the reason why it is a macro (and the macro name is entirely unhelpful, not indicating it has anything to do with MIR bodies).
There was a problem hiding this comment.
unrelated to this PR, but yea, this can likely become a method. I think the reason it's a macro is multiple refactorings after each other removing the need for the macro at some point
There was a problem hiding this comment.
I think it is a macro because it has a signature like fn(&mut self) -> &Self, which runs into a shortcoming of the borrow checker where the returned reference is not allowed to alias.
There was a problem hiding this comment.
Well that would be a useful thing to have in a doc comment. ;)
RalfJung
left a comment
RalfJung
left a comment
There was a problem hiding this comment.
This LGTM, except for that new testcase which seems out-of-place to me.
oli-obk
force-pushed
the
const_check_cleanup
branch
from
a50bc6b to
316a401
Compare
|
I removed the test commit @bors r=RalfJung |
|
📌 Commit 316a401 has been approved by |
bors
added
S-waiting-on-bors
…Jung Const check/promotion cleanup and sanity assertion r? @RalfJung This is just the part of rust-lang#70042 (comment) that does not change behaviour
|
☔ The latest upstream changes (presumably #71306) made this pull request unmergeable. Please resolve the merge conflicts. |
bors
added
S-waiting-on-author
…g had content there before the current changes
This renaming was already done in some modules via import renaming. It's strictly used as a context, and it contains a `TyCtxt`.
oli-obk
force-pushed
the
const_check_cleanup
branch
from
316a401 to
4cdc31b
Compare
|
rebased @bors r=RalfJung |
|
📌 Commit 4cdc31b has been approved by |
bors
added
S-waiting-on-bors
Rollup of 6 pull requests Successful merges: - rust-lang#71005 (Reading from the return place is fine) - rust-lang#71198 (Const check/promotion cleanup and sanity assertion) - rust-lang#71396 (Improve E0308 error message wording again) - rust-lang#71452 (Remove outdated reference to interpreter snapshotting) - rust-lang#71454 (Inline some function docs in `core::ptr`) - rust-lang#71461 (Improve E0567 explanation) Failed merges: r? @ghost
5 participants
r? @RalfJung
This is just the part of #70042 (comment) that does not change behaviour