move borrow checker tests#154473
Conversation
rustbot
added
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
danieljofficial
force-pushed
the
move-tests-borrowck
branch
from
f71695d to
06b9cce
Compare
|
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. |
There was a problem hiding this comment.
Hey, I think it would be helpful to include the corresponding issue link near the top of the file. It's a bit hectic to search for the issue by number instead of having a direct link in the code. This way, one can easily ctrl + click on the link or use the gf or ge motion in Vim or Neovim.
I am unsure if other reviewers have the same opinion. If they do, please update in all such files ( :
If you want to make it more beautiful, you can put the link in between angle brackets <>.
There was a problem hiding this comment.
Hey, I think it would be helpful to include the corresponding issue link near the top of the file. It's a bit hectic to search for the issue by number instead of having a direct link in the code. This way, one can easily
ctrl+ click on the link or use thegforgemotion in Vim or Neovim.I am unsure if other reviewers have the same opinion. If they do, please update in all such files ( : If you want to make it more beautiful, you can put the link in between angle brackets
<>.
Must have missed those three, thank you.
There was a problem hiding this comment.
Although tests/ui/borrowck is still a better choice in my opinion, it may also belong to following:
tests/ui/lifetimes: Because variablelinedoesn't live long enough, and the description oftests/ui/lifetimesin thetests/ui/README.mdsays:
Broad directory on lifetimes, including proper specifiers, lifetimes not living long enough, or undeclared lifetime names.
But, since it's the job of the borrow-checker to handle this functionality, your choice is better. It's just another option.
Personal opinion 1: The file name can be prefixed with add- so that it will become add-assign-...; just clearer.
danieljofficial
force-pushed
the
move-tests-borrowck
branch
from
06b9cce to
d8c8028
Compare
There was a problem hiding this comment.
Ah sorry I forgot, this one too ( :
There was a problem hiding this comment.
There is one more thing that randomly hit my mind when I was having dinner. Some of these test files do not contain any compiletest directives. You can try adding them.
I think the default directive is run-pass, which is costly. Therefore, if we can use check-pass or build-pass, it would be better.
default is |
Oh. got it. |
|
cool, thanks and sorry for a delay a lot of stuff has happened latelyyyyy @bors r+ rollup |
rust-bors
bot
added
S-waiting-on-bors
…uwer Rollup of 15 pull requests Successful merges: - #153995 (Use convergent attribute to funcs for GPU targets) - #154184 (stabilize s390x vector registers) - #151898 (constify DoubleEndedIterator) - #154235 (remove unnecessary variables and delimiter check) - #154473 (move borrow checker tests) - #154745 (Replace span_look_ahead with span_followed_by) - #154778 (make field representing types invariant over the base type) - #154867 (Fix private fields diagnostics and improve error messages) - #154879 (Don't store `pattern_ty` in `TestableCase`) - #154910 (Suppress `unreachable_code` lint in `derive(PartialEq, Clone)`) - #154923 (Fix ICE in next-solver dyn-compatibility check) - #154934 (Add getters for `rustc_pattern_analysis::constructor::Slice` fields) - #154938 (match exhaustiveness: Show the guard exhaustivity note only when it's the guards alone that cause non-exhaustiveness) - #154961 (Use derived impl for `GappedRange` subdiagnostic) - #154980 (rustc-dev-guide subtree update)
4 participants
Hi, I have moved some tests I think should be in the borrowck category. Please let me know if there are issues. r? @Kivooeo
Related to #133895