Skip to content

Conversation

Zalathar
Copy link
Contributor

This is an incremental step towards being able to clean up and centralize compiletest directive parsing.

My original plan was to add features to DirectiveLine, and then gradually migrate parsing code to use those features. However, that turned out to be impractical, because of how the existing directive parsers call each other. So instead this PR focuses on getting them to all take DirectiveLine instead of bare strings, to enable incremental work in the future.

Because this is part of an ongoing cleanup, I've prioritised clean diffs over nice code, because much of this code is going to be modified again when DirectiveLine is more capable.

r? jieyouxu

@rustbot
Copy link
Collaborator

rustbot commented Sep 30, 2025

Some changes occurred in src/tools/compiletest

cc @jieyouxu

@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 Sep 30, 2025
@rustbot
Copy link
Collaborator

rustbot commented Sep 30, 2025

jieyouxu is currently at their maximum review capacity.
They may take a while to respond.

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.

Comment on lines +108 to +109
/// Split off from the main `parse_name_value_directive`, so that improvements
/// to directive handling aren't held back by debuginfo test commands.
Copy link
Member

Choose a reason for hiding this comment

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

👍

Comment on lines -1120 to -1122
fn parse_negative_name_directive(&self, line: &str, directive: &str) -> bool {
line.starts_with("no-") && self.parse_name_directive(&line[3..], directive)
}
Copy link
Member

Choose a reason for hiding this comment

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

🤦

@jieyouxu
Copy link
Member

jieyouxu commented Oct 1, 2025

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Oct 1, 2025

📌 Commit e491056 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 Oct 1, 2025
bors added a commit that referenced this pull request Oct 1, 2025
Rollup of 11 pull requests

Successful merges:

 - #146918 (add regression test)
 - #146980 (simplify setup_constraining_predicates, and note it is potentially cubic)
 - #147170 (compiletest: Pass around `DirectiveLine` instead of bare strings)
 - #147180 (add tests)
 - #147188 (Remove usage of `compiletest-use-stage0-libtest` from CI)
 - #147189 (Replace `rustc_span::Span` with a stripped down version for librustdoc's highlighter)
 - #147199 (remove outdated comment in (inner) `InferCtxt`)
 - #147200 (Fix autodiff empty ret regression)
 - #147209 (Remove `no-remap-src-base` from tests)
 - #147213 (Fix broken STD build for ESP-IDF)
 - #147217 (Don't create a top-level `true` directory when running UI tests)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 3102f00 into rust-lang:master Oct 1, 2025
10 checks passed
@rustbot rustbot added this to the 1.92.0 milestone Oct 1, 2025
rust-timer added a commit that referenced this pull request Oct 1, 2025
Rollup merge of #147170 - Zalathar:directive, r=jieyouxu

compiletest: Pass around `DirectiveLine` instead of bare strings

This is an incremental step towards being able to clean up and centralize compiletest directive parsing.

My original plan was to add features to `DirectiveLine`, and then gradually migrate parsing code to use those features. However, that turned out to be impractical, because of how the existing directive parsers call each other. So instead this PR focuses on getting them to all take `DirectiveLine` instead of bare strings, to enable incremental work in the future.

Because this is part of an ongoing cleanup, I've prioritised clean diffs over nice code, because much of this code is going to be modified again when `DirectiveLine` is more capable.

r? jieyouxu
@Zalathar Zalathar deleted the directive branch October 1, 2025 21:24
Zalathar added a commit to Zalathar/rust that referenced this pull request Oct 4, 2025
compiletest: Make `DirectiveLine` responsible for name/value splitting

- Follow-up to rust-lang#147170.

---

Now that all of the directive-parsing functions have access to a `DirectiveLine`, we can move all of the ad-hoc name/value splitting code into `DirectiveLine` itself, making directive parsing simpler and more consistent.

The first commit is just moving code into a submodule, so the actual changes can be seen in the subsequent commits.

r? jieyouxu
Zalathar added a commit to Zalathar/rust that referenced this pull request Oct 4, 2025
compiletest: Make `DirectiveLine` responsible for name/value splitting

- Follow-up to rust-lang#147170.

---

Now that all of the directive-parsing functions have access to a `DirectiveLine`, we can move all of the ad-hoc name/value splitting code into `DirectiveLine` itself, making directive parsing simpler and more consistent.

