Remove git repository from git config
It is no longer needed after https://github.com/rust-lang/rust/pull/138591. We could even remove the `nightly_branch` field, but it still has one usage.
r? ``@jieyouxu``
Suggest {to,from}_ne_bytes for transmutations between arrays and integers, etc
implements #136067
Rust has helper methods for many kinds of safe transmutes, for example integer<->bytes. This is a lint against using transmute for these cases.
```rs
fn bytes_at_home(x: [u8; 4]) -> u32 {
transmute(x)
}
// other examples
transmute::<[u8; 2], u16>();
transmute::<[u8; 8], f64>();
transmute::<u32, [u8; 4]>();
transmute::<char, u32>();
transmute::<u32, char>();
```
It would be handy to suggest `u32::from_ne_bytes(x)`.
This is implemented for `[u8; _]` -> `{float int}`
This also implements the cases:
`fXX` <-> `uXX` = `{from_bits, to_bits}`
`uXX` -> `iXX` via `cast_unsigned` and `cast_signed`
{`char` -> `u32`, `bool` -> `n8`} via `from`
`u32` -> `char` via `from_u32_unchecked` (note: notes `from_u32().unwrap()`) (contested)
`u8` -> `bool` via `==` (debatable)
---
try-job: aarch64-gnu
try-job: test-various
minicore: Have `//@ add-core-stubs` also imply `-Cforce-unwind-tables=yes`
To preserve CFI directives in assembly tests, as `//@ add-core-stubs` already imply `-C panic=abort`.
This is a blocker for https://github.com/rust-lang/rust/pull/140037#issuecomment-2816665358.
cc ```@RalfJung```
r? ```@bjorn3```
Clippy subtree update
r? `@Manishearth`
Cargo.lock update due to the Clippy version bump and because Clippy moved from rinja (unmaintained) to askama.
Last sync was skipped due to the askama issue and me not getting to fixing this in time.
compiletest: Use the new non-libtest executor by default
The new executor was implemented in #139660, but required a manual opt-in. This PR activates the new executor by default, but leaves the old libtest-based executor in place (temporarily) to make reverting easier if something unexpectedly goes horribly wrong.
Currently the new executor can be explicitly disabled by passing the `-N` flag to compiletest (e.g. `./x test ui -- -N`), but eventually that flag will be removed, alongside the removal of the libtest dependency. The flag is mostly there to make manual comparative testing easier if something does go wrong.
As before, there *should* be no user-visible difference between the old executor and the new executor.
---
I didn't get much of a response to my [call for testing thread on Zulip](https://rust-lang.zulipchat.com/#narrow/channel/122651-general/topic/Call.20for.20testing.3A.20New.20test.20executor.20for.20compiletest/with/512452105), and the reports I did get (along with my own usage) indicate that there aren't any problems. So I think it's reasonable to move forward with making this the default, in the hopes of being able to remove the libtest dependency relatively soon.
When the libtest dependency is removed, it should be reasonable to build compiletest against pre-built stage0 std by default, even after the stage0 redesign. (Though we should probably have at least one CI job using in-tree stage1 std instead, to guard against the possibility of the `#![feature(internal_output_capture)]` API actually changing.)
Refactor git change detection in bootstrap
While working on https://github.com/rust-lang/rust/pull/138395, I finally found the courage to delve into the insides of git path change detection in bootstrap, which is used (amongst other things) to detect if we should rebuilt od download `[llvm|rustc|gcc]`. I found it a bit hard to understand, and given that this code was historically quite fragile, I thought that it would be better to rebuild it from scratch.
The previous approach had a bunch of limitations:
- It separated the computation of "are there local changes?" and "what upstream SHA should we use?" even though these two things are intertwined.
- It used hacks to work around what happens on CI.
- It had special cases for CI scattered throughout the codebase, rather than centralized in one place.
- It wasn't documented enough and didn't have tests for the git behavior.
The current approach should hopefully resolve all of that. I implemented a single entrypoint called `check_path_modifications` (naming bikeshed pending, half of the time I spend on this PR was thinking about names, as it's quite tricky here..) that explicitly receives a mode of operation (in CI or outside CI), and accordingly figures out that upstream SHA that we should use for downloading artifacts and it also figures out if there are any local changes. Users of this function can then use this unified output to implement `download-ci-X` and other functionality. Notably, this change detection no longer uses `git merge-base`, which makes it easier to use and doesn't require setting up remotes.
I also added a bunch of integration tests that literally spawn a git repository on disk and then check that the function can deal with various situations (PR CI, auto/try CI, local builds).
After I built this inner layer, I used it for downloading GCC, LLVM and rustc. The latter two (and especially rustc) were using the `last_modified_commit` function before, but in all cases but one this function was actually only used to check if there are any local changes, which was IMO confusing. The LLVM handling would deserve a bit of refactoring, but that's a larger change that can be done as a follow-up.
I hope that the implementation is now clear and easy to understand, so that in combination with the tests we can have more confidence that it does what we want. I tried to include a lot of documentation in the code, so I won't be repeating the actual implementation details here, if there are any questions, I'll add the answers to the documentation too :)
The new approach explicitly supports three scenarios:
- Running on PR CI, where we have one upstream bors parent commit and one PR merge commit made by GitHub.
- Running on try/auto CI, where we have one upstream bors parent commit and one PR merge commit made by bors.
- Running locally, where we assume that we have at least one upstream bors parent commit in our git history.
I removed the handling of upstreams on CI, as I think that it shouldn't be needed and I considered it to be a hack. However, it's possible that there are other use-cases that I haven't considered, so I want to ask around if people have other situations than the three use-cases described above. If there are other such use-cases, I would like to include them in the new centralized implementation and add them to the git test suite, rather than going back to the old ways :)
In particular, the code before relied on `git merge-base`, but I don't see why we can't just lookup the most recent bors commit and assume that is a merge commit that is also upstream? I might be running into Chesterton's Fence here :)
CC `@pietroalbini` To make sure that this won't break downstream users of Rust's CI.
Best reviewed commit by commit.
Companion PRs:
- For testing beta: https://github.com/rust-lang/rust/pull/138597
r? `@onur-ozkan`
Fixes: https://github.com/rust-lang/rust/issues/101907
try-job: x86_64-gnu-aux
try-job: aarch64-gnu
try-job: dist-x86_64-apple
Currently the new executor can be explicitly disabled by passing the `-N` flag
to compiletest (e.g. `./x test ui -- -N`), but eventually that flag will be
removed, alongside the removal of the libtest dependency.
Remove `token::{Open,Close}Delim`
By replacing them with `{Open,Close}{Param,Brace,Bracket,Invisible}`.
PR #137902 made `ast::TokenKind` more like `lexer::TokenKind` by
replacing the compound `BinOp{,Eq}(BinOpToken)` variants with fieldless
variants `Plus`, `Minus`, `Star`, etc. This commit does a similar thing
with delimiters. It also makes `ast::TokenKind` more similar to
`parser::TokenType`.
This requires a few new methods:
- `TokenKind::is_{,open_,close_}delim()` replace various kinds of
pattern matches.
- `Delimiter::as_{open,close}_token_kind` are used to convert
`Delimiter` values to `TokenKind`.
Despite these additions, it's a net reduction in lines of code. This is
because e.g. `token::OpenParen` is so much shorter than
`token::OpenDelim(Delimiter::Parenthesis)` that many multi-line forms
reduce to single line forms. And many places where the number of lines
doesn't change are still easier to read, just because the names are
shorter, e.g.:
```
- } else if self.token != token::CloseDelim(Delimiter::Brace) {
+ } else if self.token != token::CloseBrace {
```
r? `@petrochenkov`
jsondocck: Require command is at start of line
In one place we use `///``@``` instead of `//``@`.`` The test-runner allowed it, but it probably shouldn't. Ran into by ``@lolbinarycat`` in https://github.com/rust-lang/rust/pull/132748#issuecomment-2816469322:
```
error: unknown disambiguator `?(`
##[error] --> /checkout/tests/rustdoc-json/fns/return_type_alias.rs:3:25
|
3 | ///@ set foo = "$.index[?(``@.name=='Foo')].id"``
| ^^
|
```
Maybe it's also worth erroring on this like we added in #137103
r? ``@GuillaumeGomez``
Use `output_base_dir` for `mir_dump_dir`
It just occurred to me that the problem might be due to multiple revisions using the same dump directory (and therefore deleting the other revision's dir). This fixes that by simply using the normal per-test output directory, which is revision safe.
By replacing them with `{Open,Close}{Param,Brace,Bracket,Invisible}`.
PR #137902 made `ast::TokenKind` more like `lexer::TokenKind` by
replacing the compound `BinOp{,Eq}(BinOpToken)` variants with fieldless
variants `Plus`, `Minus`, `Star`, etc. This commit does a similar thing
with delimiters. It also makes `ast::TokenKind` more similar to
`parser::TokenType`.
This requires a few new methods:
- `TokenKind::is_{,open_,close_}delim()` replace various kinds of
pattern matches.
- `Delimiter::as_{open,close}_token_kind` are used to convert
`Delimiter` values to `TokenKind`.
Despite these additions, it's a net reduction in lines of code. This is
because e.g. `token::OpenParen` is so much shorter than
`token::OpenDelim(Delimiter::Parenthesis)` that many multi-line forms
reduce to single line forms. And many places where the number of lines
doesn't change are still easier to read, just because the names are
shorter, e.g.:
```
- } else if self.token != token::CloseDelim(Delimiter::Brace) {
+ } else if self.token != token::CloseBrace {
```
simd intrinsics with mask: accept unsigned integer masks, and fix some of the errors
It's not clear at all why the mask would have to be signed, it is anyway interpreted bitwise. The backend should just make sure that works no matter the surface-level type; our LLVM backend already does this correctly. The note of "the mask may be widened, which only has the correct behavior for signed integers" explains... nothing? Why can't the code do the widening correctly? If necessary, just cast to the signed type first...
Also while we are at it, fix the errors. For simd_masked_load/store, the errors talked about the "third argument" but they meant the first argument (the mask is the first argument there). They also used the wrong type for `expected_element`.
I have extremely low confidence in the GCC part of this PR.
See [discussion on Zulip](https://rust-lang.zulipchat.com/#narrow/channel/257879-project-portable-simd/topic/On.20the.20sign.20of.20masks)
Do not remove trivial `SwitchInt` in analysis MIR
This PR ensures that we don't prematurely remove trivial `SwitchInt` terminators which affects both the borrow-checking and runtime semantics (i.e. UB) of the code. Previously the `SimplifyCfg` optimization was removing `SwitchInt` terminators when they was "trivial", i.e. when all arms branched to the same basic block, even if that `SwitchInt` terminator had the side-effect of reading an operand which (for example) may not be initialized or may point to an invalid place in memory.
This behavior is unlike all other optimizations, which are only applied after "analysis" (i.e. borrow-checking) is finished, and which Miri disables to make sure the compiler doesn't silently remove UB.
Fixing this code "breaks" (i.e. unmasks) code that used to borrow-check but no longer does, like:
```rust
fn foo() {
let x;
let (0 | _) = x;
}
```
This match expression should perform a read because `_` does not shadow the `0` literal pattern, and the compiler should have to read the match scrutinee to compare it to 0. I've checked that this behavior does not actually manifest in practice via a crater run which came back clean: https://github.com/rust-lang/rust/pull/139042#issuecomment-2767436367
As a side-note, it may be tempting to suggest that this is actually a good thing or that we should preserve this behavior. If we wanted to make this work (i.e. trivially optimize out reads from matches that are redundant like `0 | _`), then we should be enabling this behavior *after* fixing this. However, I think it's kinda unprincipled, and for example other variations of the code don't even work today, e.g.:
```rust
fn foo() {
let x;
let (0.. | _) = x;
}
```
Clean UI tests 4 of n
Cleaned up some tests that have `issue` in the title. I kept the commits to be one per "`issue`" cleanup/rename to make it easier to check. I can rebase to one commit once the changes are approved.
Related Issues:
#73494#133895
r? jieyouxu
Rewrite on_unimplemented format string parser.
This PR rewrites the format string parser for `rustc_on_unimplemented` and `diagnostic::on_unimplemented`. I plan on moving this code (and more) into the new attribute parsing system soon and wanted to PR it separately.
This PR introduces some minor differences though:
- `rustc_on_unimplemented` on trait *implementations* is no longer checked/used - this is actually never used (outside of some tests) so I plan on removing it in the future.
- for `rustc_on_unimplemented`, it introduces the `{This}` argument in favor of `{ThisTraitname}` (to be removed later). It'll be easier to parse.
- for `rustc_on_unimplemented`, `Self` can now consistently be used as a filter, rather than just `_Self`. It used to not match correctly on for example `Self = "[{integer}]"`
- Some error messages now have better spans.
Fixes https://github.com/rust-lang/rust/issues/130627
