Skip to content

Conversation

Zalathar
Copy link
Contributor

Fixes #147761.

Fixing this is of somewhat marginal benefit, since users shouldn't be poking around with internal attribute anyway.

But an ICE on stable is still an ICE, and the fix is simple enough, so I think it's worth doing this to reduce future bug reports like #147756.

(Note that because #![feature(rustc_attrs)] is not enabled, the logic for suppressing “please report an ICE” is not activated, so the compiler actually does ask the user to send a bug report.)

…` step

By default, all `//@ build-fail` UI tests must first succeed in `check-pass`
mode, because build-fail normally only makes sense for tests that would check
successfully.

However, in some cases a full build will fail in a different way, e.g. due to
an ICE that is not present in check builds. For those tests, it's useful to be
able to opt out of the check-pass step.
@rustbot
Copy link
Collaborator

rustbot commented Oct 16, 2025

Some changes occurred in src/tools/compiletest

cc @jieyouxu

The rustc-dev-guide subtree was changed. If this PR only touches the dev guide consider submitting a PR directly to rust-lang/rustc-dev-guide otherwise thank you for updating the dev guide with your changes.

cc @BoxyUwU, @jieyouxu, @Kobzol, @tshepang

@rustbot rustbot added A-compiletest Area: The compiletest test runner A-rustc-dev-guide Area: rustc-dev-guide A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 16, 2025
@rustbot
Copy link
Collaborator

rustbot commented Oct 16, 2025

r? @davidtwco

rustbot has assigned @davidtwco.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@Zalathar
Copy link
Contributor Author

The //@ dont-require-check-pass directive is something I've come close to needing in the past for other tests/bugs, so I'm fine with adding it just for this test, since it's occasionally nice to have available.

@Zalathar
Copy link
Contributor Author

I suppose another possible implementation would be to have the attribute parser bail immediately as soon as it sees a #![feature(rustc_attrs)]) attribute without the feature enabled.

@Zalathar
Copy link
Contributor Author

Putting this on hold while I look into making the #![feature(rustc_attrs)] gate more reliable instead.

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 16, 2025
@rustbot
Copy link
Collaborator

rustbot commented Oct 16, 2025

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@rust-log-analyzer
Copy link
Collaborator

The job aarch64-gnu-llvm-20-1 failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)
test [crashes] tests/crashes/121963.rs ... ok
test [crashes] tests/crashes/121858.rs ... ok
test [crashes] tests/crashes/121575.rs ... ok
test [crashes] tests/crashes/122259.rs ... ok
2025-10-16T07:34:56.773290Z ERROR compiletest::runtest: fatal error, panic: "crashtest no longer crashes/triggers ICE, hooray! Please give it a meaningful name, add a doc-comment to the start of the test explaining why it exists and move it to tests/ui or wherever you see fit. Adding 'Fixes #<issueNr>' to your PR description ensures that the corresponding ticket is auto-closed upon merge. If you want to see verbose output, set `COMPILETEST_VERBOSE_CRASHES=1`."
test [crashes] tests/crashes/122681.rs ... FAILED
test [crashes] tests/crashes/122529.rs ... ok
test [crashes] tests/crashes/122903-1.rs ... ok
test [crashes] tests/crashes/122710.rs ... ok
test [crashes] tests/crashes/123140.rs ... ok
---

---- [crashes] tests/crashes/122681.rs stdout ----
------rustc stdout------------------------------

------rustc stderr------------------------------
error[E0658]: use of an internal attribute
##[error] --> /checkout/tests/crashes/122681.rs:2:1
  |
2 | #[rustc_layout_scalar_valid_range_start(1)]
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
---
For more information about an error, try `rustc --explain E0601`.

------------------------------------------

error: crashtest no longer crashes/triggers ICE, hooray! Please give it a meaningful name, add a doc-comment to the start of the test explaining why it exists and move it to tests/ui or wherever you see fit. Adding 'Fixes #<issueNr>' to your PR description ensures that the corresponding ticket is auto-closed upon merge. If you want to see verbose output, set `COMPILETEST_VERBOSE_CRASHES=1`.

thread '[crashes] tests/crashes/122681.rs' panicked at src/tools/compiletest/src/runtest/crashes.rs:17:18:
fatal error
stack backtrace:
   5: __rustc::rust_begin_unwind

@Noratrieb
Copy link
Member

Why not just make the use of an internal attribute error fatal? Not doing these kinds of fixes is exactly what the policy is about, and if we want people to not hit those on stable, we should just bail out immediately.

@Zalathar
Copy link
Contributor Author

Yeah, that’s more or less what I realised after submitting this.

As soon as we see an internal attribute without the feature enabled, we should stop compilation immediately. Then panicking in later code doesn’t matter, because it can only happen if the feature is on.

@Zalathar
Copy link
Contributor Author

I’ll close this PR, and open a new one when I figure out where to put the fatal error.

@Zalathar Zalathar closed this Oct 16, 2025
@rustbot rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Oct 16, 2025
@Zalathar Zalathar deleted the non-scalar-valid-range branch October 16, 2025 08:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-compiletest Area: The compiletest test runner A-rustc-dev-guide Area: rustc-dev-guide A-testsuite Area: The testsuite used to check the correctness of rustc T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ICE: Misuse of #[rustc_layout_scalar_valid_range_start(..)] causes ICE even though#![feature(rustc_attrs)] is not enabled

5 participants