The first commit is just moving code into a submodule, so the actual changes can be seen in the subsequent commits.

r? jieyouxu
Zalathar added a commit to Zalathar/rust that referenced this pull request Oct 4, 2025
compiletest: Make `DirectiveLine` responsible for name/value splitting

- Follow-up to rust-lang#147170.

---

Now that all of the directive-parsing functions have access to a `DirectiveLine`, we can move all of the ad-hoc name/value splitting code into `DirectiveLine` itself, making directive parsing simpler and more consistent.

The first commit is just moving code into a submodule, so the actual changes can be seen in the subsequent commits.

r? jieyouxu
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Oct 4, 2025
compiletest: Make `DirectiveLine` responsible for name/value splitting

- Follow-up to rust-lang#147170.

---

Now that all of the directive-parsing functions have access to a `DirectiveLine`, we can move all of the ad-hoc name/value splitting code into `DirectiveLine` itself, making directive parsing simpler and more consistent.

The first commit is just moving code into a submodule, so the actual changes can be seen in the subsequent commits.

r? jieyouxu
Zalathar added a commit to Zalathar/rust that referenced this pull request Oct 5, 2025
compiletest: Make `DirectiveLine` responsible for name/value splitting

- Follow-up to rust-lang#147170.

---

Now that all of the directive-parsing functions have access to a `DirectiveLine`, we can move all of the ad-hoc name/value splitting code into `DirectiveLine` itself, making directive parsing simpler and more consistent.

The first commit is just moving code into a submodule, so the actual changes can be seen in the subsequent commits.

r? jieyouxu
Zalathar added a commit to Zalathar/rust that referenced this pull request Oct 5, 2025
compiletest: Make `DirectiveLine` responsible for name/value splitting

- Follow-up to rust-lang#147170.

---

Now that all of the directive-parsing functions have access to a `DirectiveLine`, we can move all of the ad-hoc name/value splitting code into `DirectiveLine` itself, making directive parsing simpler and more consistent.

The first commit is just moving code into a submodule, so the actual changes can be seen in the subsequent commits.

r? jieyouxu
Zalathar added a commit to Zalathar/rust that referenced this pull request Oct 5, 2025
compiletest: Make `DirectiveLine` responsible for name/value splitting

- Follow-up to rust-lang#147170.

---

Now that all of the directive-parsing functions have access to a `DirectiveLine`, we can move all of the ad-hoc name/value splitting code into `DirectiveLine` itself, making directive parsing simpler and more consistent.

The first commit is just moving code into a submodule, so the actual changes can be seen in the subsequent commits.

r? jieyouxu
Zalathar added a commit to Zalathar/rust that referenced this pull request Oct 5, 2025
compiletest: Make `DirectiveLine` responsible for name/value splitting

- Follow-up to rust-lang#147170.

---

Now that all of the directive-parsing functions have access to a `DirectiveLine`, we can move all of the ad-hoc name/value splitting code into `DirectiveLine` itself, making directive parsing simpler and more consistent.

The first commit is just moving code into a submodule, so the actual changes can be seen in the subsequent commits.

r? jieyouxu
Zalathar added a commit to Zalathar/rust that referenced this pull request Oct 5, 2025
compiletest: Make `DirectiveLine` responsible for name/value splitting

- Follow-up to rust-lang#147170.

---

Now that all of the directive-parsing functions have access to a `DirectiveLine`, we can move all of the ad-hoc name/value splitting code into `DirectiveLine` itself, making directive parsing simpler and more consistent.

The first commit is just moving code into a submodule, so the actual changes can be seen in the subsequent commits.

r? jieyouxu
rust-timer added a commit that referenced this pull request Oct 5, 2025
Rollup merge of #147288 - Zalathar:directive, r=jieyouxu

compiletest: Make `DirectiveLine` responsible for name/value splitting

- Follow-up to #147170.

---

Now that all of the directive-parsing functions have access to a `DirectiveLine`, we can move all of the ad-hoc name/value splitting code into `DirectiveLine` itself, making directive parsing simpler and more consistent.

The first commit is just moving code into a submodule, so the actual changes can be seen in the subsequent commits.

r? jieyouxu
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.

4 participants