Allow getting no_std from the config file
#69381
Conversation
|
(rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
Currently, it is only set correctly in the sanity checking implicit default fallback code. Having a config file at all will for force `no_std = false`.
Ericson2314
force-pushed
the
no-std-from-config
branch
from
75dde0c
to
03ca0e2
Compare
src/bootstrap/config.rs
Outdated
| @@ -615,6 +616,8 @@ impl Config { | |||
| target.musl_root = cfg.musl_root.clone().map(PathBuf::from); | |||
| target.wasi_root = cfg.wasi_root.clone().map(PathBuf::from); | |||
| target.qemu_rootfs = cfg.qemu_rootfs.clone().map(PathBuf::from); | |||
| target.no_std = | |||
| cfg.no_std.unwrap_or(triple.contains("-none-") || triple.contains("nvptx")); | |||
There was a problem hiding this comment.
Hm, is -none- perhaps a bit too broad? I guess we presumably don't ship any platforms with that triple today that do have std compiled?
There was a problem hiding this comment.
The logic is taken from if target.contains("-none-") || target.contains("nvptx") {. Some later refactor should just ensure there are Targets for all platforms specified by name, rather than config, so we can deduplicate this.
src/bootstrap/sanity.rs
Outdated
| @@ -194,9 +194,7 @@ pub fn check(build: &mut Build) { | |||
|
|
|||
| if target.contains("-none-") || target.contains("nvptx") { | |||
| if build.no_std(*target).is_none() { | |||
There was a problem hiding this comment.
This function looks potentially relevant; is there a reason it's not receiving updates or so?
There was a problem hiding this comment.
Ah, I should replace the calls to Default::default with a new method that defaults based on the triple. Then this will work as I intended: the defaulting logic happens in one place. and we simply ensure that the target exists here so that the triple-specific defaults rather than triple-agnostic defaults are used.
|
Okay, I think the new logic is better, thanks! @bors r+ |
|
📌 Commit 03ca0e2 has been approved by |
bors
added
S-waiting-on-bors
|
Wait I should have called out the issue in #69381 (comment) more strongly. This as-is will break just doing |
|
@bors r- |
bors
added
S-waiting-on-author
|
oh cool, that does work for me as author. |
Background: targets can be specied with or without config files; unneccessarily differences in the logic between those cases has caused a) the bug I tried to fix in the previous commit, b) the bug I introduced in the previous commit. The solution is to make the code paths the same as much as possible. 1. Targets are now not created from the `default` method. (I would both remove the impl if this was a public library, but just wrap it for convience becaues it's not.) Instead, there is a `from_triple` method which does the defaulting. 2. Besides the sanity checking, use the new method in the code reading config files. Now `no_std` is overriden iff set explicitly just like the other fields which are optional in the TOML AST type. 3. In sanity checking, just populate the map for all targets no matter what. That way do don't duplicate logic trying to be clever and remember which targets have "non standard" overrides. Sanity checking is back to just sanity checking, and out of the game of trying to default too.
|
@bors r+ |
|
@Ericson2314: 🔑 Insufficient privileges: Not in reviewers |
|
@bors r+ |
|
📌 Commit 4f15867 has been approved by |
bors
added
S-waiting-on-bors
Rollup of 5 pull requests Successful merges: - #68712 (Add methods to 'leak' RefCell borrows as references with the lifetime of the original reference) - #69209 (Miscellaneous cleanup to formatting) - #69381 (Allow getting `no_std` from the config file) - #69434 (rustc_metadata: Use binary search from standard library) - #69447 (Minor refactoring of statement parsing) Failed merges: r? @ghost
Note: The removed patch has been merged in rust-lang/rust#69381
Currently, it is only set correctly in the sanity checking implicit
default fallback code. Having a config file at all will for force
no_std = false.