compiletest: Isolate public APIs and minimize public surface area #147506

Zalathar · 2025-10-09T03:51:30Z

As part of my ongoing efforts to improve directive parsing, I would like to be able to make internal changes without worrying about whether they're going to break the rustdoc-gui-test tool. That tool currently uses compiletest as a dependency to help with directive parsing.

This PR therefore isolates all of compiletest's public APIs into two dedicated modules, one used by rustdoc-gui-test, and one used by the compiletest binary in compiletest/src/bin/main.rs.

All other modules (and crate-root items) are then made non-pub to achieve the API isolation. Doing so reveals some unused items, which have been removed.

(To reduce the amount of immediate textual churn, this PR does not comprehensively replace pub with pub(crate) throughout the whole crate; that could be done in a follow-up PR.)

Ideally, rustdoc-gui-test would not depend on compiletest at all, but properly fixing that is out of scope for this PR.

src/tools/rustdoc-gui-test uses compiletest directives + TestProp handling internals in a questionable way #143827

r? jieyouxu

All APIs used from outside the compiletest library crate have been isolated to the `cli` and `rustdoc_gui_test` modules, so no other items need to be publicly exported.

rustbot · 2025-10-09T03:51:33Z

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

Some changes occurred in src/tools/compiletest

cc @jieyouxu

rustbot · 2025-10-09T03:51:35Z

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

jieyouxu

Thanks, most of these are very nice cleanups. The enzyme change seems unexpected tho, could you double-check?

View changes since this review

jieyouxu · 2025-10-09T05:24:51Z

src/tools/compiletest/src/rustdoc_gui_test.rs

+/// Subset of compiletest directive values that are actually used by
+/// rustdoc-gui-test.
+#[derive(Debug)]
+pub struct RustdocGuiTestProps {
+    pub compile_flags: Vec<String>,
+    pub run_flags: Vec<String>,
+}


jieyouxu · 2025-10-09T05:26:26Z

src/bootstrap/src/core/build_steps/test.rs

-        if let Some(ref npm) = builder.config.npm {
-            cmd.arg("--npm").arg(npm);
-        }


Remark: oh nice, I didn't realize this was rustdoc-gui-test-only.

src/tools/compiletest/src/directives.rs

jieyouxu · 2025-10-09T05:29:52Z

@rustbot author

rustbot · 2025-10-09T05:29:57Z

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

Zalathar · 2025-10-09T05:54:04Z

I'm pretty confident that TestProps::has_enzyme is actually unused, and that it's Config::has_enzyme (which still exists) that is responsible for //@ needs-enzyme and //@ ignore-enzyme, so those should continue to work.

@rustbot ready

jieyouxu · 2025-10-09T06:05:17Z

Thanks for double-checking!
@bors r+ rollup

bors · 2025-10-09T06:05:20Z

📌 Commit ce4699d has been approved by jieyouxu

It is now in the queue for this repository.

compiletest: Isolate public APIs and minimize public surface area As part of my ongoing efforts to improve directive parsing, I would like to be able to make internal changes without worrying about whether they're going to break the rustdoc-gui-test tool. That tool currently uses compiletest as a dependency to help with directive parsing. This PR therefore isolates all of compiletest's public APIs into two dedicated modules, one used by rustdoc-gui-test, and one used by the compiletest binary in `compiletest/src/bin/main.rs`. All other modules (and crate-root items) are then made non-`pub` to achieve the API isolation. Doing so reveals some unused items, which have been removed. (To reduce the amount of immediate textual churn, this PR does not comprehensively replace `pub` with `pub(crate)` throughout the whole crate; that could be done in a follow-up PR.) --- Ideally, rustdoc-gui-test would not depend on compiletest at all, but properly fixing that is out of scope for this PR. - rust-lang#143827 r? jieyouxu

Rollup of 7 pull requests Successful merges: - #146568 (Port the implemention of SIMD intrinsics from Miri to const-eval) - #147373 (give a better example why `std` modules named like primitives are needed) - #147419 (bootstrap: add `Builder::rustc_cmd` that includes the lib path) - #147457 (specialize slice::fill to use memset when possible) - #147468 (Implement fs api set_times and set_times_nofollow) - #147489 (Prefer to use repeat_n over repeat().take()) - #147506 (compiletest: Isolate public APIs and minimize public surface area) r? `@ghost` `@rustbot` modify labels: rollup

