Emit warnings for unused fields in custom targets.

This commit is contained in:
Adam Bratschi-Kaye 2021-05-27 10:21:53 +02:00
parent 0ef2b4a29b
commit 88b01f1178
7 changed files with 235 additions and 91 deletions

View File

@ -1114,6 +1114,15 @@ pub fn find(&self, key: &str) -> Option<&Json> {
}
}
/// If the Json value is an Object, deletes the value associated with the
/// provided key from the Object and returns it. Otherwise, returns None.
pub fn remove_key(&mut self, key: &str) -> Option<Json> {
match *self {
Json::Object(ref mut map) => map.remove(key),
_ => None,
}
}
/// Attempts to get a nested Json Object for each key in `keys`.
/// If any key is found not to exist, `find_path` will return `None`.
/// Otherwise, it will return the Json value associated with the final key.

View File

@ -12,7 +12,7 @@
use rustc_data_structures::impl_stable_hash_via_hash;
use rustc_target::abi::{Align, TargetDataLayout};
use rustc_target::spec::{SplitDebuginfo, Target, TargetTriple};
use rustc_target::spec::{SplitDebuginfo, Target, TargetTriple, TargetWarnings};
use rustc_serialize::json;
@ -899,9 +899,11 @@ pub(super) fn build_target_config(
target_override: Option<Target>,
sysroot: &PathBuf,
) -> Target {
let target_result =
target_override.map_or_else(|| Target::search(&opts.target_triple, sysroot), Ok);
let target = target_result.unwrap_or_else(|e| {
let target_result = target_override.map_or_else(
|| Target::search(&opts.target_triple, sysroot),
|t| Ok((t, TargetWarnings::empty())),
);
let (target, target_warnings) = target_result.unwrap_or_else(|e| {
early_error(
opts.error_format,
&format!(
@ -911,6 +913,9 @@ pub(super) fn build_target_config(
),
)
});
for warning in target_warnings.warning_messages() {
early_warn(opts.error_format, &warning)
}
if !matches!(target.pointer_width, 16 | 32 | 64) {
early_error(

View File

@ -1284,9 +1284,12 @@ pub fn build_session(
let target_cfg = config::build_target_config(&sopts, target_override, &sysroot);
let host_triple = TargetTriple::from_triple(config::host_triple());
let host = Target::search(&host_triple, &sysroot).unwrap_or_else(|e| {
let (host, target_warnings) = Target::search(&host_triple, &sysroot).unwrap_or_else(|e| {
early_error(sopts.error_format, &format!("Error loading host specification: {}", e))
});
for warning in target_warnings.warning_messages() {
early_warn(sopts.error_format, &warning)
}
let loader = file_loader.unwrap_or_else(|| Box::new(RealFileLoader));
let hash_kind = sopts.debugging_opts.src_hash_algorithm.unwrap_or_else(|| {

View File

@ -27,6 +27,9 @@
pub mod asm;
pub mod spec;
#[cfg(test)]
mod tests;
/// Requirements for a `StableHashingContext` to be used in this crate.
/// This is a hack to allow using the `HashStable_Generic` derive macro
/// instead of implementing everything in `rustc_middle`.

View File

@ -905,6 +905,38 @@ fn $module() {
("bpfel-unknown-none", bpfel_unknown_none),
}
/// Warnings encountered when parsing the target `json`.
///
/// Includes fields that weren't recognized and fields that don't have the expected type.
#[derive(Debug, PartialEq)]
pub struct TargetWarnings {
unused_fields: Vec<String>,
incorrect_type: Vec<String>,
}
impl TargetWarnings {
pub fn empty() -> Self {
Self { unused_fields: Vec::new(), incorrect_type: Vec::new() }
}
pub fn warning_messages(&self) -> Vec<String> {
let mut warnings = vec![];
if !self.unused_fields.is_empty() {
warnings.push(format!(
"target json file contains unused fields: {}",
self.unused_fields.join(", ")
));
}
if !self.incorrect_type.is_empty() {
warnings.push(format!(
"target json file contains fields whose value doesn't have the correct json type: {}",
self.incorrect_type.join(", ")
));
}
warnings
}
}
/// Everything `rustc` knows about how to compile for a specific target.
///
/// Every field here must be specified, and has no default value.
@ -1447,7 +1479,7 @@ pub fn is_abi_supported(&self, abi: Abi) -> bool {
}
/// Loads a target descriptor from a JSON object.
pub fn from_json(obj: Json) -> Result<Target, String> {
pub fn from_json(mut obj: Json) -> Result<(Target, TargetWarnings), String> {
// While ugly, this code must remain this way to retain
// compatibility with existing JSON fields and the internal
// expected naming of the Target and TargetOptions structs.
@ -1455,10 +1487,9 @@ pub fn from_json(obj: Json) -> Result<Target, String> {
// are round-tripped through this code to catch cases where
// the JSON parser is not updated to match the structs.
let get_req_field = |name: &str| {
obj.find(name)
.and_then(Json::as_string)
.map(str::to_string)
let mut get_req_field = |name: &str| {
obj.remove_key(name)
.and_then(|j| Json::as_string(&j).map(str::to_string))
.ok_or_else(|| format!("Field {} in target specification is required", name))
};
@ -1472,28 +1503,30 @@ pub fn from_json(obj: Json) -> Result<Target, String> {
options: Default::default(),
};
let mut incorrect_type = vec![];
macro_rules! key {
($key_name:ident) => ( {
let name = (stringify!($key_name)).replace("_", "-");
if let Some(s) = obj.find(&name).and_then(Json::as_string) {
base.$key_name = s.to_string();
if let Some(s) = obj.remove_key(&name).and_then(|j| Json::as_string(&j).map(str::to_string)) {
base.$key_name = s;
}
} );
($key_name:ident = $json_name:expr) => ( {
let name = $json_name;
if let Some(s) = obj.find(&name).and_then(Json::as_string) {
base.$key_name = s.to_string();
if let Some(s) = obj.remove_key(&name).and_then(|j| Json::as_string(&j).map(str::to_string)) {
base.$key_name = s;
}
} );
($key_name:ident, bool) => ( {
let name = (stringify!($key_name)).replace("_", "-");
if let Some(s) = obj.find(&name).and_then(Json::as_boolean) {
if let Some(s) = obj.remove_key(&name).and_then(|j| Json::as_boolean(&j)) {
base.$key_name = s;
}
} );
($key_name:ident, Option<u32>) => ( {
let name = (stringify!($key_name)).replace("_", "-");
if let Some(s) = obj.find(&name).and_then(Json::as_u64) {
if let Some(s) = obj.remove_key(&name).and_then(|j| Json::as_u64(&j)) {
if s < 1 || s > 5 {
return Err("Not a valid DWARF version number".to_string());
}
@ -1502,13 +1535,13 @@ macro_rules! key {
} );
($key_name:ident, Option<u64>) => ( {
let name = (stringify!($key_name)).replace("_", "-");
if let Some(s) = obj.find(&name).and_then(Json::as_u64) {
if let Some(s) = obj.remove_key(&name).and_then(|j| Json::as_u64(&j)) {
base.$key_name = Some(s);
}
} );
($key_name:ident, MergeFunctions) => ( {
let name = (stringify!($key_name)).replace("_", "-");
obj.find(&name[..]).and_then(|o| o.as_string().and_then(|s| {
obj.remove_key(&name[..]).and_then(|o| o.as_string().and_then(|s| {
match s.parse::<MergeFunctions>() {
Ok(mergefunc) => base.$key_name = mergefunc,
_ => return Some(Err(format!("'{}' is not a valid value for \
@ -1521,7 +1554,7 @@ macro_rules! key {
} );
($key_name:ident, RelocModel) => ( {
let name = (stringify!($key_name)).replace("_", "-");
obj.find(&name[..]).and_then(|o| o.as_string().and_then(|s| {
obj.remove_key(&name[..]).and_then(|o| o.as_string().and_then(|s| {
match s.parse::<RelocModel>() {
Ok(relocation_model) => base.$key_name = relocation_model,
_ => return Some(Err(format!("'{}' is not a valid relocation model. \
@ -1533,7 +1566,7 @@ macro_rules! key {
} );
($key_name:ident, CodeModel) => ( {
let name = (stringify!($key_name)).replace("_", "-");
obj.find(&name[..]).and_then(|o| o.as_string().and_then(|s| {
obj.remove_key(&name[..]).and_then(|o| o.as_string().and_then(|s| {
match s.parse::<CodeModel>() {
Ok(code_model) => base.$key_name = Some(code_model),
_ => return Some(Err(format!("'{}' is not a valid code model. \
@ -1545,7 +1578,7 @@ macro_rules! key {
} );
($key_name:ident, TlsModel) => ( {
let name = (stringify!($key_name)).replace("_", "-");
obj.find(&name[..]).and_then(|o| o.as_string().and_then(|s| {
obj.remove_key(&name[..]).and_then(|o| o.as_string().and_then(|s| {
match s.parse::<TlsModel>() {
Ok(tls_model) => base.$key_name = tls_model,
_ => return Some(Err(format!("'{}' is not a valid TLS model. \
@ -1557,7 +1590,7 @@ macro_rules! key {
} );
($key_name:ident, PanicStrategy) => ( {
let name = (stringify!($key_name)).replace("_", "-");
obj.find(&name[..]).and_then(|o| o.as_string().and_then(|s| {
obj.remove_key(&name[..]).and_then(|o| o.as_string().and_then(|s| {
match s {
"unwind" => base.$key_name = PanicStrategy::Unwind,
"abort" => base.$key_name = PanicStrategy::Abort,
@ -1570,7 +1603,7 @@ macro_rules! key {
} );
($key_name:ident, RelroLevel) => ( {
let name = (stringify!($key_name)).replace("_", "-");
obj.find(&name[..]).and_then(|o| o.as_string().and_then(|s| {
obj.remove_key(&name[..]).and_then(|o| o.as_string().and_then(|s| {
match s.parse::<RelroLevel>() {
Ok(level) => base.$key_name = level,
_ => return Some(Err(format!("'{}' is not a valid value for \
@ -1582,7 +1615,7 @@ macro_rules! key {
} );
($key_name:ident, SplitDebuginfo) => ( {
let name = (stringify!($key_name)).replace("_", "-");
obj.find(&name[..]).and_then(|o| o.as_string().and_then(|s| {
obj.remove_key(&name[..]).and_then(|o| o.as_string().and_then(|s| {
match s.parse::<SplitDebuginfo>() {
Ok(level) => base.$key_name = level,
_ => return Some(Err(format!("'{}' is not a valid value for \
@ -1594,23 +1627,31 @@ macro_rules! key {
} );
($key_name:ident, list) => ( {
let name = (stringify!($key_name)).replace("_", "-");
if let Some(v) = obj.find(&name).and_then(Json::as_array) {
if let Some(j) = obj.remove_key(&name){
if let Some(v) = Json::as_array(&j) {
base.$key_name = v.iter()
.map(|a| a.as_string().unwrap().to_string())
.collect();
} else {
incorrect_type.push(name)
}
}
} );
($key_name:ident, opt_list) => ( {
let name = (stringify!($key_name)).replace("_", "-");
if let Some(v) = obj.find(&name).and_then(Json::as_array) {
if let Some(j) = obj.remove_key(&name) {
if let Some(v) = Json::as_array(&j) {
base.$key_name = Some(v.iter()
.map(|a| a.as_string().unwrap().to_string())
.collect());
} else {
incorrect_type.push(name)
}
}
} );
($key_name:ident, optional) => ( {
let name = (stringify!($key_name)).replace("_", "-");
if let Some(o) = obj.find(&name[..]) {
if let Some(o) = obj.remove_key(&name[..]) {
base.$key_name = o
.as_string()
.map(|s| s.to_string() );
@ -1618,7 +1659,7 @@ macro_rules! key {
} );
($key_name:ident, LldFlavor) => ( {
let name = (stringify!($key_name)).replace("_", "-");
obj.find(&name[..]).and_then(|o| o.as_string().and_then(|s| {
obj.remove_key(&name[..]).and_then(|o| o.as_string().and_then(|s| {
if let Some(flavor) = LldFlavor::from_str(&s) {
base.$key_name = flavor;
} else {
@ -1632,7 +1673,7 @@ macro_rules! key {
} );
($key_name:ident, LinkerFlavor) => ( {
let name = (stringify!($key_name)).replace("_", "-");
obj.find(&name[..]).and_then(|o| o.as_string().and_then(|s| {
obj.remove_key(&name[..]).and_then(|o| o.as_string().and_then(|s| {
match LinkerFlavor::from_str(s) {
Some(linker_flavor) => base.$key_name = linker_flavor,
_ => return Some(Err(format!("'{}' is not a valid value for linker-flavor. \
@ -1643,7 +1684,7 @@ macro_rules! key {
} );
($key_name:ident, StackProbeType) => ( {
let name = (stringify!($key_name)).replace("_", "-");
obj.find(&name[..]).and_then(|o| match StackProbeType::from_json(o) {
obj.remove_key(&name[..]).and_then(|o| match StackProbeType::from_json(&o) {
Ok(v) => {
base.$key_name = v;
Some(Ok(()))
@ -1655,7 +1696,8 @@ macro_rules! key {
} );
($key_name:ident, SanitizerSet) => ( {
let name = (stringify!($key_name)).replace("_", "-");
obj.find(&name[..]).and_then(|o| o.as_array()).and_then(|a| {
if let Some(o) = obj.remove_key(&name[..]) {
if let Some(a) = o.as_array() {
for s in a {
base.$key_name |= match s.as_string() {
Some("address") => SanitizerSet::ADDRESS,
@ -1663,17 +1705,20 @@ macro_rules! key {
Some("memory") => SanitizerSet::MEMORY,
Some("thread") => SanitizerSet::THREAD,
Some("hwaddress") => SanitizerSet::HWADDRESS,
Some(s) => return Some(Err(format!("unknown sanitizer {}", s))),
_ => return Some(Err(format!("not a string: {:?}", s))),
Some(s) => return Err(format!("unknown sanitizer {}", s)),
_ => return Err(format!("not a string: {:?}", s)),
};
}
Some(Ok(()))
}).unwrap_or(Ok(()))
} else {
incorrect_type.push(name)
}
}
Ok::<(), String>(())
} );
($key_name:ident, crt_objects_fallback) => ( {
let name = (stringify!($key_name)).replace("_", "-");
obj.find(&name[..]).and_then(|o| o.as_string().and_then(|s| {
obj.remove_key(&name[..]).and_then(|o| o.as_string().and_then(|s| {
match s.parse::<CrtObjectsFallback>() {
Ok(fallback) => base.$key_name = Some(fallback),
_ => return Some(Err(format!("'{}' is not a valid CRT objects fallback. \
@ -1684,7 +1729,7 @@ macro_rules! key {
} );
($key_name:ident, link_objects) => ( {
let name = (stringify!($key_name)).replace("_", "-");
if let Some(val) = obj.find(&name[..]) {
if let Some(val) = obj.remove_key(&name[..]) {
let obj = val.as_object().ok_or_else(|| format!("{}: expected a \
JSON object with fields per CRT object kind.", name))?;
let mut args = CrtObjects::new();
@ -1712,7 +1757,7 @@ macro_rules! key {
} );
($key_name:ident, link_args) => ( {
let name = (stringify!($key_name)).replace("_", "-");
if let Some(val) = obj.find(&name[..]) {
if let Some(val) = obj.remove_key(&name[..]) {
let obj = val.as_object().ok_or_else(|| format!("{}: expected a \
JSON object with fields per linker-flavor.", name))?;
let mut args = LinkArgs::new();
@ -1739,7 +1784,8 @@ macro_rules! key {
} );
($key_name:ident, env) => ( {
let name = (stringify!($key_name)).replace("_", "-");
if let Some(a) = obj.find(&name[..]).and_then(|o| o.as_array()) {
if let Some(o) = obj.remove_key(&name[..]) {
if let Some(a) = o.as_array() {
for o in a {
if let Some(s) = o.as_string() {
let p = s.split('=').collect::<Vec<_>>();
@ -1750,11 +1796,14 @@ macro_rules! key {
}
}
}
} else {
incorrect_type.push(name)
}
}
} );
($key_name:ident, Option<Abi>) => ( {
let name = (stringify!($key_name)).replace("_", "-");
obj.find(&name[..]).and_then(|o| o.as_string().and_then(|s| {
obj.remove_key(&name[..]).and_then(|o| o.as_string().and_then(|s| {
match lookup_abi(s) {
Some(abi) => base.$key_name = Some(abi),
_ => return Some(Err(format!("'{}' is not a valid value for abi", s))),
@ -1763,20 +1812,26 @@ macro_rules! key {
})).unwrap_or(Ok(()))
} );
($key_name:ident, TargetFamilies) => ( {
let value = obj.find("target-family");
if let Some(v) = value.and_then(Json::as_array) {
if let Some(value) = obj.remove_key("target-family") {
if let Some(v) = Json::as_array(&value) {
base.$key_name = v.iter()
.map(|a| a.as_string().unwrap().to_string())
.collect();
} else if let Some(v) = value.and_then(Json::as_string) {
} else if let Some(v) = Json::as_string(&value) {
base.$key_name = vec![v.to_string()];
}
}
} );
}
if let Some(s) = obj.find("target-endian").and_then(Json::as_string) {
if let Some(j) = obj.remove_key("target-endian") {
if let Some(s) = Json::as_string(&j) {
base.endian = s.parse()?;
} else {
incorrect_type.push("target-endian".to_string())
}
}
key!(is_builtin, bool);
key!(c_int_width = "target-c-int-width");
key!(os);
@ -1876,7 +1931,8 @@ macro_rules! key {
// NB: The old name is deprecated, but support for it is retained for
// compatibility.
for name in ["abi-blacklist", "unsupported-abis"].iter() {
if let Some(array) = obj.find(name).and_then(Json::as_array) {
if let Some(j) = obj.remove_key(name) {
if let Some(array) = Json::as_array(&j) {
for name in array.iter().filter_map(|abi| abi.as_string()) {
match lookup_abi(name) {
Some(abi) => {
@ -1900,8 +1956,14 @@ macro_rules! key {
}
}
}
}
Ok(base)
// Each field should have been read using `Json::remove_key` so any keys remaining are unused.
let remaining_keys = obj.as_object().ok_or("Expected JSON object for target")?.keys();
Ok((
base,
TargetWarnings { unused_fields: remaining_keys.cloned().collect(), incorrect_type },
))
}
/// Search for a JSON file specifying the given target triple.
@ -1913,12 +1975,15 @@ macro_rules! key {
///
/// The error string could come from any of the APIs called, including filesystem access and
/// JSON decoding.
pub fn search(target_triple: &TargetTriple, sysroot: &PathBuf) -> Result<Target, String> {
pub fn search(
target_triple: &TargetTriple,
sysroot: &PathBuf,
) -> Result<(Target, TargetWarnings), String> {
use rustc_serialize::json;
use std::env;
use std::fs;
fn load_file(path: &Path) -> Result<Target, String> {
fn load_file(path: &Path) -> Result<(Target, TargetWarnings), String> {
let contents = fs::read(path).map_err(|e| e.to_string())?;
let obj = json::from_reader(&mut &contents[..]).map_err(|e| e.to_string())?;
Target::from_json(obj)
@ -1928,7 +1993,7 @@ fn load_file(path: &Path) -> Result<Target, String> {
TargetTriple::TargetTriple(ref target_triple) => {
// check if triple is in list of built-in targets
if let Some(t) = load_builtin(target_triple) {
return Ok(t);
return Ok((t, TargetWarnings::empty()));
}
// search for a file named `target_triple`.json in RUST_TARGET_PATH

View File

@ -3,7 +3,7 @@
// Test target self-consistency and JSON encoding/decoding roundtrip.
pub(super) fn test_target(target: Target) {
target.check_consistency();
assert_eq!(Target::from_json(target.to_json()), Ok(target));
assert_eq!(Target::from_json(target.to_json()).map(|(j, _)| j), Ok(target));
}
impl Target {

View File

@ -0,0 +1,59 @@
use crate::spec::Target;
use rustc_serialize::json::Json;
use std::str::FromStr;
#[test]
fn report_unused_fields() {
let json = Json::from_str(
r#"
{
"arch": "powerpc64",
"data-layout": "e-m:e-i64:64-n32:64",
"llvm-target": "powerpc64le-elf",
"target-pointer-width": "64",
"code-mode": "foo"
}
"#,
)
.unwrap();
let warnings = Target::from_json(json).unwrap().1;
assert_eq!(warnings.warning_messages().len(), 1);
assert!(warnings.warning_messages().join("\n").contains("code-mode"));
}
#[test]
fn report_incorrect_json_type() {
let json = Json::from_str(
r#"
{
"arch": "powerpc64",
"data-layout": "e-m:e-i64:64-n32:64",
"llvm-target": "powerpc64le-elf",
"target-pointer-width": "64",
"link-env-remove": "foo"
}
"#,
)
.unwrap();
let warnings = Target::from_json(json).unwrap().1;
assert_eq!(warnings.warning_messages().len(), 1);
assert!(warnings.warning_messages().join("\n").contains("link-env-remove"));
}
#[test]
fn no_warnings_for_valid_target() {
let json = Json::from_str(
r#"
{
"arch": "powerpc64",
"data-layout": "e-m:e-i64:64-n32:64",
"llvm-target": "powerpc64le-elf",
"target-pointer-width": "64",
"link-env-remove": ["foo"]
}
"#,
)
.unwrap();
let warnings = Target::from_json(json).unwrap().1;
assert_eq!(warnings.warning_messages().len(), 0);
}