Skip to content

Commit

Permalink
bootstrap: Further centralize target defaulting logic.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Ericson2314 committed Feb 25, 2020
1 parent 03ca0e2 commit 4f15867
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 7 deletions.
16 changes: 13 additions & 3 deletions src/bootstrap/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,15 @@ pub struct Target {
pub no_std: bool,
}

impl Target {
pub fn from_triple(triple: &str) -> Self {
let mut target: Self = Default::default();
if triple.contains("-none-") || triple.contains("nvptx") {
target.no_std = true;
}
target
}
}
/// Structure of the `config.toml` file that configuration is read from.
///
/// This structure uses `Decodable` to automatically decode a TOML configuration
Expand Down Expand Up @@ -596,7 +605,7 @@ impl Config {

if let Some(ref t) = toml.target {
for (triple, cfg) in t {
let mut target = Target::default();
let mut target = Target::from_triple(triple);

if let Some(ref s) = cfg.llvm_config {
target.llvm_config = Some(config.src.join(s));
Expand All @@ -607,6 +616,9 @@ impl Config {
if let Some(ref s) = cfg.android_ndk {
target.ndk = Some(config.src.join(s));
}
if let Some(s) = cfg.no_std {
target.no_std = s;
}
target.cc = cfg.cc.clone().map(PathBuf::from);
target.cxx = cfg.cxx.clone().map(PathBuf::from);
target.ar = cfg.ar.clone().map(PathBuf::from);
Expand All @@ -616,8 +628,6 @@ 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"));

config.target_config.insert(INTERNER.intern_string(triple.clone()), target);
}
Expand Down
7 changes: 3 additions & 4 deletions src/bootstrap/sanity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use std::process::Command;

use build_helper::{output, t};

use crate::config::Target;
use crate::Build;

struct Finder {
Expand Down Expand Up @@ -192,11 +193,9 @@ pub fn check(build: &mut Build) {
panic!("the iOS target is only supported on macOS");
}

if target.contains("-none-") || target.contains("nvptx") {
if build.no_std(*target).is_none() {
build.config.target_config.entry(target.clone()).or_default();
}
build.config.target_config.entry(target.clone()).or_insert(Target::from_triple(target));

if target.contains("-none-") || target.contains("nvptx") {
if build.no_std(*target) == Some(false) {
panic!("All the *-none-* and nvptx* targets are no-std targets")
}
Expand Down

0 comments on commit 4f15867

Please sign in to comment.