Rollup of 12 pull requests Successful merges: - #146568 (Port the implemention of SIMD intrinsics from Miri to const-eval) - #147373 (give a better example why `std` modules named like primitives are needed) - #147419 (bootstrap: add `Builder::rustc_cmd` that includes the lib path) - #147420 (Add diagnostic items for `pub mod consts` of FP types) - #147457 (specialize slice::fill to use memset when possible) - #147467 (Fix double warnings on `#[no_mangle]`) - #147470 (Clarify how to remediate the panic_immediate_abort error) - #147480 (Do not invalidate CFG caches in CtfeLimit.) - #147481 (format: some small cleanup) - #147488 (refactor: Remove `LLVMRustInsertPrivateGlobal` and `define_private_global`) - #147489 (Prefer to use repeat_n over repeat().take()) - #147506 (compiletest: Isolate public APIs and minimize public surface area) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of #147506 - Zalathar:isolate, r=jieyouxu compiletest: Isolate public APIs and minimize public surface area As part of my ongoing efforts to improve directive parsing, I would like to be able to make internal changes without worrying about whether they're going to break the rustdoc-gui-test tool. That tool currently uses compiletest as a dependency to help with directive parsing. This PR therefore isolates all of compiletest's public APIs into two dedicated modules, one used by rustdoc-gui-test, and one used by the compiletest binary in `compiletest/src/bin/main.rs`. All other modules (and crate-root items) are then made non-`pub` to achieve the API isolation. Doing so reveals some unused items, which have been removed. (To reduce the amount of immediate textual churn, this PR does not comprehensively replace `pub` with `pub(crate)` throughout the whole crate; that could be done in a follow-up PR.) --- Ideally, rustdoc-gui-test would not depend on compiletest at all, but properly fixing that is out of scope for this PR. - #143827 r? jieyouxu

Zalathar added 3 commits October 9, 2025 14:34

compiletest: Isolate APIs used by rustdoc-gui-test

8f08156

compiletest: Isolate APIs used by bin/main.rs

b4f64fd

compiletest: Make all other modules non-public

ce4699d

All APIs used from outside the compiletest library crate have been isolated to the `cli` and `rustdoc_gui_test` modules, so no other items need to be publicly exported.

rustbot assigned jieyouxu Oct 9, 2025

jieyouxu reviewed Oct 9, 2025

View reviewed changes

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 9, 2025

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

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 9, 2025

Zalathar mentioned this pull request Oct 9, 2025

Rollup of 7 pull requests #147510

Closed

Zalathar mentioned this pull request Oct 9, 2025

Rollup of 12 pull requests #147512

Merged

bors merged commit 76d4f37 into rust-lang:master Oct 9, 2025
10 checks passed

rustbot added this to the 1.92.0 milestone Oct 9, 2025

Zalathar deleted the isolate branch October 9, 2025 11:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

compiletest: Isolate public APIs and minimize public surface area #147506

compiletest: Isolate public APIs and minimize public surface area #147506

Uh oh!

Zalathar commented Oct 9, 2025

Uh oh!

rustbot commented Oct 9, 2025

Uh oh!

rustbot commented Oct 9, 2025

Uh oh!

jieyouxu left a comment •

edited by rustbot

Loading

Uh oh!

jieyouxu Oct 9, 2025

Uh oh!

jieyouxu Oct 9, 2025

Uh oh!

Uh oh!

jieyouxu commented Oct 9, 2025

Uh oh!

rustbot commented Oct 9, 2025

Uh oh!

Zalathar commented Oct 9, 2025

Uh oh!

jieyouxu commented Oct 9, 2025

Uh oh!

bors commented Oct 9, 2025

Uh oh!

Uh oh!

Uh oh!

compiletest: Isolate public APIs and minimize public surface area #147506

compiletest: Isolate public APIs and minimize public surface area #147506

Uh oh!

Conversation

Zalathar commented Oct 9, 2025

Uh oh!

rustbot commented Oct 9, 2025

Uh oh!

rustbot commented Oct 9, 2025

Uh oh!

jieyouxu left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jieyouxu Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

jieyouxu Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jieyouxu commented Oct 9, 2025

Uh oh!

rustbot commented Oct 9, 2025

Uh oh!

Zalathar commented Oct 9, 2025

Uh oh!

jieyouxu commented Oct 9, 2025

Uh oh!

bors commented Oct 9, 2025

Uh oh!

Uh oh!

Uh oh!

jieyouxu left a comment •

edited by rustbot

Loading