Skip to content

Conversation

@Zalathar
Copy link
Member

@Zalathar Zalathar commented Nov 5, 2025

This PR is a combination of three changes:

  • Store the list of known directives in a set, so that checking a directive name doesn't require a linear scan
  • Extract the known-directives check out of iter_directives and do it only once, instead of multiple times per file
  • Improve the error message for unknown directives, as requested on Zulip

The change to error messages is fused with the extraction, since doing it independently would have been more awkward.


Before

Testing stage1 with compiletest suite=coverage mode=coverage-map (aarch64-apple-darwin)
errors encountered during EarlyProps parsing: /Users/stuart/Dev/rust/rust/tests/coverage/trivial.rs
2025-11-05T01:55:46.440012Z ERROR compiletest::directives: /Users/stuart/Dev/rust/rust/tests/coverage/trivial.rs:2: detected unknown compiletest test directive `add-core-stubs`

thread '<unnamed>' (36268582) panicked at src/tools/compiletest/src/directives.rs:72:13:
errors encountered during EarlyProps parsing
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

After

Testing stage1 with compiletest suite=coverage mode=coverage-map (aarch64-apple-darwin)

thread '<unnamed>' (36270772) panicked at src/tools/compiletest/src/lib.rs:876:9:
directives check failed:
ERROR: unknown compiletest directive `add-core-stubs` at /Users/stuart/Dev/rust/rust/tests/coverage/trivial.rs:2
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

This allows every check to be a single hashtable lookup instead of a linear
scan.
This also incorporates some requested improvements to the error message for
unknown directives.
@rustbot
Copy link
Collaborator

rustbot commented Nov 5, 2025

Some changes occurred in src/tools/compiletest

cc @jieyouxu

compiletest directives have been modified. Please add or update docs for the
new or modified directive in src/doc/rustc-dev-guide/.

@rustbot rustbot added A-compiletest Area: The compiletest test runner 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) labels Nov 5, 2025
@rustbot
Copy link
Collaborator

rustbot commented Nov 5, 2025

r? @Kobzol

rustbot has assigned @Kobzol.
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
Member Author

Zalathar commented Nov 5, 2025

This was originally going to wait until after #147955, but then Ralf and I both got annoyed by the error message, and improving the error message would be a lot harder without extracting the check first.

@jieyouxu jieyouxu self-assigned this Nov 5, 2025
Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Eventually, I would like us to move towards a late rejection, where if we fail to match the directive against any registered handle, then that constitutes an unknown directive. But in the meantime, making this "stop the bleeding" logic nicer is also a good improvement.

View changes since this review

@jieyouxu
Copy link
Member

jieyouxu commented Nov 9, 2025

We can land this first.
@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Nov 9, 2025

📌 Commit 795e906 has been approved by jieyouxu

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 9, 2025
bors added a commit that referenced this pull request Nov 9, 2025
Rollup of 4 pull requests

Successful merges:

 - #148248 (Constify `ControlFlow` methods without unstable bounds)
 - #148285 (Constify `ControlFlow` methods with unstable bounds)
 - #148510 (compiletest: Do the known-directives check only once, and improve its error message)
 - #148655 (Fix invalid macro tag generation for keywords which can be followed by values)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 192bb9c into rust-lang:master Nov 10, 2025
11 checks passed
@rustbot rustbot added this to the 1.93.0 milestone Nov 10, 2025
rust-timer added a commit that referenced this pull request Nov 10, 2025
Rollup merge of #148510 - Zalathar:known-directives, r=jieyouxu

compiletest: Do the known-directives check only once, and improve its error message

This PR is a combination of three changes:
- Store the list of known directives in a set, so that checking a directive name doesn't require a linear scan
- Extract the known-directives check out of `iter_directives` and do it only once, instead of multiple times per file
- Improve the error message for unknown directives, [as requested on Zulip](https://rust-lang.zulipchat.com/#narrow/channel/326414-t-infra.2Fbootstrap/topic/Compiletest.20directive.20parsing.20error/with/553598083)

The change to error messages is fused with the extraction, since doing it independently would have been more awkward.

---

## Before

```text
Testing stage1 with compiletest suite=coverage mode=coverage-map (aarch64-apple-darwin)
errors encountered during EarlyProps parsing: /Users/stuart/Dev/rust/rust/tests/coverage/trivial.rs
2025-11-05T01:55:46.440012Z ERROR compiletest::directives: /Users/stuart/Dev/rust/rust/tests/coverage/trivial.rs:2: detected unknown compiletest test directive `add-core-stubs`

thread '<unnamed>' (36268582) panicked at src/tools/compiletest/src/directives.rs:72:13:
errors encountered during EarlyProps parsing
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
```

## After

```text
Testing stage1 with compiletest suite=coverage mode=coverage-map (aarch64-apple-darwin)

thread '<unnamed>' (36270772) panicked at src/tools/compiletest/src/lib.rs:876:9:
directives check failed:
ERROR: unknown compiletest directive `add-core-stubs` at /Users/stuart/Dev/rust/rust/tests/coverage/trivial.rs:2
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
```
@Zalathar Zalathar deleted the known-directives branch November 10, 2025 02:53
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-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants