use `cargo rustc` for macro expansion so -Zbuild-std can be supported by skrap · Pull Request #5214 · rust-lang/libc

skrap · 2026-06-24T17:03:09Z

Description

Address #5204 by invoking macro expansion through cargo rustc instead of rustc directly. This allows the use of the -Zbuild-std flag, enabling targets without a prebuilt std (e.g. tier 3 targets) to build the libc tests.

Checklist

Relevant tests in libc-test/semver have been updated
No placeholder or unstable values like *LAST or *MAX are
included (see #3131)
Tested locally (cd libc-test && cargo test --target mytarget);
especially relevant for platforms that may not be checked in CI

tgross35 · 2026-06-24T19:09:34Z

Cc @mbyx for ctest

tgross35 · 2026-06-24T20:39:25Z

-        .arg("--edition")
-        .arg(EDITION) // By default, -Zunpretty=expanded uses 2015 edition.
-        .arg(canonicalize(crate_path)?);
+        .env("RUSTFLAGS", "") // ignore RUSTFLAGS so we don't get -Dwarnings


Add a TestGenerator::expansion_rustflags(&str) and set RUSTFLAGS only if that has been called, otherwise just let it inherit. Since there might be some global options needed to build.

View changes since the review

We were not building with inherited RUSTFLAGS before this change, as RUSTFLAGS are a cargo-ism, and we were not using cargo before, but rather using rustc directly.

I think I will be a bit more tactical and just add a -Awarnings at the end of RUSTFLAGS so we don't error out over warnings if -Dwarnings is present. No new API required, and should be compatible with existing clients.

Fair point about compatibility! We probably shouldn't suppress warnings though, there could be some papercuts.

I think it might be best to leave exact env up to the user, since sometimes build scripts pick up other env that you may want to control. So perhaps:

impl TestGenerator { fn expansion_env(&mut self, k: impl AsRef<OsStr>, v: impl AsRef<OsStr>) -> &mut Self; fn expansion_env_clear(&mut self) -> &mut Self; }

Then call cmd.env_clear().env(expansion_env) on the Command.

But to what end? This is adding API surface which no crate user has asked for, and it might or might not be the correct thing. The previous code would emit warnings into /dev/null and then move on, and I think we should retain that behavior until some motivated person comes along to ask for it to change.

I would prefer we stick to the goal of this PR which is to enable tier 3 folks like me to build libc, and this API is beyond that goal.

If just calling .env_clear() works then I'm fine with that and no new API. But I don't want to hardcode any RUSTFLAGS - if those need to be adjusted then we should go with the suggested API.

We should give users a chance to catch anything warnings that happen during expansion in case it's due to the config they're providing. Macro expansion can be env-dependent as well.

rustbot · 2026-06-24T20:44:15Z

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

skrap · 2026-07-01T03:17:19Z

@rustbot review

I think I have addressed the feedback, though in exploring the changes I sometimes made some different choices. I've left comments in the conversations where appropriate.

tgross35 · 2026-07-01T07:35:50Z

+    /// Configures any extra arguments which to be passed to cargo
+    /// during macro expansion.  This can be used, for example, to pass
+    /// -Zbuild-std if required.
+    pub fn set_macro_expansion_cargo_args(&mut self, args: Vec<String>) {
+        self.macro_expansion_cargo_args = args;
+    }


To keep things a bit more composable so you can set args one at a time, replace this with fn expansion_cargo_arg(&mut self, &str) and fn clear_expansion_cargo_args(&mut self).

Also these methods should return &mut Self

View changes since the review

Curious why we would need a clear(). Why would a crate user add arguments only to remove them? Again I find myself concerned about increasing the API surface without a clear use case.

Any kind of pattern that sets defaults then clears them, so you can just use store the args in the TestGenerator rather than tracking them separately. Not likely to be used a lot: it's just the sort of thing that's pretty easy to add now, but more likely to be worked around than added in the future.

Rather than clear, something like expansion_cargo_args_mut(&mut self) -> &mut Vec<String> would be fine too for more flexibility.

tgross35 · 2026-07-01T08:53:13Z

-        .arg("--edition")
-        .arg(EDITION) // By default, -Zunpretty=expanded uses 2015 edition.
-        .arg(canonicalize(crate_path)?);
+        .env("RUSTFLAGS", "") // ignore RUSTFLAGS so we don't get -Dwarnings


Fair point about compatibility! We probably shouldn't suppress warnings though, there could be some papercuts.

I think it might be best to leave exact env up to the user, since sometimes build scripts pick up other env that you may want to control. So perhaps:

impl TestGenerator { fn expansion_env(&mut self, k: impl AsRef<OsStr>, v: impl AsRef<OsStr>) -> &mut Self; fn expansion_env_clear(&mut self) -> &mut Self; }

Then call cmd.env_clear().env(expansion_env) on the Command.

rustbot · 2026-07-01T15:46:40Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

skrap · 2026-07-01T15:56:43Z

@rustbot review

Further review feedback. I'm a bit shy about adding more API than necessary for the stated purpose of the PR. If we have a clear use case for the API proposed, I think that should be proposed in a separate PR.

tgross35 · 2026-07-02T00:44:33Z

+    /// Configures the crate name which should be used during macro expansion.
+    /// If the tested crate uses `#![crate_name = "..."]`, this must be called with the
+    /// same name. Otherwise, there will be an error about `--crate-name` not
+    /// matching.


Suggested change

/// Configures the crate name which should be used during macro expansion.

/// If the tested crate uses `#![crate_name = "..."]`, this must be called with the

/// same name. Otherwise, there will be an error about `--crate-name` not

/// matching.

/// Configures the crate name which should be used during macro expansion.

///

/// If the tested crate uses `#![crate_name = "..."]`, this must be called with the

/// same name. Otherwise, there will be an error about `--crate-name` not

/// matching.

so rustdoc doesn't mush into the summary

View changes since the review

tgross35 · 2026-07-02T00:52:42Z

+    /// Configures any extra arguments which to be passed to cargo
+    /// during macro expansion.  This can be used, for example, to pass
+    /// -Zbuild-std if required.
+    pub fn set_macro_expansion_cargo_args(&mut self, args: Vec<String>) {
+        self.macro_expansion_cargo_args = args;
+    }


Any kind of pattern that sets defaults then clears them, so you can just use store the args in the TestGenerator rather than tracking them separately. Not likely to be used a lot: it's just the sort of thing that's pretty easy to add now, but more likely to be worked around than added in the future.

Rather than clear, something like expansion_cargo_args_mut(&mut self) -> &mut Vec<String> would be fine too for more flexibility.

tgross35 · 2026-07-02T01:08:40Z

-        .arg("--edition")
-        .arg(EDITION) // By default, -Zunpretty=expanded uses 2015 edition.
-        .arg(canonicalize(crate_path)?);
+        .env("RUSTFLAGS", "") // ignore RUSTFLAGS so we don't get -Dwarnings


If just calling .env_clear() works then I'm fine with that and no new API. But I don't want to hardcode any RUSTFLAGS - if those need to be adjusted then we should go with the suggested API.

We should give users a chance to catch anything warnings that happen during expansion in case it's due to the config they're providing. Macro expansion can be env-dependent as well.

tgross35

Just updating the status here.

Also, regarding the general concern about API surface area: don't worry about it :) as long as it's somewhat sane, for ctest I tend to err on the side of adding small things we might need while related code is already being modified, rather than leaving a gap that might need ctest-specific changes in the future. There might be somebody who appreciates it even if libc doesn't have an immediate use.

View changes since this review

rustbot added ctest Issues relating to the ctest crate S-waiting-on-review labels Jun 24, 2026

skrap force-pushed the bugfix/allow-zbuild-std-macro-expansion branch 4 times, most recently from 0689c20 to b5da051 Compare June 24, 2026 18:34

tgross35 reviewed Jun 24, 2026

View reviewed changes

Comment thread ctest/src/macro_expansion.rs Outdated

tgross35 added the stable-unneeded This PR is applied to main but already exists on libc-0.2 label Jun 24, 2026

skrap force-pushed the bugfix/allow-zbuild-std-macro-expansion branch from b5da051 to 1100563 Compare June 24, 2026 19:18

tgross35 requested changes Jun 24, 2026

View reviewed changes

rustbot added S-waiting-on-author and removed S-waiting-on-review labels Jun 24, 2026

skrap force-pushed the bugfix/allow-zbuild-std-macro-expansion branch from bef315b to af836b6 Compare July 1, 2026 03:16

This comment has been minimized.

Sign in to view

rustbot added S-waiting-on-review and removed S-waiting-on-author labels Jul 1, 2026

skrap force-pushed the bugfix/allow-zbuild-std-macro-expansion branch 2 times, most recently from 2ef0daf to 3294988 Compare July 1, 2026 03:18

tgross35 mentioned this pull request Jul 1, 2026

ctest: Allow crate_path in TestGenerator::generate_files to be either a Cargo.toml or a .rs #5238

Open

tgross35 reviewed Jul 1, 2026

View reviewed changes

skrap force-pushed the bugfix/allow-zbuild-std-macro-expansion branch from 3294988 to 9dd5dd1 Compare July 1, 2026 13:12

This comment has been minimized.

Sign in to view

skrap force-pushed the bugfix/allow-zbuild-std-macro-expansion branch from 9dd5dd1 to ba0cdaa Compare July 1, 2026 15:42

Jonah Petri added 3 commits July 1, 2026 11:46

use cargo rustc for macro expansion so -Zbuild-std can be supported

5e86813

incorporate review feedback

9acadb3

address feedback round 2

f752fb9

skrap force-pushed the bugfix/allow-zbuild-std-macro-expansion branch from ba0cdaa to f752fb9 Compare July 1, 2026 15:46

tgross35 reviewed Jul 2, 2026

View reviewed changes

tgross35 requested changes Jul 4, 2026

View reviewed changes

rustbot added S-waiting-on-author and removed S-waiting-on-review labels Jul 4, 2026

Uh oh!

Conversation

skrap commented Jun 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

Uh oh!

tgross35 commented Jun 24, 2026

Uh oh!

Uh oh!

Uh oh!

tgross35 Jun 24, 2026 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rustbot commented Jun 24, 2026

Uh oh!

This comment has been minimized.

skrap commented Jul 1, 2026

Uh oh!

tgross35 Jul 1, 2026 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

rustbot commented Jul 1, 2026

Uh oh!

skrap commented Jul 1, 2026

Uh oh!

tgross35 Jul 2, 2026 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tgross35 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

skrap commented Jun 24, 2026 •

edited

Loading

tgross35 Jun 24, 2026 •

edited by rustbot

Loading

tgross35 Jul 1, 2026 •

edited by rustbot

Loading

tgross35 Jul 2, 2026 •

edited by rustbot

Loading

tgross35 left a comment •

edited by rustbot

Loading