From 35c4aae99ece980c7c594940724f0728fcccf226 Mon Sep 17 00:00:00 2001 From: Sebastian Imlay Date: Fri, 10 Jan 2020 18:48:37 -0800 Subject: [PATCH 01/27] Added tvOS as a backend --- .../spec/aarch64_apple_tvos.rs | 25 ++++ src/librustc_target/spec/apple_tvos_base.rs | 123 ++++++++++++++++++ src/librustc_target/spec/mod.rs | 3 + src/librustc_target/spec/x86_64_apple_tvos.rs | 19 +++ 4 files changed, 170 insertions(+) create mode 100644 src/librustc_target/spec/aarch64_apple_tvos.rs create mode 100644 src/librustc_target/spec/apple_tvos_base.rs create mode 100644 src/librustc_target/spec/x86_64_apple_tvos.rs diff --git a/src/librustc_target/spec/aarch64_apple_tvos.rs b/src/librustc_target/spec/aarch64_apple_tvos.rs new file mode 100644 index 0000000000000..420e0d1c64c35 --- /dev/null +++ b/src/librustc_target/spec/aarch64_apple_tvos.rs @@ -0,0 +1,25 @@ +use super::apple_tvos_base::{opts, Arch}; +use crate::spec::{LinkerFlavor, Target, TargetOptions, TargetResult}; + +pub fn target() -> TargetResult { + let base = opts(Arch::Arm64)?; + Ok(Target { + llvm_target: "arm64-apple-tvos".to_string(), + target_endian: "little".to_string(), + target_pointer_width: "64".to_string(), + target_c_int_width: "32".to_string(), + data_layout: "e-m:o-i64:64-i128:128-n32:64-S128".to_string(), + arch: "aarch64".to_string(), + target_os: "tvos".to_string(), + target_env: String::new(), + target_vendor: "apple".to_string(), + linker_flavor: LinkerFlavor::Gcc, + options: TargetOptions { + features: "+neon,+fp-armv8,+cyclone".to_string(), + eliminate_frame_pointer: false, + max_atomic_width: Some(128), + abi_blacklist: super::arm_base::abi_blacklist(), + ..base + }, + }) +} diff --git a/src/librustc_target/spec/apple_tvos_base.rs b/src/librustc_target/spec/apple_tvos_base.rs new file mode 100644 index 0000000000000..70dc8093f88c1 --- /dev/null +++ b/src/librustc_target/spec/apple_tvos_base.rs @@ -0,0 +1,123 @@ +use crate::spec::{LinkArgs, LinkerFlavor, TargetOptions}; +use std::env; +use std::io; +use std::path::Path; +use std::process::Command; + +use Arch::*; + +#[allow(non_camel_case_types)] +#[derive(Copy, Clone)] +pub enum Arch { + Arm64, + X86_64, +} + +impl Arch { + pub fn to_string(self) -> &'static str { + match self { + Arm64 => "arm64", + X86_64 => "x86_64", + } + } +} + +pub fn get_sdk_root(sdk_name: &str) -> Result { + // Following what clang does + // (https://github.com/llvm/llvm-project/blob/ + // 296a80102a9b72c3eda80558fb78a3ed8849b341/clang/lib/Driver/ToolChains/Darwin.cpp#L1661-L1678) + // to allow the SDK path to be set. (For clang, xcrun sets + // SDKROOT; for rustc, the user or build system can set it, or we + // can fall back to checking for xcrun on PATH.) + if let Some(sdkroot) = env::var("SDKROOT").ok() { + let p = Path::new(&sdkroot); + match sdk_name { + // Ignore `SDKROOT` if it's clearly set for the wrong platform. + "appletvos" + if sdkroot.contains("TVSimulator.platform") + || sdkroot.contains("MacOSX.platform") => + { + () + } + "appletvsimulator" + if sdkroot.contains("TVOS.platform") || sdkroot.contains("MacOSX.platform") => + { + () + } + // Ignore `SDKROOT` if it's not a valid path. + _ if !p.is_absolute() || p == Path::new("/") || !p.exists() => (), + _ => return Ok(sdkroot), + } + } + let res = + Command::new("xcrun").arg("--show-sdk-path").arg("-sdk").arg(sdk_name).output().and_then( + |output| { + if output.status.success() { + Ok(String::from_utf8(output.stdout).unwrap()) + } else { + let error = String::from_utf8(output.stderr); + let error = format!("process exit with error: {}", error.unwrap()); + Err(io::Error::new(io::ErrorKind::Other, &error[..])) + } + }, + ); + + match res { + Ok(output) => Ok(output.trim().to_string()), + Err(e) => Err(format!("failed to get {} SDK path: {}", sdk_name, e)), + } +} + +fn build_pre_link_args(arch: Arch) -> Result { + let sdk_name = match arch { + Arm64 => "appletvos", + X86_64 => "appletvsimulator", + }; + + let arch_name = arch.to_string(); + + let sdk_root = get_sdk_root(sdk_name)?; + + let mut args = LinkArgs::new(); + args.insert( + LinkerFlavor::Gcc, + vec![ + "-arch".to_string(), + arch_name.to_string(), + "-isysroot".to_string(), + sdk_root.clone(), + "-Wl,-syslibroot".to_string(), + sdk_root, + ], + ); + + Ok(args) +} + +fn target_cpu(arch: Arch) -> String { + match arch { + Arm64 => "cyclone", + X86_64 => "core2", + } + .to_string() +} + +fn link_env_remove(arch: Arch) -> Vec { + match arch { + Arm64 | X86_64 => vec!["MACOSX_DEPLOYMENT_TARGET".to_string()], + } +} + +pub fn opts(arch: Arch) -> Result { + let pre_link_args = build_pre_link_args(arch)?; + Ok(TargetOptions { + cpu: target_cpu(arch), + dynamic_linking: false, + executables: true, + pre_link_args, + link_env_remove: link_env_remove(arch), + has_elf_tls: false, + eliminate_frame_pointer: false, + ..super::apple_base::opts() + }) +} diff --git a/src/librustc_target/spec/mod.rs b/src/librustc_target/spec/mod.rs index f08634cc770e0..fddbcb90da143 100644 --- a/src/librustc_target/spec/mod.rs +++ b/src/librustc_target/spec/mod.rs @@ -48,6 +48,7 @@ pub mod abi; mod android_base; mod apple_base; mod apple_ios_base; +mod apple_tvos_base; mod arm_base; mod cloudabi_base; mod dragonfly_base; @@ -434,6 +435,8 @@ supported_targets! { ("armv7-apple-ios", armv7_apple_ios), ("armv7s-apple-ios", armv7s_apple_ios), ("x86_64-apple-ios-macabi", x86_64_apple_ios_macabi), + ("aarch64-apple-tvos", aarch64_apple_tvos), + ("x86_64-apple-tvos", x86_64_apple_tvos), ("armebv7r-none-eabi", armebv7r_none_eabi), ("armebv7r-none-eabihf", armebv7r_none_eabihf), diff --git a/src/librustc_target/spec/x86_64_apple_tvos.rs b/src/librustc_target/spec/x86_64_apple_tvos.rs new file mode 100644 index 0000000000000..e40d978e750b7 --- /dev/null +++ b/src/librustc_target/spec/x86_64_apple_tvos.rs @@ -0,0 +1,19 @@ +use super::apple_tvos_base::{opts, Arch}; +use crate::spec::{LinkerFlavor, Target, TargetOptions, TargetResult}; + +pub fn target() -> TargetResult { + let base = opts(Arch::X86_64)?; + Ok(Target { + llvm_target: "x86_64-apple-tvos".to_string(), + target_endian: "little".to_string(), + target_pointer_width: "64".to_string(), + target_c_int_width: "32".to_string(), + data_layout: "e-m:o-i64:64-f80:128-n8:16:32:64-S128".to_string(), + arch: "x86_64".to_string(), + target_os: "tvos".to_string(), + target_env: String::new(), + target_vendor: "apple".to_string(), + linker_flavor: LinkerFlavor::Gcc, + options: TargetOptions { max_atomic_width: Some(64), stack_probes: true, ..base }, + }) +} From 6b17330ef4f9a97bd539b55edb4e7ccdb6c00133 Mon Sep 17 00:00:00 2001 From: Sebastian Imlay Date: Fri, 14 Feb 2020 16:05:45 -0800 Subject: [PATCH 02/27] Merged apple_tvos_base and apple_ios_base into apple_sdk_base. --- src/librustc_target/spec/aarch64_apple_ios.rs | 4 +- .../spec/aarch64_apple_tvos.rs | 4 +- .../{apple_ios_base.rs => apple_sdk_base.rs} | 43 ++++-- src/librustc_target/spec/apple_tvos_base.rs | 123 ------------------ src/librustc_target/spec/armv7_apple_ios.rs | 4 +- src/librustc_target/spec/armv7s_apple_ios.rs | 4 +- src/librustc_target/spec/i386_apple_ios.rs | 4 +- src/librustc_target/spec/mod.rs | 3 +- src/librustc_target/spec/x86_64_apple_ios.rs | 4 +- .../spec/x86_64_apple_ios_macabi.rs | 4 +- src/librustc_target/spec/x86_64_apple_tvos.rs | 4 +- 11 files changed, 51 insertions(+), 150 deletions(-) rename src/librustc_target/spec/{apple_ios_base.rs => apple_sdk_base.rs} (74%) delete mode 100644 src/librustc_target/spec/apple_tvos_base.rs diff --git a/src/librustc_target/spec/aarch64_apple_ios.rs b/src/librustc_target/spec/aarch64_apple_ios.rs index 6549be41ea944..2216af428fa13 100644 --- a/src/librustc_target/spec/aarch64_apple_ios.rs +++ b/src/librustc_target/spec/aarch64_apple_ios.rs @@ -1,8 +1,8 @@ -use super::apple_ios_base::{opts, Arch}; +use super::apple_sdk_base::{opts, Arch, AppleOS}; use crate::spec::{LinkerFlavor, Target, TargetOptions, TargetResult}; pub fn target() -> TargetResult { - let base = opts(Arch::Arm64)?; + let base = opts(Arch::Arm64, AppleOS::iOS)?; Ok(Target { llvm_target: "arm64-apple-ios".to_string(), target_endian: "little".to_string(), diff --git a/src/librustc_target/spec/aarch64_apple_tvos.rs b/src/librustc_target/spec/aarch64_apple_tvos.rs index 420e0d1c64c35..a87d5965c3dae 100644 --- a/src/librustc_target/spec/aarch64_apple_tvos.rs +++ b/src/librustc_target/spec/aarch64_apple_tvos.rs @@ -1,8 +1,8 @@ -use super::apple_tvos_base::{opts, Arch}; +use super::apple_sdk_base::{opts, Arch, AppleOS}; use crate::spec::{LinkerFlavor, Target, TargetOptions, TargetResult}; pub fn target() -> TargetResult { - let base = opts(Arch::Arm64)?; + let base = opts(Arch::Arm64, AppleOS::tvOS)?; Ok(Target { llvm_target: "arm64-apple-tvos".to_string(), target_endian: "little".to_string(), diff --git a/src/librustc_target/spec/apple_ios_base.rs b/src/librustc_target/spec/apple_sdk_base.rs similarity index 74% rename from src/librustc_target/spec/apple_ios_base.rs rename to src/librustc_target/spec/apple_sdk_base.rs index 2673748321d79..2c93cbc4d8595 100644 --- a/src/librustc_target/spec/apple_ios_base.rs +++ b/src/librustc_target/spec/apple_sdk_base.rs @@ -1,3 +1,4 @@ + use crate::spec::{LinkArgs, LinkerFlavor, TargetOptions}; use std::env; use std::io; @@ -5,7 +6,6 @@ use std::path::Path; use std::process::Command; use Arch::*; - #[allow(non_camel_case_types)] #[derive(Copy, Clone)] pub enum Arch { @@ -17,6 +17,13 @@ pub enum Arch { X86_64_macabi, } +#[allow(non_camel_case_types)] +#[derive(Copy, Clone)] +pub enum AppleOS { + tvOS, + iOS, +} + impl Arch { pub fn to_string(self) -> &'static str { match self { @@ -41,6 +48,17 @@ pub fn get_sdk_root(sdk_name: &str) -> Result { let p = Path::new(&sdkroot); match sdk_name { // Ignore `SDKROOT` if it's clearly set for the wrong platform. + "appletvos" + if sdkroot.contains("TVSimulator.platform") + || sdkroot.contains("MacOSX.platform") => + { + () + } + "appletvsimulator" + if sdkroot.contains("TVOS.platform") || sdkroot.contains("MacOSX.platform") => + { + () + } "iphoneos" if sdkroot.contains("iPhoneSimulator.platform") || sdkroot.contains("MacOSX.platform") => @@ -82,11 +100,17 @@ pub fn get_sdk_root(sdk_name: &str) -> Result { } } -fn build_pre_link_args(arch: Arch) -> Result { - let sdk_name = match arch { - Armv7 | Armv7s | Arm64 => "iphoneos", - I386 | X86_64 => "iphonesimulator", - X86_64_macabi => "macosx10.15", +fn build_pre_link_args(arch: Arch, os: AppleOS) -> Result { + let sdk_name = match (arch, os) { + (Arm64, AppleOS::tvOS) => "appletvos", + (X86_64, AppleOS::tvOS) => "appletvsimulator", + (Armv7, AppleOS::iOS) => "iphoneos", + (Armv7s, AppleOS::iOS) => "iphoneos", + (Arm64, AppleOS::iOS) => "iphoneos", + (I386, AppleOS::iOS) => "iphonesimulator", + (X86_64, AppleOS::iOS) => "iphonesimulator", + (X86_64_macabi, AppleOS::iOS) => "macosx10.15", + _ => unreachable!(), }; let arch_name = arch.to_string(); @@ -121,15 +145,16 @@ fn target_cpu(arch: Arch) -> String { .to_string() } + fn link_env_remove(arch: Arch) -> Vec { match arch { Armv7 | Armv7s | Arm64 | I386 | X86_64 => vec!["MACOSX_DEPLOYMENT_TARGET".to_string()], - X86_64_macabi => vec!["IPHONEOS_DEPLOYMENT_TARGET".to_string()], + X86_64_macabi => vec![ "IPHONEOS_DEPLOYMENT_TARGET".to_string() ,], } } -pub fn opts(arch: Arch) -> Result { - let pre_link_args = build_pre_link_args(arch)?; +pub fn opts(arch: Arch, os: AppleOS) -> Result { + let pre_link_args = build_pre_link_args(arch, os)?; Ok(TargetOptions { cpu: target_cpu(arch), dynamic_linking: false, diff --git a/src/librustc_target/spec/apple_tvos_base.rs b/src/librustc_target/spec/apple_tvos_base.rs deleted file mode 100644 index 70dc8093f88c1..0000000000000 --- a/src/librustc_target/spec/apple_tvos_base.rs +++ /dev/null @@ -1,123 +0,0 @@ -use crate::spec::{LinkArgs, LinkerFlavor, TargetOptions}; -use std::env; -use std::io; -use std::path::Path; -use std::process::Command; - -use Arch::*; - -#[allow(non_camel_case_types)] -#[derive(Copy, Clone)] -pub enum Arch { - Arm64, - X86_64, -} - -impl Arch { - pub fn to_string(self) -> &'static str { - match self { - Arm64 => "arm64", - X86_64 => "x86_64", - } - } -} - -pub fn get_sdk_root(sdk_name: &str) -> Result { - // Following what clang does - // (https://github.com/llvm/llvm-project/blob/ - // 296a80102a9b72c3eda80558fb78a3ed8849b341/clang/lib/Driver/ToolChains/Darwin.cpp#L1661-L1678) - // to allow the SDK path to be set. (For clang, xcrun sets - // SDKROOT; for rustc, the user or build system can set it, or we - // can fall back to checking for xcrun on PATH.) - if let Some(sdkroot) = env::var("SDKROOT").ok() { - let p = Path::new(&sdkroot); - match sdk_name { - // Ignore `SDKROOT` if it's clearly set for the wrong platform. - "appletvos" - if sdkroot.contains("TVSimulator.platform") - || sdkroot.contains("MacOSX.platform") => - { - () - } - "appletvsimulator" - if sdkroot.contains("TVOS.platform") || sdkroot.contains("MacOSX.platform") => - { - () - } - // Ignore `SDKROOT` if it's not a valid path. - _ if !p.is_absolute() || p == Path::new("/") || !p.exists() => (), - _ => return Ok(sdkroot), - } - } - let res = - Command::new("xcrun").arg("--show-sdk-path").arg("-sdk").arg(sdk_name).output().and_then( - |output| { - if output.status.success() { - Ok(String::from_utf8(output.stdout).unwrap()) - } else { - let error = String::from_utf8(output.stderr); - let error = format!("process exit with error: {}", error.unwrap()); - Err(io::Error::new(io::ErrorKind::Other, &error[..])) - } - }, - ); - - match res { - Ok(output) => Ok(output.trim().to_string()), - Err(e) => Err(format!("failed to get {} SDK path: {}", sdk_name, e)), - } -} - -fn build_pre_link_args(arch: Arch) -> Result { - let sdk_name = match arch { - Arm64 => "appletvos", - X86_64 => "appletvsimulator", - }; - - let arch_name = arch.to_string(); - - let sdk_root = get_sdk_root(sdk_name)?; - - let mut args = LinkArgs::new(); - args.insert( - LinkerFlavor::Gcc, - vec![ - "-arch".to_string(), - arch_name.to_string(), - "-isysroot".to_string(), - sdk_root.clone(), - "-Wl,-syslibroot".to_string(), - sdk_root, - ], - ); - - Ok(args) -} - -fn target_cpu(arch: Arch) -> String { - match arch { - Arm64 => "cyclone", - X86_64 => "core2", - } - .to_string() -} - -fn link_env_remove(arch: Arch) -> Vec { - match arch { - Arm64 | X86_64 => vec!["MACOSX_DEPLOYMENT_TARGET".to_string()], - } -} - -pub fn opts(arch: Arch) -> Result { - let pre_link_args = build_pre_link_args(arch)?; - Ok(TargetOptions { - cpu: target_cpu(arch), - dynamic_linking: false, - executables: true, - pre_link_args, - link_env_remove: link_env_remove(arch), - has_elf_tls: false, - eliminate_frame_pointer: false, - ..super::apple_base::opts() - }) -} diff --git a/src/librustc_target/spec/armv7_apple_ios.rs b/src/librustc_target/spec/armv7_apple_ios.rs index aa2d32e2d7935..19d189e2543fc 100644 --- a/src/librustc_target/spec/armv7_apple_ios.rs +++ b/src/librustc_target/spec/armv7_apple_ios.rs @@ -1,8 +1,8 @@ -use super::apple_ios_base::{opts, Arch}; +use super::apple_sdk_base::{opts, Arch, AppleOS}; use crate::spec::{LinkerFlavor, Target, TargetOptions, TargetResult}; pub fn target() -> TargetResult { - let base = opts(Arch::Armv7)?; + let base = opts(Arch::Armv7, AppleOS::iOS)?; Ok(Target { llvm_target: "armv7-apple-ios".to_string(), target_endian: "little".to_string(), diff --git a/src/librustc_target/spec/armv7s_apple_ios.rs b/src/librustc_target/spec/armv7s_apple_ios.rs index 6514643a64dae..53fb8a9ff9fdc 100644 --- a/src/librustc_target/spec/armv7s_apple_ios.rs +++ b/src/librustc_target/spec/armv7s_apple_ios.rs @@ -1,8 +1,8 @@ -use super::apple_ios_base::{opts, Arch}; +use super::apple_sdk_base::{opts, Arch, AppleOS}; use crate::spec::{LinkerFlavor, Target, TargetOptions, TargetResult}; pub fn target() -> TargetResult { - let base = opts(Arch::Armv7s)?; + let base = opts(Arch::Armv7s, AppleOS::iOS)?; Ok(Target { llvm_target: "armv7s-apple-ios".to_string(), target_endian: "little".to_string(), diff --git a/src/librustc_target/spec/i386_apple_ios.rs b/src/librustc_target/spec/i386_apple_ios.rs index a6c1d24fa62a1..eb0c68bc7b83e 100644 --- a/src/librustc_target/spec/i386_apple_ios.rs +++ b/src/librustc_target/spec/i386_apple_ios.rs @@ -1,8 +1,8 @@ -use super::apple_ios_base::{opts, Arch}; +use super::apple_sdk_base::{opts, Arch, AppleOS}; use crate::spec::{LinkerFlavor, Target, TargetOptions, TargetResult}; pub fn target() -> TargetResult { - let base = opts(Arch::I386)?; + let base = opts(Arch::I386, AppleOS::iOS)?; Ok(Target { llvm_target: "i386-apple-ios".to_string(), target_endian: "little".to_string(), diff --git a/src/librustc_target/spec/mod.rs b/src/librustc_target/spec/mod.rs index a110fd0367380..37eabb4f26d4b 100644 --- a/src/librustc_target/spec/mod.rs +++ b/src/librustc_target/spec/mod.rs @@ -47,8 +47,7 @@ use rustc_macros::HashStable_Generic; pub mod abi; mod android_base; mod apple_base; -mod apple_ios_base; -mod apple_tvos_base; +mod apple_sdk_base; mod arm_base; mod cloudabi_base; mod dragonfly_base; diff --git a/src/librustc_target/spec/x86_64_apple_ios.rs b/src/librustc_target/spec/x86_64_apple_ios.rs index ca02e2deabcf2..db5361f1d813d 100644 --- a/src/librustc_target/spec/x86_64_apple_ios.rs +++ b/src/librustc_target/spec/x86_64_apple_ios.rs @@ -1,8 +1,8 @@ -use super::apple_ios_base::{opts, Arch}; +use super::apple_sdk_base::{opts, Arch, AppleOS}; use crate::spec::{LinkerFlavor, Target, TargetOptions, TargetResult}; pub fn target() -> TargetResult { - let base = opts(Arch::X86_64)?; + let base = opts(Arch::X86_64, AppleOS::iOS)?; Ok(Target { llvm_target: "x86_64-apple-ios".to_string(), target_endian: "little".to_string(), diff --git a/src/librustc_target/spec/x86_64_apple_ios_macabi.rs b/src/librustc_target/spec/x86_64_apple_ios_macabi.rs index 5f4f6ade682d8..cce6221134df3 100644 --- a/src/librustc_target/spec/x86_64_apple_ios_macabi.rs +++ b/src/librustc_target/spec/x86_64_apple_ios_macabi.rs @@ -1,8 +1,8 @@ -use super::apple_ios_base::{opts, Arch}; +use super::apple_sdk_base::{opts, Arch, AppleOS}; use crate::spec::{LinkerFlavor, Target, TargetOptions, TargetResult}; pub fn target() -> TargetResult { - let base = opts(Arch::X86_64_macabi)?; + let base = opts(Arch::X86_64_macabi, AppleOS::iOS)?; Ok(Target { llvm_target: "x86_64-apple-ios13.0-macabi".to_string(), target_endian: "little".to_string(), diff --git a/src/librustc_target/spec/x86_64_apple_tvos.rs b/src/librustc_target/spec/x86_64_apple_tvos.rs index e40d978e750b7..794df42f43e32 100644 --- a/src/librustc_target/spec/x86_64_apple_tvos.rs +++ b/src/librustc_target/spec/x86_64_apple_tvos.rs @@ -1,8 +1,8 @@ -use super::apple_tvos_base::{opts, Arch}; +use super::apple_sdk_base::{opts, Arch, AppleOS}; use crate::spec::{LinkerFlavor, Target, TargetOptions, TargetResult}; pub fn target() -> TargetResult { - let base = opts(Arch::X86_64)?; + let base = opts(Arch::X86_64, AppleOS::iOS)?; Ok(Target { llvm_target: "x86_64-apple-tvos".to_string(), target_endian: "little".to_string(), From 259977158e19acccc1bbc71c8cc4449ad40a8bef Mon Sep 17 00:00:00 2001 From: Sebastian Imlay Date: Thu, 20 Feb 2020 10:16:32 -0800 Subject: [PATCH 03/27] fmt --- src/librustc_target/spec/aarch64_apple_ios.rs | 2 +- .../spec/aarch64_apple_tvos.rs | 2 +- src/librustc_target/spec/apple_sdk_base.rs | 20 +++++++++---------- src/librustc_target/spec/armv7_apple_ios.rs | 2 +- src/librustc_target/spec/armv7s_apple_ios.rs | 2 +- src/librustc_target/spec/i386_apple_ios.rs | 2 +- src/librustc_target/spec/x86_64_apple_ios.rs | 2 +- .../spec/x86_64_apple_ios_macabi.rs | 2 +- src/librustc_target/spec/x86_64_apple_tvos.rs | 2 +- 9 files changed, 17 insertions(+), 19 deletions(-) diff --git a/src/librustc_target/spec/aarch64_apple_ios.rs b/src/librustc_target/spec/aarch64_apple_ios.rs index 2216af428fa13..e896b46da9a62 100644 --- a/src/librustc_target/spec/aarch64_apple_ios.rs +++ b/src/librustc_target/spec/aarch64_apple_ios.rs @@ -1,4 +1,4 @@ -use super::apple_sdk_base::{opts, Arch, AppleOS}; +use super::apple_sdk_base::{opts, AppleOS, Arch}; use crate::spec::{LinkerFlavor, Target, TargetOptions, TargetResult}; pub fn target() -> TargetResult { diff --git a/src/librustc_target/spec/aarch64_apple_tvos.rs b/src/librustc_target/spec/aarch64_apple_tvos.rs index a87d5965c3dae..794bc7900e747 100644 --- a/src/librustc_target/spec/aarch64_apple_tvos.rs +++ b/src/librustc_target/spec/aarch64_apple_tvos.rs @@ -1,4 +1,4 @@ -use super::apple_sdk_base::{opts, Arch, AppleOS}; +use super::apple_sdk_base::{opts, AppleOS, Arch}; use crate::spec::{LinkerFlavor, Target, TargetOptions, TargetResult}; pub fn target() -> TargetResult { diff --git a/src/librustc_target/spec/apple_sdk_base.rs b/src/librustc_target/spec/apple_sdk_base.rs index 2c93cbc4d8595..513754352fbfb 100644 --- a/src/librustc_target/spec/apple_sdk_base.rs +++ b/src/librustc_target/spec/apple_sdk_base.rs @@ -1,4 +1,3 @@ - use crate::spec::{LinkArgs, LinkerFlavor, TargetOptions}; use std::env; use std::io; @@ -102,14 +101,14 @@ pub fn get_sdk_root(sdk_name: &str) -> Result { fn build_pre_link_args(arch: Arch, os: AppleOS) -> Result { let sdk_name = match (arch, os) { - (Arm64, AppleOS::tvOS) => "appletvos", - (X86_64, AppleOS::tvOS) => "appletvsimulator", - (Armv7, AppleOS::iOS) => "iphoneos", - (Armv7s, AppleOS::iOS) => "iphoneos", - (Arm64, AppleOS::iOS) => "iphoneos", - (I386, AppleOS::iOS) => "iphonesimulator", - (X86_64, AppleOS::iOS) => "iphonesimulator", - (X86_64_macabi, AppleOS::iOS) => "macosx10.15", + (Arm64, AppleOS::tvOS) => "appletvos", + (X86_64, AppleOS::tvOS) => "appletvsimulator", + (Armv7, AppleOS::iOS) => "iphoneos", + (Armv7s, AppleOS::iOS) => "iphoneos", + (Arm64, AppleOS::iOS) => "iphoneos", + (I386, AppleOS::iOS) => "iphonesimulator", + (X86_64, AppleOS::iOS) => "iphonesimulator", + (X86_64_macabi, AppleOS::iOS) => "macosx10.15", _ => unreachable!(), }; @@ -145,11 +144,10 @@ fn target_cpu(arch: Arch) -> String { .to_string() } - fn link_env_remove(arch: Arch) -> Vec { match arch { Armv7 | Armv7s | Arm64 | I386 | X86_64 => vec!["MACOSX_DEPLOYMENT_TARGET".to_string()], - X86_64_macabi => vec![ "IPHONEOS_DEPLOYMENT_TARGET".to_string() ,], + X86_64_macabi => vec!["IPHONEOS_DEPLOYMENT_TARGET".to_string()], } } diff --git a/src/librustc_target/spec/armv7_apple_ios.rs b/src/librustc_target/spec/armv7_apple_ios.rs index 19d189e2543fc..c0c2ae909f8f0 100644 --- a/src/librustc_target/spec/armv7_apple_ios.rs +++ b/src/librustc_target/spec/armv7_apple_ios.rs @@ -1,4 +1,4 @@ -use super::apple_sdk_base::{opts, Arch, AppleOS}; +use super::apple_sdk_base::{opts, AppleOS, Arch}; use crate::spec::{LinkerFlavor, Target, TargetOptions, TargetResult}; pub fn target() -> TargetResult { diff --git a/src/librustc_target/spec/armv7s_apple_ios.rs b/src/librustc_target/spec/armv7s_apple_ios.rs index 53fb8a9ff9fdc..6a5654f10d416 100644 --- a/src/librustc_target/spec/armv7s_apple_ios.rs +++ b/src/librustc_target/spec/armv7s_apple_ios.rs @@ -1,4 +1,4 @@ -use super::apple_sdk_base::{opts, Arch, AppleOS}; +use super::apple_sdk_base::{opts, AppleOS, Arch}; use crate::spec::{LinkerFlavor, Target, TargetOptions, TargetResult}; pub fn target() -> TargetResult { diff --git a/src/librustc_target/spec/i386_apple_ios.rs b/src/librustc_target/spec/i386_apple_ios.rs index eb0c68bc7b83e..a121d49769d61 100644 --- a/src/librustc_target/spec/i386_apple_ios.rs +++ b/src/librustc_target/spec/i386_apple_ios.rs @@ -1,4 +1,4 @@ -use super::apple_sdk_base::{opts, Arch, AppleOS}; +use super::apple_sdk_base::{opts, AppleOS, Arch}; use crate::spec::{LinkerFlavor, Target, TargetOptions, TargetResult}; pub fn target() -> TargetResult { diff --git a/src/librustc_target/spec/x86_64_apple_ios.rs b/src/librustc_target/spec/x86_64_apple_ios.rs index db5361f1d813d..cfcf856836b14 100644 --- a/src/librustc_target/spec/x86_64_apple_ios.rs +++ b/src/librustc_target/spec/x86_64_apple_ios.rs @@ -1,4 +1,4 @@ -use super::apple_sdk_base::{opts, Arch, AppleOS}; +use super::apple_sdk_base::{opts, AppleOS, Arch}; use crate::spec::{LinkerFlavor, Target, TargetOptions, TargetResult}; pub fn target() -> TargetResult { diff --git a/src/librustc_target/spec/x86_64_apple_ios_macabi.rs b/src/librustc_target/spec/x86_64_apple_ios_macabi.rs index cce6221134df3..c42d09117259d 100644 --- a/src/librustc_target/spec/x86_64_apple_ios_macabi.rs +++ b/src/librustc_target/spec/x86_64_apple_ios_macabi.rs @@ -1,4 +1,4 @@ -use super::apple_sdk_base::{opts, Arch, AppleOS}; +use super::apple_sdk_base::{opts, AppleOS, Arch}; use crate::spec::{LinkerFlavor, Target, TargetOptions, TargetResult}; pub fn target() -> TargetResult { diff --git a/src/librustc_target/spec/x86_64_apple_tvos.rs b/src/librustc_target/spec/x86_64_apple_tvos.rs index 794df42f43e32..a56062c0b2b5d 100644 --- a/src/librustc_target/spec/x86_64_apple_tvos.rs +++ b/src/librustc_target/spec/x86_64_apple_tvos.rs @@ -1,4 +1,4 @@ -use super::apple_sdk_base::{opts, Arch, AppleOS}; +use super::apple_sdk_base::{opts, AppleOS, Arch}; use crate::spec::{LinkerFlavor, Target, TargetOptions, TargetResult}; pub fn target() -> TargetResult { From 0e1cd5935faf9240850c07afb2b7fd8d582c5e25 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Wed, 4 Mar 2020 08:23:12 -0800 Subject: [PATCH 04/27] Toolstate: remove redundant beta-week check. --- src/bootstrap/toolstate.rs | 39 +++++++++----------------------------- 1 file changed, 9 insertions(+), 30 deletions(-) diff --git a/src/bootstrap/toolstate.rs b/src/bootstrap/toolstate.rs index 7cffc47293070..7b41ba371a83a 100644 --- a/src/bootstrap/toolstate.rs +++ b/src/bootstrap/toolstate.rs @@ -215,6 +215,9 @@ impl Step for ToolStateCheck { tool, old_state, state ); } else { + // This warning only appears in the logs, which most + // people won't read. It's mostly here for testing and + // debugging. eprintln!( "warning: Tool `{}` is not test-pass (is `{}`), \ this should be fixed before beta is branched.", @@ -222,6 +225,8 @@ impl Step for ToolStateCheck { ); } } + // publish_toolstate.py will be responsible for creating + // comments/issues warning people if there is a regression. } } @@ -230,7 +235,7 @@ impl Step for ToolStateCheck { } if builder.config.channel == "nightly" && env::var_os("TOOLSTATE_PUBLISH").is_some() { - commit_toolstate_change(&toolstates, in_beta_week); + commit_toolstate_change(&toolstates); } } @@ -373,14 +378,12 @@ fn read_old_toolstate() -> Vec { /// /// * See /// if a private email by GitHub is wanted. -fn commit_toolstate_change(current_toolstate: &ToolstateData, in_beta_week: bool) { - let old_toolstate = read_old_toolstate(); - +fn commit_toolstate_change(current_toolstate: &ToolstateData) { let message = format!("({} CI update)", OS.expect("linux/windows only")); let mut success = false; for _ in 1..=5 { // Update the toolstate results (the new commit-to-toolstate mapping) in the toolstate repo. - change_toolstate(¤t_toolstate, &old_toolstate, in_beta_week); + change_toolstate(¤t_toolstate); // `git commit` failing means nothing to commit. let status = t!(Command::new("git") @@ -429,31 +432,7 @@ fn commit_toolstate_change(current_toolstate: &ToolstateData, in_beta_week: bool } } -fn change_toolstate( - current_toolstate: &ToolstateData, - old_toolstate: &[RepoState], - in_beta_week: bool, -) { - let mut regressed = false; - for repo_state in old_toolstate { - let tool = &repo_state.tool; - let state = repo_state.state(); - let new_state = current_toolstate[tool.as_str()]; - - if new_state != state { - eprintln!("The state of `{}` has changed from `{}` to `{}`", tool, state, new_state); - if new_state < state { - if !NIGHTLY_TOOLS.iter().any(|(name, _path)| name == tool) { - regressed = true; - } - } - } - } - - if regressed && in_beta_week { - std::process::exit(1); - } - +fn change_toolstate(current_toolstate: &ToolstateData) { let commit = t!(std::process::Command::new("git").arg("rev-parse").arg("HEAD").output()); let commit = t!(String::from_utf8(commit.stdout)); From a6d8c9c5ebea585304df74bdf56cff186ad8ef1a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 4 Mar 2020 10:17:15 +0100 Subject: [PATCH 05/27] more toolstate comments --- src/bootstrap/toolstate.rs | 8 +++++--- src/ci/publish_toolstate.sh | 4 +++- src/tools/publish_toolstate.py | 9 ++++----- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/bootstrap/toolstate.rs b/src/bootstrap/toolstate.rs index 7b41ba371a83a..c8d8a29ac4e16 100644 --- a/src/bootstrap/toolstate.rs +++ b/src/bootstrap/toolstate.rs @@ -330,11 +330,11 @@ fn prepare_toolstate_config(token: &str) { Err(_) => false, }; if !success { - panic!("git config key={} value={} successful (status: {:?})", key, value, status); + panic!("git config key={} value={} failed (status: {:?})", key, value, status); } } - // If changing anything here, then please check that src/ci/publish_toolstate.sh is up to date + // If changing anything here, then please check that `src/ci/publish_toolstate.sh` is up to date // as well. git_config("user.email", "7378925+rust-toolstate-update@users.noreply.github.com"); git_config("user.name", "Rust Toolstate Update"); @@ -382,7 +382,9 @@ fn commit_toolstate_change(current_toolstate: &ToolstateData) { let message = format!("({} CI update)", OS.expect("linux/windows only")); let mut success = false; for _ in 1..=5 { - // Update the toolstate results (the new commit-to-toolstate mapping) in the toolstate repo. + // Upload the test results (the new commit-to-toolstate mapping) to the toolstate repo. + // This does *not* change the "current toolstate"; that only happens post-landing + // via `src/ci/docker/publish_toolstate.sh`. change_toolstate(¤t_toolstate); // `git commit` failing means nothing to commit. diff --git a/src/ci/publish_toolstate.sh b/src/ci/publish_toolstate.sh index 7c43d034d8b7f..691df04e754a6 100755 --- a/src/ci/publish_toolstate.sh +++ b/src/ci/publish_toolstate.sh @@ -23,7 +23,9 @@ GIT_COMMIT_MSG="$(git log --format=%s -n1 HEAD)" cd rust-toolstate FAILURE=1 for RETRY_COUNT in 1 2 3 4 5; do - # The purpose is to publish the new "current" toolstate in the toolstate repo. + # The purpose of this is to publish the new "current" toolstate in the toolstate repo. + # This happens post-landing, on master. + # (Publishing the per-commit test results happens pre-landing in src/bootstrap/toolstate.rs). "$(ciCheckoutPath)/src/tools/publish_toolstate.py" "$GIT_COMMIT" \ "$GIT_COMMIT_MSG" \ "$MESSAGE_FILE" \ diff --git a/src/tools/publish_toolstate.py b/src/tools/publish_toolstate.py index 5fbb986286ade..b389cd0373cc4 100755 --- a/src/tools/publish_toolstate.py +++ b/src/tools/publish_toolstate.py @@ -1,11 +1,10 @@ #!/usr/bin/env python # -*- coding: utf-8 -*- -# This script publishes the new "current" toolstate in the toolstate repo (not to be -# confused with publishing the test results, which happens in -# `src/ci/docker/x86_64-gnu-tools/checktools.sh`). -# It is set as callback for `src/ci/docker/x86_64-gnu-tools/repo.sh` by the CI scripts -# when a new commit lands on `master` (i.e., after it passed all checks on `auto`). +# This script computes the new "current" toolstate for the toolstate repo (not to be +# confused with publishing the test results, which happens in `src/bootstrap/toolstate.rs`). +# It gets called from `src/ci/publish_toolstate.sh` when a new commit lands on `master` +# (i.e., after it passed all checks on `auto`). from __future__ import print_function From a41f1f128ee9fbe595a588b9b91d545a895f5c39 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Wed, 4 Mar 2020 09:49:55 -0800 Subject: [PATCH 06/27] Further clarifications and comments on toolstate operation. --- src/bootstrap/toolstate.rs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/bootstrap/toolstate.rs b/src/bootstrap/toolstate.rs index c8d8a29ac4e16..f0e0f92af55fc 100644 --- a/src/bootstrap/toolstate.rs +++ b/src/bootstrap/toolstate.rs @@ -225,8 +225,11 @@ impl Step for ToolStateCheck { ); } } - // publish_toolstate.py will be responsible for creating - // comments/issues warning people if there is a regression. + // `publish_toolstate.py` is responsible for updating + // `latest.json` and creating comments/issues warning people + // if there is a regression. That all happens in a separate CI + // job on the master branch once the PR has passed all tests + // on the `auto` branch. } } @@ -385,7 +388,7 @@ fn commit_toolstate_change(current_toolstate: &ToolstateData) { // Upload the test results (the new commit-to-toolstate mapping) to the toolstate repo. // This does *not* change the "current toolstate"; that only happens post-landing // via `src/ci/docker/publish_toolstate.sh`. - change_toolstate(¤t_toolstate); + publish_test_results(¤t_toolstate); // `git commit` failing means nothing to commit. let status = t!(Command::new("git") @@ -434,7 +437,12 @@ fn commit_toolstate_change(current_toolstate: &ToolstateData) { } } -fn change_toolstate(current_toolstate: &ToolstateData) { +/// Updates the "history" files with the latest results. +/// +/// These results will later be promoted to `latest.json` by the +/// `publish_toolstate.py` script if the PR passes all tests and is merged to +/// master. +fn publish_test_results(current_toolstate: &ToolstateData) { let commit = t!(std::process::Command::new("git").arg("rev-parse").arg("HEAD").output()); let commit = t!(String::from_utf8(commit.stdout)); From 91525fd078bb6f3ec833aa42141ea027b37b26a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 4 Mar 2020 16:15:23 -0800 Subject: [PATCH 07/27] Tweak output for invalid negative impl AST errors --- src/librustc_ast/ast.rs | 4 +-- src/librustc_ast_passes/ast_validation.rs | 28 ++++++++++++------- src/librustc_ast_passes/feature_gate.rs | 8 +++--- src/librustc_ast_pretty/pprust.rs | 2 +- src/librustc_hir/print.rs | 2 +- src/librustc_parse/parser/item.rs | 2 +- src/librustc_save_analysis/sig.rs | 2 +- src/librustc_typeck/coherence/unsafety.rs | 4 +-- src/librustc_typeck/collect.rs | 2 +- .../coherence-negative-impls-safe.stderr | 4 +-- src/test/ui/error-codes/E0197.stderr | 2 +- src/test/ui/error-codes/E0198.stderr | 4 +-- .../feature-gate-optin-builtin-traits.stderr | 4 +-- .../inherent-impl.stderr | 8 +++--- .../defaultimpl/validation.stderr | 2 +- .../syntax-trait-polarity-feature-gate.stderr | 4 +-- src/test/ui/syntax-trait-polarity.stderr | 16 +++++------ .../traits/trait-safety-inherent-impl.stderr | 10 ++----- 18 files changed, 56 insertions(+), 52 deletions(-) diff --git a/src/librustc_ast/ast.rs b/src/librustc_ast/ast.rs index 88564647d61f9..e86ec2ebfa499 100644 --- a/src/librustc_ast/ast.rs +++ b/src/librustc_ast/ast.rs @@ -2117,14 +2117,14 @@ pub enum ImplPolarity { /// `impl Trait for Type` Positive, /// `impl !Trait for Type` - Negative, + Negative(Span), } impl fmt::Debug for ImplPolarity { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match *self { ImplPolarity::Positive => "positive".fmt(f), - ImplPolarity::Negative => "negative".fmt(f), + ImplPolarity::Negative(_) => "negative".fmt(f), } } } diff --git a/src/librustc_ast_passes/ast_validation.rs b/src/librustc_ast_passes/ast_validation.rs index 9f04c01bfa8f4..1070043458a5a 100644 --- a/src/librustc_ast_passes/ast_validation.rs +++ b/src/librustc_ast_passes/ast_validation.rs @@ -779,7 +779,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> { defaultness: _, constness: _, generics: _, - of_trait: Some(_), + of_trait: Some(ref t), ref self_ty, items: _, } => { @@ -794,10 +794,10 @@ impl<'a> Visitor<'a> for AstValidator<'a> { .help("use `auto trait Trait {}` instead") .emit(); } - if let (Unsafe::Yes(span), ImplPolarity::Negative) = (unsafety, polarity) { + if let (Unsafe::Yes(span), ImplPolarity::Negative(sp)) = (unsafety, polarity) { struct_span_err!( this.session, - item.span, + sp.to(t.path.span), E0198, "negative impls cannot be unsafe" ) @@ -816,7 +816,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> { constness, generics: _, of_trait: None, - self_ty: _, + ref self_ty, items: _, } => { self.invalid_visibility( @@ -826,28 +826,36 @@ impl<'a> Visitor<'a> for AstValidator<'a> { if let Unsafe::Yes(span) = unsafety { struct_span_err!( self.session, - item.span, + vec![span, self_ty.span], E0197, "inherent impls cannot be unsafe" ) .span_label(span, "unsafe because of this") + .span_label(self_ty.span, "inherent impl for this type") .emit(); } - if polarity == ImplPolarity::Negative { - self.err_handler().span_err(item.span, "inherent impls cannot be negative"); + if let ImplPolarity::Negative(span) = polarity { + self.err_handler().span_err(span, "inherent impls cannot be negative"); } if let Defaultness::Default(def_span) = defaultness { - let span = self.session.source_map().def_span(item.span); self.err_handler() - .struct_span_err(span, "inherent impls cannot be `default`") + .struct_span_err( + vec![def_span, self_ty.span], + "inherent impls cannot be `default`", + ) .span_label(def_span, "`default` because of this") + .span_label(self_ty.span, "inherent impl for this type") .note("only trait implementations may be annotated with `default`") .emit(); } if let Const::Yes(span) = constness { self.err_handler() - .struct_span_err(item.span, "inherent impls cannot be `const`") + .struct_span_err( + vec![span, self_ty.span], + "inherent impls cannot be `const`", + ) .span_label(span, "`const` because of this") + .span_label(self_ty.span, "inherent impl for this type") .note("only trait implementations may be annotated with `const`") .emit(); } diff --git a/src/librustc_ast_passes/feature_gate.rs b/src/librustc_ast_passes/feature_gate.rs index 05e69d0cfd74e..3d9001d5d58bb 100644 --- a/src/librustc_ast_passes/feature_gate.rs +++ b/src/librustc_ast_passes/feature_gate.rs @@ -338,14 +338,14 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> { } } - ast::ItemKind::Impl { polarity, defaultness, .. } => { - if polarity == ast::ImplPolarity::Negative { + ast::ItemKind::Impl { polarity, defaultness, ref of_trait, .. } => { + if let ast::ImplPolarity::Negative(span) = polarity { gate_feature_post!( &self, optin_builtin_traits, - i.span, + span.to(of_trait.as_ref().map(|t| t.path.span).unwrap_or(span)), "negative trait bounds are not yet fully implemented; \ - use marker types for now" + use marker types for now" ); } diff --git a/src/librustc_ast_pretty/pprust.rs b/src/librustc_ast_pretty/pprust.rs index f95c154bb3b5a..dbfe5d34bc0cb 100644 --- a/src/librustc_ast_pretty/pprust.rs +++ b/src/librustc_ast_pretty/pprust.rs @@ -1175,7 +1175,7 @@ impl<'a> State<'a> { self.s.space(); } - if polarity == ast::ImplPolarity::Negative { + if let ast::ImplPolarity::Negative(_) = polarity { self.s.word("!"); } diff --git a/src/librustc_hir/print.rs b/src/librustc_hir/print.rs index 8cbbef959ce75..f03ab41f12d0a 100644 --- a/src/librustc_hir/print.rs +++ b/src/librustc_hir/print.rs @@ -652,7 +652,7 @@ impl<'a> State<'a> { self.word_nbsp("const"); } - if let hir::ImplPolarity::Negative = polarity { + if let hir::ImplPolarity::Negative(_) = polarity { self.s.word("!"); } diff --git a/src/librustc_parse/parser/item.rs b/src/librustc_parse/parser/item.rs index 9bca1d0990159..09c512f32546b 100644 --- a/src/librustc_parse/parser/item.rs +++ b/src/librustc_parse/parser/item.rs @@ -414,7 +414,7 @@ impl<'a> Parser<'a> { // Disambiguate `impl !Trait for Type { ... }` and `impl ! { ... }` for the never type. let polarity = if self.check(&token::Not) && self.look_ahead(1, |t| t.can_begin_type()) { self.bump(); // `!` - ast::ImplPolarity::Negative + ast::ImplPolarity::Negative(self.prev_token.span) } else { ast::ImplPolarity::Positive }; diff --git a/src/librustc_save_analysis/sig.rs b/src/librustc_save_analysis/sig.rs index 32da62adc3c02..4fa226712b78e 100644 --- a/src/librustc_save_analysis/sig.rs +++ b/src/librustc_save_analysis/sig.rs @@ -519,7 +519,7 @@ impl Sig for ast::Item { text.push(' '); let trait_sig = if let Some(ref t) = *of_trait { - if polarity == ast::ImplPolarity::Negative { + if let ast::ImplPolarity::Negative(_) = polarity { text.push('!'); } let trait_sig = t.path.make(offset + text.len(), id, scx)?; diff --git a/src/librustc_typeck/coherence/unsafety.rs b/src/librustc_typeck/coherence/unsafety.rs index a604421740363..3b25f67aacc63 100644 --- a/src/librustc_typeck/coherence/unsafety.rs +++ b/src/librustc_typeck/coherence/unsafety.rs @@ -69,11 +69,11 @@ impl UnsafetyChecker<'tcx> { .emit(); } - (_, _, Unsafety::Unsafe, hir::ImplPolarity::Negative) => { + (_, _, Unsafety::Unsafe, hir::ImplPolarity::Negative(_)) => { // Reported in AST validation self.tcx.sess.delay_span_bug(item.span, "unsafe negative impl"); } - (_, _, Unsafety::Normal, hir::ImplPolarity::Negative) + (_, _, Unsafety::Normal, hir::ImplPolarity::Negative(_)) | (Unsafety::Unsafe, _, Unsafety::Unsafe, hir::ImplPolarity::Positive) | (Unsafety::Normal, Some(_), Unsafety::Unsafe, hir::ImplPolarity::Positive) | (Unsafety::Normal, None, Unsafety::Normal, _) => { diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index 2dad3d1d6d708..e6c3d5b8b9e9e 100644 --- a/src/librustc_typeck/collect.rs +++ b/src/librustc_typeck/collect.rs @@ -1548,7 +1548,7 @@ fn impl_polarity(tcx: TyCtxt<'_>, def_id: DefId) -> ty::ImplPolarity { let is_rustc_reservation = tcx.has_attr(def_id, sym::rustc_reservation_impl); let item = tcx.hir().expect_item(hir_id); match &item.kind { - hir::ItemKind::Impl { polarity: hir::ImplPolarity::Negative, .. } => { + hir::ItemKind::Impl { polarity: hir::ImplPolarity::Negative(_), .. } => { if is_rustc_reservation { tcx.sess.span_err(item.span, "reservation impls can't be negative"); } diff --git a/src/test/ui/coherence/coherence-negative-impls-safe.stderr b/src/test/ui/coherence/coherence-negative-impls-safe.stderr index 4db66af6783ca..eebd1de277ecf 100644 --- a/src/test/ui/coherence/coherence-negative-impls-safe.stderr +++ b/src/test/ui/coherence/coherence-negative-impls-safe.stderr @@ -1,8 +1,8 @@ error[E0198]: negative impls cannot be unsafe - --> $DIR/coherence-negative-impls-safe.rs:7:1 + --> $DIR/coherence-negative-impls-safe.rs:7:13 | LL | unsafe impl !Send for TestType {} - | ------^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ------ ^^^^^ | | | unsafe because of this diff --git a/src/test/ui/error-codes/E0197.stderr b/src/test/ui/error-codes/E0197.stderr index 51ed9c83bc999..bc3c2857958f9 100644 --- a/src/test/ui/error-codes/E0197.stderr +++ b/src/test/ui/error-codes/E0197.stderr @@ -2,7 +2,7 @@ error[E0197]: inherent impls cannot be unsafe --> $DIR/E0197.rs:3:1 | LL | unsafe impl Foo { } - | ------^^^^^^^^^^^^^ + | ^^^^^^ ^^^ inherent impl for this type | | | unsafe because of this diff --git a/src/test/ui/error-codes/E0198.stderr b/src/test/ui/error-codes/E0198.stderr index 90e8b4abd1296..4330476870037 100644 --- a/src/test/ui/error-codes/E0198.stderr +++ b/src/test/ui/error-codes/E0198.stderr @@ -1,8 +1,8 @@ error[E0198]: negative impls cannot be unsafe - --> $DIR/E0198.rs:5:1 + --> $DIR/E0198.rs:5:13 | LL | unsafe impl !Send for Foo { } - | ------^^^^^^^^^^^^^^^^^^^^^^^ + | ------ ^^^^^ | | | unsafe because of this diff --git a/src/test/ui/feature-gates/feature-gate-optin-builtin-traits.stderr b/src/test/ui/feature-gates/feature-gate-optin-builtin-traits.stderr index d29c373a33c95..490d29ad8a35f 100644 --- a/src/test/ui/feature-gates/feature-gate-optin-builtin-traits.stderr +++ b/src/test/ui/feature-gates/feature-gate-optin-builtin-traits.stderr @@ -8,10 +8,10 @@ LL | auto trait AutoDummyTrait {} = help: add `#![feature(optin_builtin_traits)]` to the crate attributes to enable error[E0658]: negative trait bounds are not yet fully implemented; use marker types for now - --> $DIR/feature-gate-optin-builtin-traits.rs:9:1 + --> $DIR/feature-gate-optin-builtin-traits.rs:9:6 | LL | impl !AutoDummyTrait for DummyStruct {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^ | = note: see issue #13231 for more information = help: add `#![feature(optin_builtin_traits)]` to the crate attributes to enable diff --git a/src/test/ui/rfc-2632-const-trait-impl/inherent-impl.stderr b/src/test/ui/rfc-2632-const-trait-impl/inherent-impl.stderr index 3ea58a3728a5d..0a76c70b97d1e 100644 --- a/src/test/ui/rfc-2632-const-trait-impl/inherent-impl.stderr +++ b/src/test/ui/rfc-2632-const-trait-impl/inherent-impl.stderr @@ -1,18 +1,18 @@ error: inherent impls cannot be `const` - --> $DIR/inherent-impl.rs:9:1 + --> $DIR/inherent-impl.rs:9:6 | LL | impl const S {} - | ^^^^^-----^^^^^ + | ^^^^^ ^ inherent impl for this type | | | `const` because of this | = note: only trait implementations may be annotated with `const` error: inherent impls cannot be `const` - --> $DIR/inherent-impl.rs:12:1 + --> $DIR/inherent-impl.rs:12:6 | LL | impl const T {} - | ^^^^^-----^^^^^ + | ^^^^^ ^ inherent impl for this type | | | `const` because of this | diff --git a/src/test/ui/specialization/defaultimpl/validation.stderr b/src/test/ui/specialization/defaultimpl/validation.stderr index 03b1ef69ca072..2a96f41a249fa 100644 --- a/src/test/ui/specialization/defaultimpl/validation.stderr +++ b/src/test/ui/specialization/defaultimpl/validation.stderr @@ -2,7 +2,7 @@ error: inherent impls cannot be `default` --> $DIR/validation.rs:7:1 | LL | default impl S {} - | -------^^^^^^^ + | ^^^^^^^ ^ inherent impl for this type | | | `default` because of this | diff --git a/src/test/ui/syntax-trait-polarity-feature-gate.stderr b/src/test/ui/syntax-trait-polarity-feature-gate.stderr index ed76377278b82..5d4c1b354f700 100644 --- a/src/test/ui/syntax-trait-polarity-feature-gate.stderr +++ b/src/test/ui/syntax-trait-polarity-feature-gate.stderr @@ -1,8 +1,8 @@ error[E0658]: negative trait bounds are not yet fully implemented; use marker types for now - --> $DIR/syntax-trait-polarity-feature-gate.rs:7:1 + --> $DIR/syntax-trait-polarity-feature-gate.rs:7:6 | LL | impl !Send for TestType {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^ | = note: see issue #13231 for more information = help: add `#![feature(optin_builtin_traits)]` to the crate attributes to enable diff --git a/src/test/ui/syntax-trait-polarity.stderr b/src/test/ui/syntax-trait-polarity.stderr index fef3a65088855..b7d5b4570aacc 100644 --- a/src/test/ui/syntax-trait-polarity.stderr +++ b/src/test/ui/syntax-trait-polarity.stderr @@ -1,28 +1,28 @@ error: inherent impls cannot be negative - --> $DIR/syntax-trait-polarity.rs:7:1 + --> $DIR/syntax-trait-polarity.rs:7:6 | LL | impl !TestType {} - | ^^^^^^^^^^^^^^^^^ + | ^ error[E0198]: negative impls cannot be unsafe - --> $DIR/syntax-trait-polarity.rs:12:1 + --> $DIR/syntax-trait-polarity.rs:12:13 | LL | unsafe impl !Send for TestType {} - | ------^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ------ ^^^^^ | | | unsafe because of this error: inherent impls cannot be negative - --> $DIR/syntax-trait-polarity.rs:19:1 + --> $DIR/syntax-trait-polarity.rs:19:9 | LL | impl !TestType2 {} - | ^^^^^^^^^^^^^^^^^^^^^^^^ + | ^ error[E0198]: negative impls cannot be unsafe - --> $DIR/syntax-trait-polarity.rs:22:1 + --> $DIR/syntax-trait-polarity.rs:22:16 | LL | unsafe impl !Send for TestType2 {} - | ------^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ------ ^^^^^ | | | unsafe because of this diff --git a/src/test/ui/traits/trait-safety-inherent-impl.stderr b/src/test/ui/traits/trait-safety-inherent-impl.stderr index c398785d3949e..09ad4585ab156 100644 --- a/src/test/ui/traits/trait-safety-inherent-impl.stderr +++ b/src/test/ui/traits/trait-safety-inherent-impl.stderr @@ -1,14 +1,10 @@ error[E0197]: inherent impls cannot be unsafe --> $DIR/trait-safety-inherent-impl.rs:5:1 | -LL | unsafe impl SomeStruct { - | ^----- - | | - | _unsafe because of this +LL | unsafe impl SomeStruct { + | ^^^^^^ ^^^^^^^^^^ inherent impl for this type | | -LL | | fn foo(self) { } -LL | | } - | |_^ + | unsafe because of this error: aborting due to previous error From 713a291441d2c71e74141ddc9387166bd6755d9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 5 Mar 2020 15:39:35 -0800 Subject: [PATCH 08/27] review comments --- src/librustc_ast_passes/ast_validation.rs | 52 +++++++++---------- src/librustc_parse/parser/item.rs | 18 ++++--- .../coherence-negative-impls-safe.stderr | 5 +- src/test/ui/error-codes/E0197.stderr | 4 +- src/test/ui/error-codes/E0198.stderr | 5 +- .../inherent-impl.stderr | 8 +-- .../defaultimpl/validation.stderr | 4 +- src/test/ui/syntax-trait-polarity.stderr | 22 +++++--- .../traits/trait-safety-inherent-impl.stderr | 4 +- 9 files changed, 65 insertions(+), 57 deletions(-) diff --git a/src/librustc_ast_passes/ast_validation.rs b/src/librustc_ast_passes/ast_validation.rs index 1070043458a5a..d90e9c4df269c 100644 --- a/src/librustc_ast_passes/ast_validation.rs +++ b/src/librustc_ast_passes/ast_validation.rs @@ -801,6 +801,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> { E0198, "negative impls cannot be unsafe" ) + .span_label(sp, "negative because of this") .span_label(span, "unsafe because of this") .emit(); } @@ -819,45 +820,40 @@ impl<'a> Visitor<'a> for AstValidator<'a> { ref self_ty, items: _, } => { + let error = |annotation_span, annotation, note, code| { + let mut err = self.err_handler().struct_span_err( + self_ty.span, + &format!("inherent impls cannot be {}", annotation), + ); + err.span_label(annotation_span, &format!("{} because of this", annotation)); + err.span_label(self_ty.span, "inherent impl for this type"); + if note { + err.note(&format!( + "only trait implementations may be annotated with {}", + annotation + )); + } + if code { + err.code(error_code!(E0197)); + } + err.emit(); + }; + self.invalid_visibility( &item.vis, Some("place qualifiers on individual impl items instead"), ); if let Unsafe::Yes(span) = unsafety { - struct_span_err!( - self.session, - vec![span, self_ty.span], - E0197, - "inherent impls cannot be unsafe" - ) - .span_label(span, "unsafe because of this") - .span_label(self_ty.span, "inherent impl for this type") - .emit(); + error(span, "unsafe", false, true) } if let ImplPolarity::Negative(span) = polarity { - self.err_handler().span_err(span, "inherent impls cannot be negative"); + error(span, "negative", false, false); } if let Defaultness::Default(def_span) = defaultness { - self.err_handler() - .struct_span_err( - vec![def_span, self_ty.span], - "inherent impls cannot be `default`", - ) - .span_label(def_span, "`default` because of this") - .span_label(self_ty.span, "inherent impl for this type") - .note("only trait implementations may be annotated with `default`") - .emit(); + error(def_span, "`default`", true, false); } if let Const::Yes(span) = constness { - self.err_handler() - .struct_span_err( - vec![span, self_ty.span], - "inherent impls cannot be `const`", - ) - .span_label(span, "`const` because of this") - .span_label(self_ty.span, "inherent impl for this type") - .note("only trait implementations may be annotated with `const`") - .emit(); + error(span, "`const`", true, false); } } ItemKind::Fn(def, ref sig, ref generics, ref body) => { diff --git a/src/librustc_parse/parser/item.rs b/src/librustc_parse/parser/item.rs index 09c512f32546b..85bb546b74806 100644 --- a/src/librustc_parse/parser/item.rs +++ b/src/librustc_parse/parser/item.rs @@ -373,6 +373,16 @@ impl<'a> Parser<'a> { self.token.is_keyword(kw::Async) && self.is_keyword_ahead(1, &[kw::Fn]) } + fn parse_polarity(&mut self) -> ast::ImplPolarity { + // Disambiguate `impl !Trait for Type { ... }` and `impl ! { ... }` for the never type. + if self.check(&token::Not) && self.look_ahead(1, |t| t.can_begin_type()) { + self.bump(); // `!` + ast::ImplPolarity::Negative(self.prev_token.span) + } else { + ast::ImplPolarity::Positive + } + } + /// Parses an implementation item. /// /// ``` @@ -411,13 +421,7 @@ impl<'a> Parser<'a> { self.sess.gated_spans.gate(sym::const_trait_impl, span); } - // Disambiguate `impl !Trait for Type { ... }` and `impl ! { ... }` for the never type. - let polarity = if self.check(&token::Not) && self.look_ahead(1, |t| t.can_begin_type()) { - self.bump(); // `!` - ast::ImplPolarity::Negative(self.prev_token.span) - } else { - ast::ImplPolarity::Positive - }; + let polarity = self.parse_polarity(); // Parse both types and traits as a type, then reinterpret if necessary. let err_path = |span| ast::Path::from_ident(Ident::new(kw::Invalid, span)); diff --git a/src/test/ui/coherence/coherence-negative-impls-safe.stderr b/src/test/ui/coherence/coherence-negative-impls-safe.stderr index eebd1de277ecf..1bd37f395902a 100644 --- a/src/test/ui/coherence/coherence-negative-impls-safe.stderr +++ b/src/test/ui/coherence/coherence-negative-impls-safe.stderr @@ -2,8 +2,9 @@ error[E0198]: negative impls cannot be unsafe --> $DIR/coherence-negative-impls-safe.rs:7:13 | LL | unsafe impl !Send for TestType {} - | ------ ^^^^^ - | | + | ------ -^^^^ + | | | + | | negative because of this | unsafe because of this error: aborting due to previous error diff --git a/src/test/ui/error-codes/E0197.stderr b/src/test/ui/error-codes/E0197.stderr index bc3c2857958f9..35e1042649ef9 100644 --- a/src/test/ui/error-codes/E0197.stderr +++ b/src/test/ui/error-codes/E0197.stderr @@ -1,8 +1,8 @@ error[E0197]: inherent impls cannot be unsafe - --> $DIR/E0197.rs:3:1 + --> $DIR/E0197.rs:3:13 | LL | unsafe impl Foo { } - | ^^^^^^ ^^^ inherent impl for this type + | ------ ^^^ inherent impl for this type | | | unsafe because of this diff --git a/src/test/ui/error-codes/E0198.stderr b/src/test/ui/error-codes/E0198.stderr index 4330476870037..bb2efefb427ba 100644 --- a/src/test/ui/error-codes/E0198.stderr +++ b/src/test/ui/error-codes/E0198.stderr @@ -2,8 +2,9 @@ error[E0198]: negative impls cannot be unsafe --> $DIR/E0198.rs:5:13 | LL | unsafe impl !Send for Foo { } - | ------ ^^^^^ - | | + | ------ -^^^^ + | | | + | | negative because of this | unsafe because of this error: aborting due to previous error diff --git a/src/test/ui/rfc-2632-const-trait-impl/inherent-impl.stderr b/src/test/ui/rfc-2632-const-trait-impl/inherent-impl.stderr index 0a76c70b97d1e..834f6a409f5b6 100644 --- a/src/test/ui/rfc-2632-const-trait-impl/inherent-impl.stderr +++ b/src/test/ui/rfc-2632-const-trait-impl/inherent-impl.stderr @@ -1,18 +1,18 @@ error: inherent impls cannot be `const` - --> $DIR/inherent-impl.rs:9:6 + --> $DIR/inherent-impl.rs:9:12 | LL | impl const S {} - | ^^^^^ ^ inherent impl for this type + | ----- ^ inherent impl for this type | | | `const` because of this | = note: only trait implementations may be annotated with `const` error: inherent impls cannot be `const` - --> $DIR/inherent-impl.rs:12:6 + --> $DIR/inherent-impl.rs:12:12 | LL | impl const T {} - | ^^^^^ ^ inherent impl for this type + | ----- ^ inherent impl for this type | | | `const` because of this | diff --git a/src/test/ui/specialization/defaultimpl/validation.stderr b/src/test/ui/specialization/defaultimpl/validation.stderr index 2a96f41a249fa..6e19d79e48f6b 100644 --- a/src/test/ui/specialization/defaultimpl/validation.stderr +++ b/src/test/ui/specialization/defaultimpl/validation.stderr @@ -1,8 +1,8 @@ error: inherent impls cannot be `default` - --> $DIR/validation.rs:7:1 + --> $DIR/validation.rs:7:14 | LL | default impl S {} - | ^^^^^^^ ^ inherent impl for this type + | ------- ^ inherent impl for this type | | | `default` because of this | diff --git a/src/test/ui/syntax-trait-polarity.stderr b/src/test/ui/syntax-trait-polarity.stderr index b7d5b4570aacc..5777e0ade908e 100644 --- a/src/test/ui/syntax-trait-polarity.stderr +++ b/src/test/ui/syntax-trait-polarity.stderr @@ -1,29 +1,35 @@ error: inherent impls cannot be negative - --> $DIR/syntax-trait-polarity.rs:7:6 + --> $DIR/syntax-trait-polarity.rs:7:7 | LL | impl !TestType {} - | ^ + | -^^^^^^^^ inherent impl for this type + | | + | negative because of this error[E0198]: negative impls cannot be unsafe --> $DIR/syntax-trait-polarity.rs:12:13 | LL | unsafe impl !Send for TestType {} - | ------ ^^^^^ - | | + | ------ -^^^^ + | | | + | | negative because of this | unsafe because of this error: inherent impls cannot be negative - --> $DIR/syntax-trait-polarity.rs:19:9 + --> $DIR/syntax-trait-polarity.rs:19:10 | LL | impl !TestType2 {} - | ^ + | -^^^^^^^^^^^^ inherent impl for this type + | | + | negative because of this error[E0198]: negative impls cannot be unsafe --> $DIR/syntax-trait-polarity.rs:22:16 | LL | unsafe impl !Send for TestType2 {} - | ------ ^^^^^ - | | + | ------ -^^^^ + | | | + | | negative because of this | unsafe because of this error[E0192]: negative impls are only allowed for auto traits (e.g., `Send` and `Sync`) diff --git a/src/test/ui/traits/trait-safety-inherent-impl.stderr b/src/test/ui/traits/trait-safety-inherent-impl.stderr index 09ad4585ab156..0738d2973e2d7 100644 --- a/src/test/ui/traits/trait-safety-inherent-impl.stderr +++ b/src/test/ui/traits/trait-safety-inherent-impl.stderr @@ -1,8 +1,8 @@ error[E0197]: inherent impls cannot be unsafe - --> $DIR/trait-safety-inherent-impl.rs:5:1 + --> $DIR/trait-safety-inherent-impl.rs:5:13 | LL | unsafe impl SomeStruct { - | ^^^^^^ ^^^^^^^^^^ inherent impl for this type + | ------ ^^^^^^^^^^ inherent impl for this type | | | unsafe because of this From 3d146a3d1a58d35fb9198472ab2ccfb5eef9d1c9 Mon Sep 17 00:00:00 2001 From: Phoebe Bell Date: Sun, 9 Feb 2020 16:45:29 -0800 Subject: [PATCH 09/27] Document unsafe blocks in core::fmt --- src/libcore/fmt/float.rs | 6 ++++-- src/libcore/fmt/mod.rs | 18 ++++++++++++++++-- src/libcore/fmt/num.rs | 27 +++++++++++++++++++++++++-- 3 files changed, 45 insertions(+), 6 deletions(-) diff --git a/src/libcore/fmt/float.rs b/src/libcore/fmt/float.rs index 5ef673009bb6d..52d8349bc9a87 100644 --- a/src/libcore/fmt/float.rs +++ b/src/libcore/fmt/float.rs @@ -2,8 +2,6 @@ use crate::fmt::{Debug, Display, Formatter, LowerExp, Result, UpperExp}; use crate::mem::MaybeUninit; use crate::num::flt2dec; -// ignore-tidy-undocumented-unsafe - // Don't inline this so callers don't use the stack space this function // requires unless they have to. #[inline(never)] @@ -16,6 +14,7 @@ fn float_to_decimal_common_exact( where T: flt2dec::DecodableFloat, { + // SAFETY: Possible undefined behavior, see FIXME(#53491) unsafe { let mut buf = MaybeUninit::<[u8; 1024]>::uninit(); // enough for f32 and f64 let mut parts = MaybeUninit::<[flt2dec::Part<'_>; 4]>::uninit(); @@ -48,6 +47,7 @@ fn float_to_decimal_common_shortest( where T: flt2dec::DecodableFloat, { + // SAFETY: Possible undefined behavior, see FIXME(#53491) unsafe { // enough for f32 and f64 let mut buf = MaybeUninit::<[u8; flt2dec::MAX_SIG_DIGITS]>::uninit(); @@ -103,6 +103,7 @@ fn float_to_exponential_common_exact( where T: flt2dec::DecodableFloat, { + // SAFETY: Possible undefined behavior, see FIXME(#53491) unsafe { let mut buf = MaybeUninit::<[u8; 1024]>::uninit(); // enough for f32 and f64 let mut parts = MaybeUninit::<[flt2dec::Part<'_>; 6]>::uninit(); @@ -132,6 +133,7 @@ fn float_to_exponential_common_shortest( where T: flt2dec::DecodableFloat, { + // SAFETY: Possible undefined behavior, see FIXME(#53491) unsafe { // enough for f32 and f64 let mut buf = MaybeUninit::<[u8; flt2dec::MAX_SIG_DIGITS]>::uninit(); diff --git a/src/libcore/fmt/mod.rs b/src/libcore/fmt/mod.rs index 993b1073493e9..a7b34e5f2f1c5 100644 --- a/src/libcore/fmt/mod.rs +++ b/src/libcore/fmt/mod.rs @@ -1,7 +1,5 @@ //! Utilities for formatting and printing strings. -// ignore-tidy-undocumented-unsafe - #![stable(feature = "rust1", since = "1.0.0")] use crate::cell::{Cell, Ref, RefCell, RefMut, UnsafeCell}; @@ -271,6 +269,14 @@ impl<'a> ArgumentV1<'a> { #[doc(hidden)] #[unstable(feature = "fmt_internals", reason = "internal to format_args!", issue = "none")] pub fn new<'b, T>(x: &'b T, f: fn(&T, &mut Formatter<'_>) -> Result) -> ArgumentV1<'b> { + // SAFETY: `mem::transmute(x)` is safe because + // 1. `&'b T` keeps the lifetime it originated with `'b` + // (so as to not have an unbounded lifetime) + // 2. `&'b T` and `&'b Void` have the same memory layout + // (when `T` is `Sized`, as it is here) + // `mem::transmute(f)` is safe since `fn(&T, &mut Formatter<'_>) -> Result` + // and `fn(&Void, &mut Formatter<'_>) -> Result` have the same ABI + // (as long as `T` is `Sized`) unsafe { ArgumentV1 { formatter: mem::transmute(f), value: mem::transmute(x) } } } @@ -1389,6 +1395,14 @@ impl<'a> Formatter<'a> { fn write_formatted_parts(&mut self, formatted: &flt2dec::Formatted<'_>) -> Result { fn write_bytes(buf: &mut dyn Write, s: &[u8]) -> Result { + // SAFETY: This is used for `flt2dec::Part::Num` and `flt2dec::Part::Copy`. + // It's safe to use for `flt2dec::Part::Num` since every char `c` is between + // `b'0'` and `b'9'`, which means `s` is valid UTF-8. + // It's also probably safe in practice to use for `flt2dec::Part::Copy(buf)` + // since `buf` should be plain ASCII, but it's possible for someone to pass + // in a bad value for `buf` into `flt2dec::to_shortest_str` since it is a + // public function. + // FIXME: Determine whether this could result in UB. buf.write_str(unsafe { str::from_utf8_unchecked(s) }) } diff --git a/src/libcore/fmt/num.rs b/src/libcore/fmt/num.rs index 5dfd3a8ecdbd6..7d77e33d74378 100644 --- a/src/libcore/fmt/num.rs +++ b/src/libcore/fmt/num.rs @@ -1,7 +1,5 @@ //! Integer and floating-point number formatting -// ignore-tidy-undocumented-unsafe - use crate::fmt; use crate::mem::MaybeUninit; use crate::num::flt2dec; @@ -84,6 +82,8 @@ trait GenericRadix { } } let buf = &buf[curr..]; + // SAFETY: The only chars in `buf` are created by `Self::digit` which are assumed to be + // valid UTF-8 let buf = unsafe { str::from_utf8_unchecked(slice::from_raw_parts(MaybeUninit::first_ptr(buf), buf.len())) }; @@ -189,11 +189,19 @@ static DEC_DIGITS_LUT: &[u8; 200] = b"0001020304050607080910111213141516171819\ macro_rules! impl_Display { ($($t:ident),* as $u:ident via $conv_fn:ident named $name:ident) => { fn $name(mut n: $u, is_nonnegative: bool, f: &mut fmt::Formatter<'_>) -> fmt::Result { + // 2^128 is about 3*10^38, so 39 gives an extra byte of space let mut buf = [MaybeUninit::::uninit(); 39]; let mut curr = buf.len() as isize; let buf_ptr = MaybeUninit::first_ptr_mut(&mut buf); let lut_ptr = DEC_DIGITS_LUT.as_ptr(); + // SAFETY: Since `d1` and `d2` are always less than or equal to `198`, we + // can copy from `lut_ptr[d1..d1 + 1]` and `lut_ptr[d2..d2 + 1]`. To show + // that it's OK to copy into `buf_ptr`, notice that at the beginning + // `curr == buf.len() == 39 > log(n)` since `n < 2^128 < 10^39`, and at + // each step this is kept the same as `n` is divided. Since `n` is always + // non-negative, this means that `curr > 0` so `buf_ptr[curr..curr + 1]` + // is safe to access. unsafe { // need at least 16 bits for the 4-characters-at-a-time to work. assert!(crate::mem::size_of::<$u>() >= 2); @@ -206,6 +214,10 @@ macro_rules! impl_Display { let d1 = (rem / 100) << 1; let d2 = (rem % 100) << 1; curr -= 4; + + // We are allowed to copy to `buf_ptr[curr..curr + 3]` here since + // otherwise `curr < 0`. But then `n` was originally at least `10000^10` + // which is `10^40 > 2^128 > n`. ptr::copy_nonoverlapping(lut_ptr.offset(d1), buf_ptr.offset(curr), 2); ptr::copy_nonoverlapping(lut_ptr.offset(d2), buf_ptr.offset(curr + 2), 2); } @@ -232,6 +244,8 @@ macro_rules! impl_Display { } } + // SAFETY: `curr` > 0 (since we made `buf` large enough), and all the chars are valid + // UTF-8 since `DEC_DIGITS_LUT` is let buf_slice = unsafe { str::from_utf8_unchecked( slice::from_raw_parts(buf_ptr.offset(curr), buf.len() - curr as usize)) @@ -304,6 +318,8 @@ macro_rules! impl_Exp { }; // 39 digits (worst case u128) + . = 40 + // Since `curr` always decreases by the number of digits copied, this means + // that `curr >= 0`. let mut buf = [MaybeUninit::::uninit(); 40]; let mut curr = buf.len() as isize; //index for buf let buf_ptr = MaybeUninit::first_ptr_mut(&mut buf); @@ -313,6 +329,8 @@ macro_rules! impl_Exp { while n >= 100 { let d1 = ((n % 100) as isize) << 1; curr -= 2; + // SAFETY: `d1 <= 198`, so we can copy from `lut_ptr[d1..d1 + 2]` since + // `DEC_DIGITS_LUT` has a length of 200. unsafe { ptr::copy_nonoverlapping(lut_ptr.offset(d1), buf_ptr.offset(curr), 2); } @@ -324,6 +342,7 @@ macro_rules! impl_Exp { // decode second-to-last character if n >= 10 { curr -= 1; + // SAFETY: Safe since `40 > curr >= 0` (see comment) unsafe { *buf_ptr.offset(curr) = (n as u8 % 10_u8) + b'0'; } @@ -333,11 +352,13 @@ macro_rules! impl_Exp { // add decimal point iff >1 mantissa digit will be printed if exponent != trailing_zeros || added_precision != 0 { curr -= 1; + // SAFETY: Safe since `40 > curr >= 0` unsafe { *buf_ptr.offset(curr) = b'.'; } } + // SAFETY: Safe since `40 > curr >= 0` let buf_slice = unsafe { // decode last character curr -= 1; @@ -350,6 +371,8 @@ macro_rules! impl_Exp { // stores 'e' (or 'E') and the up to 2-digit exponent let mut exp_buf = [MaybeUninit::::uninit(); 3]; let exp_ptr = MaybeUninit::first_ptr_mut(&mut exp_buf); + // SAFETY: In either case, `exp_buf` is written within bounds and `exp_ptr[..len]` + // is contained within `exp_buf` since `len <= 3`. let exp_slice = unsafe { *exp_ptr.offset(0) = if upper {b'E'} else {b'e'}; let len = if exponent < 10 { From 6fba412499e854d6c4cd8e6b22073d5684d549ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 6 Mar 2020 10:55:21 -0800 Subject: [PATCH 10/27] Further tweak spans in ast validation errors --- src/librustc_ast_passes/ast_validation.rs | 59 +++++++++++++++---- src/test/ui/async-await/no-const-async.stderr | 4 +- src/test/ui/auto-trait-validation.stderr | 18 ++++-- src/test/ui/issues/issue-23080-2.rs | 3 +- src/test/ui/issues/issue-23080-2.stderr | 11 ++-- src/test/ui/issues/issue-23080.rs | 3 +- src/test/ui/issues/issue-23080.stderr | 13 ++-- .../ui/parser/fn-header-semantic-fail.stderr | 10 ++-- ...inductive-overflow-supertrait-oibit.stderr | 6 +- .../typeck-auto-trait-no-supertraits-2.stderr | 6 +- .../typeck-auto-trait-no-supertraits.stderr | 6 +- 11 files changed, 91 insertions(+), 48 deletions(-) diff --git a/src/librustc_ast_passes/ast_validation.rs b/src/librustc_ast_passes/ast_validation.rs index d90e9c4df269c..eff58ad4d9aab 100644 --- a/src/librustc_ast_passes/ast_validation.rs +++ b/src/librustc_ast_passes/ast_validation.rs @@ -13,7 +13,7 @@ use rustc_ast::visit::{self, AssocCtxt, FnCtxt, FnKind, Visitor}; use rustc_ast::walk_list; use rustc_ast_pretty::pprust; use rustc_data_structures::fx::FxHashMap; -use rustc_errors::{error_code, struct_span_err, Applicability}; +use rustc_errors::{error_code, pluralize, struct_span_err, Applicability}; use rustc_parse::validate_attr; use rustc_session::lint::builtin::PATTERNS_IN_FNS_WITHOUT_BODY; use rustc_session::lint::LintBuffer; @@ -887,30 +887,63 @@ impl<'a> Visitor<'a> for AstValidator<'a> { if is_auto == IsAuto::Yes { // Auto traits cannot have generics, super traits nor contain items. if !generics.params.is_empty() { - struct_span_err!( + let spans: Vec<_> = generics.params.iter().map(|i| i.ident.span).collect(); + let last = spans.iter().last().map(|s| *s); + let len = spans.len(); + let mut err = struct_span_err!( self.session, - item.span, + spans, E0567, "auto traits cannot have generic parameters" - ) - .emit(); + ); + if let Some(span) = last { + err.span_label( + span, + &format!( + "cannot have {these} generic parameter{s}", + these = if len == 1 { "this" } else { "these" }, + s = pluralize!(len) + ), + ); + } + err.span_label( + item.ident.span, + "auto trait cannot have generic parameters", + ); + err.emit(); } if !bounds.is_empty() { - struct_span_err!( + let spans: Vec<_> = bounds.iter().map(|b| b.span()).collect(); + let last = spans.iter().last().map(|s| *s); + let len = spans.len(); + let mut err = struct_span_err!( self.session, - item.span, + spans, E0568, "auto traits cannot have super traits" - ) - .emit(); + ); + if let Some(span) = last { + err.span_label( + span, + &format!( + "cannot have {these} super trait{s}", + these = if len == 1 { "this" } else { "these" }, + s = pluralize!(len) + ), + ); + } + err.span_label(item.ident.span, "auto trait cannot have super traits"); + err.emit(); } if !trait_items.is_empty() { + let spans: Vec<_> = trait_items.iter().map(|i| i.ident.span).collect(); struct_span_err!( self.session, - item.span, + spans, E0380, "auto traits cannot have methods or associated items" ) + .span_label(item.ident.span, "auto trait cannot have items") .emit(); } } @@ -1157,9 +1190,13 @@ impl<'a> Visitor<'a> for AstValidator<'a> { }) = fk.header() { self.err_handler() - .struct_span_err(span, "functions cannot be both `const` and `async`") + .struct_span_err( + vec![*cspan, *aspan], + "functions cannot be both `const` and `async`", + ) .span_label(*cspan, "`const` because of this") .span_label(*aspan, "`async` because of this") + .span_label(span, "") // Point at the fn header. .emit(); } diff --git a/src/test/ui/async-await/no-const-async.stderr b/src/test/ui/async-await/no-const-async.stderr index 07559cd240bb6..4e59bb507676f 100644 --- a/src/test/ui/async-await/no-const-async.stderr +++ b/src/test/ui/async-await/no-const-async.stderr @@ -1,8 +1,8 @@ error: functions cannot be both `const` and `async` - --> $DIR/no-const-async.rs:4:1 + --> $DIR/no-const-async.rs:4:5 | LL | pub const async fn x() {} - | ^^^^-----^-----^^^^^^^^^^ + | ----^^^^^-^^^^^---------- | | | | | `async` because of this | `const` because of this diff --git a/src/test/ui/auto-trait-validation.stderr b/src/test/ui/auto-trait-validation.stderr index 51422fab81fda..75919a1f9913f 100644 --- a/src/test/ui/auto-trait-validation.stderr +++ b/src/test/ui/auto-trait-validation.stderr @@ -1,20 +1,26 @@ error[E0567]: auto traits cannot have generic parameters - --> $DIR/auto-trait-validation.rs:3:1 + --> $DIR/auto-trait-validation.rs:3:20 | LL | auto trait Generic {} - | ^^^^^^^^^^^^^^^^^^^^^^^^ + | ------- ^ cannot have this generic parameter + | | + | auto trait cannot have generic parameters error[E0568]: auto traits cannot have super traits - --> $DIR/auto-trait-validation.rs:5:1 + --> $DIR/auto-trait-validation.rs:5:20 | LL | auto trait Bound : Copy {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ----- ^^^^ cannot have this super trait + | | + | auto trait cannot have super traits error[E0380]: auto traits cannot have methods or associated items - --> $DIR/auto-trait-validation.rs:7:1 + --> $DIR/auto-trait-validation.rs:7:25 | LL | auto trait MyTrait { fn foo() {} } - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ------- ^^^ + | | + | auto trait cannot have items error: aborting due to 3 previous errors diff --git a/src/test/ui/issues/issue-23080-2.rs b/src/test/ui/issues/issue-23080-2.rs index 319aa2a5cce9e..d20bb4bd90726 100644 --- a/src/test/ui/issues/issue-23080-2.rs +++ b/src/test/ui/issues/issue-23080-2.rs @@ -3,8 +3,7 @@ #![feature(optin_builtin_traits)] unsafe auto trait Trait { -//~^ ERROR E0380 - type Output; + type Output; //~ ERROR E0380 } fn call_method(x: T) {} diff --git a/src/test/ui/issues/issue-23080-2.stderr b/src/test/ui/issues/issue-23080-2.stderr index 1103de0d91043..fcd1ecfa98288 100644 --- a/src/test/ui/issues/issue-23080-2.stderr +++ b/src/test/ui/issues/issue-23080-2.stderr @@ -1,11 +1,10 @@ error[E0380]: auto traits cannot have methods or associated items - --> $DIR/issue-23080-2.rs:5:1 + --> $DIR/issue-23080-2.rs:6:10 | -LL | / unsafe auto trait Trait { -LL | | -LL | | type Output; -LL | | } - | |_^ +LL | unsafe auto trait Trait { + | ----- auto trait cannot have items +LL | type Output; + | ^^^^^^ error[E0275]: overflow evaluating the requirement `<() as Trait>::Output` | diff --git a/src/test/ui/issues/issue-23080.rs b/src/test/ui/issues/issue-23080.rs index fdfee6981447d..fa5c35316bc28 100644 --- a/src/test/ui/issues/issue-23080.rs +++ b/src/test/ui/issues/issue-23080.rs @@ -1,8 +1,7 @@ #![feature(optin_builtin_traits)] unsafe auto trait Trait { -//~^ ERROR E0380 - fn method(&self) { + fn method(&self) { //~ ERROR E0380 println!("Hello"); } } diff --git a/src/test/ui/issues/issue-23080.stderr b/src/test/ui/issues/issue-23080.stderr index 91c2721732426..dbb9861b5784a 100644 --- a/src/test/ui/issues/issue-23080.stderr +++ b/src/test/ui/issues/issue-23080.stderr @@ -1,13 +1,10 @@ error[E0380]: auto traits cannot have methods or associated items - --> $DIR/issue-23080.rs:3:1 + --> $DIR/issue-23080.rs:4:8 | -LL | / unsafe auto trait Trait { -LL | | -LL | | fn method(&self) { -LL | | println!("Hello"); -LL | | } -LL | | } - | |_^ +LL | unsafe auto trait Trait { + | ----- auto trait cannot have items +LL | fn method(&self) { + | ^^^^^^ error: aborting due to previous error diff --git a/src/test/ui/parser/fn-header-semantic-fail.stderr b/src/test/ui/parser/fn-header-semantic-fail.stderr index 1142cee9851b0..d6b36fbb71450 100644 --- a/src/test/ui/parser/fn-header-semantic-fail.stderr +++ b/src/test/ui/parser/fn-header-semantic-fail.stderr @@ -2,7 +2,7 @@ error: functions cannot be both `const` and `async` --> $DIR/fn-header-semantic-fail.rs:13:5 | LL | const async unsafe extern "C" fn ff5() {} // OK. - | -----^-----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^-^^^^^------------------------------ | | | | | `async` because of this | `const` because of this @@ -45,7 +45,7 @@ error: functions cannot be both `const` and `async` --> $DIR/fn-header-semantic-fail.rs:21:9 | LL | const async unsafe extern "C" fn ft5(); - | -----^-----^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^-^^^^^---------------------------- | | | | | `async` because of this | `const` because of this @@ -88,7 +88,7 @@ error: functions cannot be both `const` and `async` --> $DIR/fn-header-semantic-fail.rs:34:9 | LL | const async unsafe extern "C" fn ft5() {} - | -----^-----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^-^^^^^------------------------------ | | | | | `async` because of this | `const` because of this @@ -97,7 +97,7 @@ error: functions cannot be both `const` and `async` --> $DIR/fn-header-semantic-fail.rs:46:9 | LL | const async unsafe extern "C" fn fi5() {} - | -----^-----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^-^^^^^------------------------------ | | | | | `async` because of this | `const` because of this @@ -160,7 +160,7 @@ error: functions cannot be both `const` and `async` --> $DIR/fn-header-semantic-fail.rs:55:9 | LL | const async unsafe extern "C" fn fe5(); - | -----^-----^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^-^^^^^---------------------------- | | | | | `async` because of this | `const` because of this diff --git a/src/test/ui/traits/traits-inductive-overflow-supertrait-oibit.stderr b/src/test/ui/traits/traits-inductive-overflow-supertrait-oibit.stderr index 63182a6bd9581..03d05f4c928a6 100644 --- a/src/test/ui/traits/traits-inductive-overflow-supertrait-oibit.stderr +++ b/src/test/ui/traits/traits-inductive-overflow-supertrait-oibit.stderr @@ -1,8 +1,10 @@ error[E0568]: auto traits cannot have super traits - --> $DIR/traits-inductive-overflow-supertrait-oibit.rs:7:1 + --> $DIR/traits-inductive-overflow-supertrait-oibit.rs:7:19 | LL | auto trait Magic: Copy {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | ----- ^^^^ cannot have this super trait + | | + | auto trait cannot have super traits error[E0277]: the trait bound `NoClone: std::marker::Copy` is not satisfied --> $DIR/traits-inductive-overflow-supertrait-oibit.rs:15:23 diff --git a/src/test/ui/typeck/typeck-auto-trait-no-supertraits-2.stderr b/src/test/ui/typeck/typeck-auto-trait-no-supertraits-2.stderr index 8755bcded9d2f..45c95b191bd59 100644 --- a/src/test/ui/typeck/typeck-auto-trait-no-supertraits-2.stderr +++ b/src/test/ui/typeck/typeck-auto-trait-no-supertraits-2.stderr @@ -1,8 +1,10 @@ error[E0568]: auto traits cannot have super traits - --> $DIR/typeck-auto-trait-no-supertraits-2.rs:3:1 + --> $DIR/typeck-auto-trait-no-supertraits-2.rs:3:20 | LL | auto trait Magic : Sized where Option : Magic {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ----- ^^^^^ cannot have this super trait + | | + | auto trait cannot have super traits error: aborting due to previous error diff --git a/src/test/ui/typeck/typeck-auto-trait-no-supertraits.stderr b/src/test/ui/typeck/typeck-auto-trait-no-supertraits.stderr index 5a38883490959..094f5d7dd24f2 100644 --- a/src/test/ui/typeck/typeck-auto-trait-no-supertraits.stderr +++ b/src/test/ui/typeck/typeck-auto-trait-no-supertraits.stderr @@ -1,8 +1,10 @@ error[E0568]: auto traits cannot have super traits - --> $DIR/typeck-auto-trait-no-supertraits.rs:27:1 + --> $DIR/typeck-auto-trait-no-supertraits.rs:27:19 | LL | auto trait Magic: Copy {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | ----- ^^^^ cannot have this super trait + | | + | auto trait cannot have super traits error: aborting due to previous error From 005fc6eaccacd129e96e049a3342d1d539316433 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 6 Mar 2020 11:45:00 -0800 Subject: [PATCH 11/27] Suggest removal of auto trait super traits and type params --- src/librustc_ast_passes/ast_validation.rs | 14 +++++++++++++- src/test/ui/auto-trait-validation.stderr | 10 ++++++++++ ...aits-inductive-overflow-supertrait-oibit.stderr | 5 +++++ .../typeck-auto-trait-no-supertraits-2.stderr | 5 +++++ .../typeck/typeck-auto-trait-no-supertraits.stderr | 5 +++++ 5 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/librustc_ast_passes/ast_validation.rs b/src/librustc_ast_passes/ast_validation.rs index eff58ad4d9aab..df229e57cf0b7 100644 --- a/src/librustc_ast_passes/ast_validation.rs +++ b/src/librustc_ast_passes/ast_validation.rs @@ -910,6 +910,12 @@ impl<'a> Visitor<'a> for AstValidator<'a> { item.ident.span, "auto trait cannot have generic parameters", ); + err.span_suggestion_verbose( + generics.span, + "remove the parameters for the auto trait to be valid", + String::new(), + Applicability::MachineApplicable, + ); err.emit(); } if !bounds.is_empty() { @@ -922,6 +928,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> { E0568, "auto traits cannot have super traits" ); + err.span_label(item.ident.span, "auto trait cannot have super traits"); if let Some(span) = last { err.span_label( span, @@ -931,8 +938,13 @@ impl<'a> Visitor<'a> for AstValidator<'a> { s = pluralize!(len) ), ); + err.span_suggestion_verbose( + generics.span.shrink_to_hi().to(span), + "remove the super traits for the auto trait to be valid", + String::new(), + Applicability::MachineApplicable, + ); } - err.span_label(item.ident.span, "auto trait cannot have super traits"); err.emit(); } if !trait_items.is_empty() { diff --git a/src/test/ui/auto-trait-validation.stderr b/src/test/ui/auto-trait-validation.stderr index 75919a1f9913f..17d9ca71d149a 100644 --- a/src/test/ui/auto-trait-validation.stderr +++ b/src/test/ui/auto-trait-validation.stderr @@ -5,6 +5,11 @@ LL | auto trait Generic {} | ------- ^ cannot have this generic parameter | | | auto trait cannot have generic parameters + | +help: remove the parameters for the auto trait to be valid + | +LL | auto trait Generic {} + | -- error[E0568]: auto traits cannot have super traits --> $DIR/auto-trait-validation.rs:5:20 @@ -13,6 +18,11 @@ LL | auto trait Bound : Copy {} | ----- ^^^^ cannot have this super trait | | | auto trait cannot have super traits + | +help: remove the super traits for the auto trait to be valid + | +LL | auto trait Bound {} + | -- error[E0380]: auto traits cannot have methods or associated items --> $DIR/auto-trait-validation.rs:7:25 diff --git a/src/test/ui/traits/traits-inductive-overflow-supertrait-oibit.stderr b/src/test/ui/traits/traits-inductive-overflow-supertrait-oibit.stderr index 03d05f4c928a6..8714cfbedbbc1 100644 --- a/src/test/ui/traits/traits-inductive-overflow-supertrait-oibit.stderr +++ b/src/test/ui/traits/traits-inductive-overflow-supertrait-oibit.stderr @@ -5,6 +5,11 @@ LL | auto trait Magic: Copy {} | ----- ^^^^ cannot have this super trait | | | auto trait cannot have super traits + | +help: remove the super traits for the auto trait to be valid + | +LL | auto trait Magic {} + | -- error[E0277]: the trait bound `NoClone: std::marker::Copy` is not satisfied --> $DIR/traits-inductive-overflow-supertrait-oibit.rs:15:23 diff --git a/src/test/ui/typeck/typeck-auto-trait-no-supertraits-2.stderr b/src/test/ui/typeck/typeck-auto-trait-no-supertraits-2.stderr index 45c95b191bd59..c652a0c6df03d 100644 --- a/src/test/ui/typeck/typeck-auto-trait-no-supertraits-2.stderr +++ b/src/test/ui/typeck/typeck-auto-trait-no-supertraits-2.stderr @@ -5,6 +5,11 @@ LL | auto trait Magic : Sized where Option : Magic {} | ----- ^^^^^ cannot have this super trait | | | auto trait cannot have super traits + | +help: remove the super traits for the auto trait to be valid + | +LL | auto trait Magic where Option : Magic {} + | -- error: aborting due to previous error diff --git a/src/test/ui/typeck/typeck-auto-trait-no-supertraits.stderr b/src/test/ui/typeck/typeck-auto-trait-no-supertraits.stderr index 094f5d7dd24f2..fe6468bc71621 100644 --- a/src/test/ui/typeck/typeck-auto-trait-no-supertraits.stderr +++ b/src/test/ui/typeck/typeck-auto-trait-no-supertraits.stderr @@ -5,6 +5,11 @@ LL | auto trait Magic: Copy {} | ----- ^^^^ cannot have this super trait | | | auto trait cannot have super traits + | +help: remove the super traits for the auto trait to be valid + | +LL | auto trait Magic {} + | -- error: aborting due to previous error From f483032e97ba7c89f803fc6f8078f0acdd9d9b3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 6 Mar 2020 11:58:52 -0800 Subject: [PATCH 12/27] review comment --- src/librustc_ast_passes/ast_validation.rs | 25 +++++++++-------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/src/librustc_ast_passes/ast_validation.rs b/src/librustc_ast_passes/ast_validation.rs index df229e57cf0b7..24cf4556dad2f 100644 --- a/src/librustc_ast_passes/ast_validation.rs +++ b/src/librustc_ast_passes/ast_validation.rs @@ -820,23 +820,14 @@ impl<'a> Visitor<'a> for AstValidator<'a> { ref self_ty, items: _, } => { - let error = |annotation_span, annotation, note, code| { + let error = |annotation_span, annotation| { let mut err = self.err_handler().struct_span_err( self_ty.span, &format!("inherent impls cannot be {}", annotation), ); err.span_label(annotation_span, &format!("{} because of this", annotation)); err.span_label(self_ty.span, "inherent impl for this type"); - if note { - err.note(&format!( - "only trait implementations may be annotated with {}", - annotation - )); - } - if code { - err.code(error_code!(E0197)); - } - err.emit(); + err }; self.invalid_visibility( @@ -844,16 +835,20 @@ impl<'a> Visitor<'a> for AstValidator<'a> { Some("place qualifiers on individual impl items instead"), ); if let Unsafe::Yes(span) = unsafety { - error(span, "unsafe", false, true) + error(span, "unsafe").code(error_code!(E0197)).emit(); } if let ImplPolarity::Negative(span) = polarity { - error(span, "negative", false, false); + error(span, "negative").emit(); } if let Defaultness::Default(def_span) = defaultness { - error(def_span, "`default`", true, false); + error(def_span, "`default`") + .note("only trait implementations may be annotated with `default`") + .emit(); } if let Const::Yes(span) = constness { - error(span, "`const`", true, false); + error(span, "`const`") + .note("only trait implementations may be annotated with `const`") + .emit(); } } ItemKind::Fn(def, ref sig, ref generics, ref body) => { From 48975946565e9a0adedb39a308db2fd2eeaa2839 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 8 Mar 2020 19:05:18 +0100 Subject: [PATCH 13/27] miri: ICE on invalid terminators --- src/librustc_mir/interpret/terminator.rs | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs index ea8378574a3e0..85fd502c69c31 100644 --- a/src/librustc_mir/interpret/terminator.rs +++ b/src/librustc_mir/interpret/terminator.rs @@ -114,15 +114,12 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { Unreachable => throw_ub!(Unreachable), // These should never occur for MIR we actually run. - DropAndReplace { .. } | FalseEdges { .. } | FalseUnwind { .. } => { - bug!("{:#?} should have been eliminated by MIR pass", terminator.kind) - } - - // These are not (yet) supported. It is unclear if they even can occur in - // MIR that we actually run. - Yield { .. } | GeneratorDrop | Abort => { - throw_unsup_format!("Unsupported terminator kind: {:#?}", terminator.kind) - } + DropAndReplace { .. } + | FalseEdges { .. } + | FalseUnwind { .. } + | Yield { .. } + | GeneratorDrop + | Abort => bug!("{:#?} should have been eliminated by MIR pass", terminator.kind), } Ok(()) From 8a8870fbae1bf601ac4d29d6c0c407a352caea57 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 9 Mar 2020 10:45:20 +0100 Subject: [PATCH 14/27] miri: add machine hook for Abort terminator --- src/librustc_mir/interpret/machine.rs | 5 +++++ src/librustc_mir/interpret/terminator.rs | 9 +++++++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index 6615cc608fb54..64a34fc7dd9ec 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -170,6 +170,11 @@ pub trait Machine<'mir, 'tcx>: Sized { unwind: Option, ) -> InterpResult<'tcx>; + /// Called to evaluate `Abort` MIR terminator. + fn abort(_ecx: &mut InterpCx<'mir, 'tcx, Self>) -> InterpResult<'tcx> { + throw_unsup_format!("aborting execution is not supported"); + } + /// Called for all binary operations where the LHS has pointer type. /// /// Returns a (value, overflowed) pair if the operation succeeded diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs index 85fd502c69c31..95d5276565f17 100644 --- a/src/librustc_mir/interpret/terminator.rs +++ b/src/librustc_mir/interpret/terminator.rs @@ -99,6 +99,10 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } } + Abort => { + M::abort(self)?; + } + // When we encounter Resume, we've finished unwinding // cleanup for the current stack frame. We pop it in order // to continue unwinding the next frame @@ -118,8 +122,9 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { | FalseEdges { .. } | FalseUnwind { .. } | Yield { .. } - | GeneratorDrop - | Abort => bug!("{:#?} should have been eliminated by MIR pass", terminator.kind), + | GeneratorDrop => { + bug!("{:#?} should have been eliminated by MIR pass", terminator.kind) + } } Ok(()) From 911c75ff5f11c28c9f355857a47c5cd2d73767e7 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 9 Mar 2020 20:18:48 +0100 Subject: [PATCH 15/27] also handle abort intrinsic with new machine hook --- src/librustc_mir/interpret/intrinsics.rs | 4 ++++ src/librustc_mir/interpret/machine.rs | 2 +- src/librustc_span/symbol.rs | 1 + 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/librustc_mir/interpret/intrinsics.rs b/src/librustc_mir/interpret/intrinsics.rs index 891afbf437f2b..88c6c26c5ba9a 100644 --- a/src/librustc_mir/interpret/intrinsics.rs +++ b/src/librustc_mir/interpret/intrinsics.rs @@ -103,6 +103,10 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { self.write_scalar(location.ptr, dest)?; } + sym::abort => { + M::abort(self)?; + } + sym::min_align_of | sym::pref_align_of | sym::needs_drop diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index 64a34fc7dd9ec..a3c43d7d5d16a 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -171,7 +171,7 @@ pub trait Machine<'mir, 'tcx>: Sized { ) -> InterpResult<'tcx>; /// Called to evaluate `Abort` MIR terminator. - fn abort(_ecx: &mut InterpCx<'mir, 'tcx, Self>) -> InterpResult<'tcx> { + fn abort(_ecx: &mut InterpCx<'mir, 'tcx, Self>) -> InterpResult<'tcx, !> { throw_unsup_format!("aborting execution is not supported"); } diff --git a/src/librustc_span/symbol.rs b/src/librustc_span/symbol.rs index c39f9f360c027..4f5c4de8569f6 100644 --- a/src/librustc_span/symbol.rs +++ b/src/librustc_span/symbol.rs @@ -120,6 +120,7 @@ symbols! { abi_unadjusted, abi_vectorcall, abi_x86_interrupt, + abort, aborts, address, add_with_overflow, From 0037f4e37cb32f6195ab57fe4e5d34eb0adcd2d5 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Fri, 6 Mar 2020 11:13:26 -0300 Subject: [PATCH 16/27] Rename rustc-guide to rustc-dev-guide --- .github/ISSUE_TEMPLATE/tracking_issue.md | 8 ++++---- CONTRIBUTING.md | 18 +++++++++--------- README.md | 6 +++--- src/README.md | 2 +- src/doc/index.md | 2 +- src/doc/rustc/src/contributing.md | 2 +- src/librustc/README.md | 2 +- src/librustc/dep_graph/README.md | 2 +- src/librustc/dep_graph/graph.rs | 2 +- src/librustc/hir/mod.rs | 2 +- src/librustc/infer/canonical.rs | 2 +- src/librustc/lib.rs | 2 +- src/librustc/middle/region.rs | 2 +- src/librustc/mir/mod.rs | 2 +- src/librustc/traits/mod.rs | 2 +- src/librustc/traits/select.rs | 2 +- src/librustc/ty/context.rs | 2 +- src/librustc/ty/query/README.md | 2 +- src/librustc/ty/sty.rs | 2 +- src/librustc_ast/README.md | 4 ++-- src/librustc_codegen_llvm/README.md | 2 +- src/librustc_codegen_ssa/README.md | 4 ++-- src/librustc_driver/README.md | 2 +- src/librustc_hir/hir.rs | 2 +- src/librustc_hir/lib.rs | 2 +- src/librustc_incremental/persist/README.md | 2 +- .../infer/canonical/canonicalizer.rs | 6 +++--- src/librustc_infer/infer/canonical/mod.rs | 2 +- .../infer/canonical/query_response.rs | 4 ++-- .../infer/canonical/substitute.rs | 2 +- .../infer/higher_ranked/README.md | 6 +++--- src/librustc_infer/infer/higher_ranked/mod.rs | 2 +- .../infer/lexical_region_resolve/README.md | 4 ++-- .../infer/region_constraints/README.md | 2 +- src/librustc_infer/lib.rs | 2 +- src/librustc_infer/traits/coherence.rs | 4 ++-- src/librustc_infer/traits/mod.rs | 2 +- src/librustc_infer/traits/query/type_op/mod.rs | 2 +- src/librustc_infer/traits/select.rs | 6 +++--- src/librustc_infer/traits/specialize/mod.rs | 2 +- src/librustc_passes/region.rs | 2 +- src/librustc_target/README.md | 2 +- src/librustc_typeck/README.md | 4 ++-- src/librustc_typeck/check/method/mod.rs | 2 +- src/librustc_typeck/variance/mod.rs | 2 +- src/librustc_typeck/variance/terms.rs | 4 ++-- src/librustdoc/README.md | 2 +- src/test/COMPILER_TESTS.md | 2 +- triagebot.toml | 4 ++-- 49 files changed, 76 insertions(+), 76 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/tracking_issue.md b/.github/ISSUE_TEMPLATE/tracking_issue.md index f93591204cd98..faf84bf8aca5c 100644 --- a/.github/ISSUE_TEMPLATE/tracking_issue.md +++ b/.github/ISSUE_TEMPLATE/tracking_issue.md @@ -36,11 +36,11 @@ for larger features an implementation could be broken up into multiple PRs. - [ ] Implement the RFC (cc @rust-lang/XXX -- can anyone write up mentoring instructions?) -- [ ] Adjust documentation ([see instructions on rustc-guide][doc-guide]) -- [ ] Stabilization PR ([see instructions on rustc-guide][stabilization-guide]) +- [ ] Adjust documentation ([see instructions on rustc-dev-guide][doc-guide]) +- [ ] Stabilization PR ([see instructions on rustc-dev-guide][stabilization-guide]) -[stabilization-guide]: https://rust-lang.github.io/rustc-guide/stabilization_guide.html#stabilization-pr -[doc-guide]: https://rust-lang.github.io/rustc-guide/stabilization_guide.html#documentation-prs +[stabilization-guide]: https://rust-lang.github.io/rustc-dev-guide/stabilization_guide.html#stabilization-pr +[doc-guide]: https://rust-lang.github.io/rustc-dev-guide/stabilization_guide.html#documentation-prs ### Unresolved Questions $DIR/auto-trait-validation.rs:3:20 + --> $DIR/auto-trait-validation.rs:3:19 | LL | auto trait Generic {} - | ------- ^ cannot have this generic parameter + | -------^^^ help: remove the parameters | | | auto trait cannot have generic parameters - | -help: remove the parameters for the auto trait to be valid - | -LL | auto trait Generic {} - | -- error[E0568]: auto traits cannot have super traits --> $DIR/auto-trait-validation.rs:5:20 | LL | auto trait Bound : Copy {} - | ----- ^^^^ cannot have this super trait + | ----- ^^^^ help: remove the super traits | | | auto trait cannot have super traits - | -help: remove the super traits for the auto trait to be valid - | -LL | auto trait Bound {} - | -- error[E0380]: auto traits cannot have methods or associated items --> $DIR/auto-trait-validation.rs:7:25 diff --git a/src/test/ui/traits/traits-inductive-overflow-supertrait-oibit.stderr b/src/test/ui/traits/traits-inductive-overflow-supertrait-oibit.stderr index 8714cfbedbbc1..a83ff3701511d 100644 --- a/src/test/ui/traits/traits-inductive-overflow-supertrait-oibit.stderr +++ b/src/test/ui/traits/traits-inductive-overflow-supertrait-oibit.stderr @@ -2,14 +2,9 @@ error[E0568]: auto traits cannot have super traits --> $DIR/traits-inductive-overflow-supertrait-oibit.rs:7:19 | LL | auto trait Magic: Copy {} - | ----- ^^^^ cannot have this super trait + | ----- ^^^^ help: remove the super traits | | | auto trait cannot have super traits - | -help: remove the super traits for the auto trait to be valid - | -LL | auto trait Magic {} - | -- error[E0277]: the trait bound `NoClone: std::marker::Copy` is not satisfied --> $DIR/traits-inductive-overflow-supertrait-oibit.rs:15:23 diff --git a/src/test/ui/typeck/typeck-auto-trait-no-supertraits-2.stderr b/src/test/ui/typeck/typeck-auto-trait-no-supertraits-2.stderr index c652a0c6df03d..e397629327754 100644 --- a/src/test/ui/typeck/typeck-auto-trait-no-supertraits-2.stderr +++ b/src/test/ui/typeck/typeck-auto-trait-no-supertraits-2.stderr @@ -2,14 +2,9 @@ error[E0568]: auto traits cannot have super traits --> $DIR/typeck-auto-trait-no-supertraits-2.rs:3:20 | LL | auto trait Magic : Sized where Option : Magic {} - | ----- ^^^^^ cannot have this super trait + | ----- ^^^^^ help: remove the super traits | | | auto trait cannot have super traits - | -help: remove the super traits for the auto trait to be valid - | -LL | auto trait Magic where Option : Magic {} - | -- error: aborting due to previous error diff --git a/src/test/ui/typeck/typeck-auto-trait-no-supertraits.stderr b/src/test/ui/typeck/typeck-auto-trait-no-supertraits.stderr index fe6468bc71621..b1602e3642ecb 100644 --- a/src/test/ui/typeck/typeck-auto-trait-no-supertraits.stderr +++ b/src/test/ui/typeck/typeck-auto-trait-no-supertraits.stderr @@ -2,14 +2,9 @@ error[E0568]: auto traits cannot have super traits --> $DIR/typeck-auto-trait-no-supertraits.rs:27:19 | LL | auto trait Magic: Copy {} - | ----- ^^^^ cannot have this super trait + | ----- ^^^^ help: remove the super traits | | | auto trait cannot have super traits - | -help: remove the super traits for the auto trait to be valid - | -LL | auto trait Magic {} - | -- error: aborting due to previous error From cdc730457e9030feaf8c4376a668a9ef61c7f189 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Fri, 6 Mar 2020 11:20:27 +0100 Subject: [PATCH 22/27] Compute the correct layout for variants of uninhabited enums and readd a long lost assertion This reverts part of commit 9712fa405944cb8d5416556ac4b1f26365a10658. --- src/librustc/ty/layout.rs | 14 +++++++++++--- src/librustc_mir/interpret/operand.rs | 2 +- src/librustc_mir/interpret/place.rs | 8 -------- src/librustc_target/abi/mod.rs | 6 +++++- 4 files changed, 17 insertions(+), 13 deletions(-) diff --git a/src/librustc/ty/layout.rs b/src/librustc/ty/layout.rs index dedb3035cedb3..f616d81603775 100644 --- a/src/librustc/ty/layout.rs +++ b/src/librustc/ty/layout.rs @@ -782,8 +782,8 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { present_first @ Some(_) => present_first, // Uninhabited because it has no variants, or only absent ones. None if def.is_enum() => return tcx.layout_raw(param_env.and(tcx.types.never)), - // if it's a struct, still compute a layout so that we can still compute the - // field offsets + // If it's a struct, still compute a layout so that we can still compute the + // field offsets. None => Some(VariantIdx::new(0)), }; @@ -1990,7 +1990,15 @@ where { fn for_variant(this: TyLayout<'tcx>, cx: &C, variant_index: VariantIdx) -> TyLayout<'tcx> { let details = match this.variants { - Variants::Single { index } if index == variant_index => this.details, + Variants::Single { index } + // If all variants but one are uninhabited, the variant layout is the enum layout. + if index == variant_index && + // Don't confuse variants of uninhabited enums with the enum itself. + // For more details see https://github.com/rust-lang/rust/issues/69763. + this.fields != FieldPlacement::Union(0) => + { + this.details + } Variants::Single { index } => { // Deny calling for_variant more than once for non-Single enums. diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 22b1a7b7137d9..5d035bbeb27c7 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -356,7 +356,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { ) -> InterpResult<'tcx, OpTy<'tcx, M::PointerTag>> { let base = match op.try_as_mplace(self) { Ok(mplace) => { - // The easy case + // We can reuse the mplace field computation logic for indirect operands let field = self.mplace_field(mplace, field)?; return Ok(field.into()); } diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index a4815b9696ebb..856c654980ab7 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -410,14 +410,6 @@ where stride * field } layout::FieldPlacement::Union(count) => { - // This is a narrow bug-fix for rust-lang/rust#69191: if we are - // trying to access absent field of uninhabited variant, then - // signal UB (but don't ICE the compiler). - // FIXME temporary hack to work around incoherence between - // layout computation and MIR building - if field >= count as u64 && base.layout.abi == layout::Abi::Uninhabited { - throw_ub!(Unreachable); - } assert!( field < count as u64, "Tried to access field {} of union {:#?} with {} fields", diff --git a/src/librustc_target/abi/mod.rs b/src/librustc_target/abi/mod.rs index 2f8bbd66c322b..681326b0f150a 100644 --- a/src/librustc_target/abi/mod.rs +++ b/src/librustc_target/abi/mod.rs @@ -660,7 +660,11 @@ impl FieldPlacement { pub fn offset(&self, i: usize) -> Size { match *self { - FieldPlacement::Union(_) => Size::ZERO, + FieldPlacement::Union(count) => { + assert!(i < count, + "Tried to access field {} of union with {} fields", i, count); + Size::ZERO + }, FieldPlacement::Array { stride, count } => { let i = i as u64; assert!(i < count); From ec88ffa38cee51fa7290aa6c99d928ffe346ca6c Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Wed, 11 Mar 2020 13:57:54 +0100 Subject: [PATCH 23/27] Comment nits Co-Authored-By: Ralf Jung --- src/librustc_mir/interpret/operand.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 5d035bbeb27c7..07c0f76e03a7e 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -356,7 +356,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { ) -> InterpResult<'tcx, OpTy<'tcx, M::PointerTag>> { let base = match op.try_as_mplace(self) { Ok(mplace) => { - // We can reuse the mplace field computation logic for indirect operands + // We can reuse the mplace field computation logic for indirect operands. let field = self.mplace_field(mplace, field)?; return Ok(field.into()); } From 3244c84363e337ae07cf8dda07876d3c044e759e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20Kr=C3=BCger?= Date: Wed, 11 Mar 2020 14:24:07 +0100 Subject: [PATCH 24/27] rustdoc: remove unused import --- src/librustdoc/clean/inline.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index 7cb870ae702fb..f600b3308e882 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -12,7 +12,6 @@ use rustc_hir::Mutability; use rustc_metadata::creader::LoadedMacro; use rustc_mir::const_eval::is_min_const_fn; use rustc_span::hygiene::MacroKind; -use rustc_span::symbol::sym; use rustc_span::Span; use crate::clean::{self, GetDefId, ToSource, TypeKind}; From 74608c7f206171cb72c020a03800b2d9035a35fa Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Wed, 11 Mar 2020 14:31:07 +0100 Subject: [PATCH 25/27] Rustfmt and adjust capitalization --- src/librustc_target/abi/mod.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/librustc_target/abi/mod.rs b/src/librustc_target/abi/mod.rs index 681326b0f150a..afa30e7e632a7 100644 --- a/src/librustc_target/abi/mod.rs +++ b/src/librustc_target/abi/mod.rs @@ -661,10 +661,9 @@ impl FieldPlacement { pub fn offset(&self, i: usize) -> Size { match *self { FieldPlacement::Union(count) => { - assert!(i < count, - "Tried to access field {} of union with {} fields", i, count); + assert!(i < count, "tried to access field {} of union with {} fields", i, count); Size::ZERO - }, + } FieldPlacement::Array { stride, count } => { let i = i as u64; assert!(i < count); From 7ee1b470920d2ff4d6ffb100a5bbcf594b9031a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 11 Mar 2020 09:17:55 -0700 Subject: [PATCH 26/27] review comments --- src/librustc_ast_passes/ast_validation.rs | 103 +++++++++++----------- src/librustc_ast_passes/lib.rs | 1 + 2 files changed, 53 insertions(+), 51 deletions(-) diff --git a/src/librustc_ast_passes/ast_validation.rs b/src/librustc_ast_passes/ast_validation.rs index 668ac8047d17b..2a39dc6b011e7 100644 --- a/src/librustc_ast_passes/ast_validation.rs +++ b/src/librustc_ast_passes/ast_validation.rs @@ -9,6 +9,7 @@ use rustc_ast::ast::*; use rustc_ast::attr; use rustc_ast::expand::is_proc_macro_attr; +use rustc_ast::ptr::P; use rustc_ast::visit::{self, AssocCtxt, FnCtxt, FnKind, Visitor}; use rustc_ast::walk_list; use rustc_ast_pretty::pprust; @@ -594,6 +595,54 @@ impl<'a> AstValidator<'a> { .span_label(ident.span, format!("`_` is not a valid name for this `{}` item", kind)) .emit(); } + + fn deny_generic_params(&self, generics: &Generics, ident_span: Span) { + if !generics.params.is_empty() { + struct_span_err!( + self.session, + generics.span, + E0567, + "auto traits cannot have generic parameters" + ) + .span_label(ident_span, "auto trait cannot have generic parameters") + .span_suggestion( + generics.span, + "remove the parameters", + String::new(), + Applicability::MachineApplicable, + ) + .emit(); + } + } + + fn deny_super_traits(&self, bounds: &GenericBounds, ident_span: Span) { + if let [first @ last] | [first, .., last] = &bounds[..] { + let span = first.span().to(last.span()); + struct_span_err!(self.session, span, E0568, "auto traits cannot have super traits") + .span_label(ident_span, "auto trait cannot have super traits") + .span_suggestion( + span, + "remove the super traits", + String::new(), + Applicability::MachineApplicable, + ) + .emit(); + } + } + + fn deny_items(&self, trait_items: &[P], ident_span: Span) { + if !trait_items.is_empty() { + let spans: Vec<_> = trait_items.iter().map(|i| i.ident.span).collect(); + struct_span_err!( + self.session, + spans, + E0380, + "auto traits cannot have methods or associated items" + ) + .span_label(ident_span, "auto trait cannot have items") + .emit(); + } + } } fn validate_generic_param_order<'a>( @@ -881,57 +930,9 @@ impl<'a> Visitor<'a> for AstValidator<'a> { ItemKind::Trait(is_auto, _, ref generics, ref bounds, ref trait_items) => { if is_auto == IsAuto::Yes { // Auto traits cannot have generics, super traits nor contain items. - if !generics.params.is_empty() { - let mut err = struct_span_err!( - self.session, - generics.span, - E0567, - "auto traits cannot have generic parameters" - ); - err.span_label( - item.ident.span, - "auto trait cannot have generic parameters", - ); - err.span_suggestion( - generics.span, - "remove the parameters", - String::new(), - Applicability::MachineApplicable, - ); - err.emit(); - } - if !bounds.is_empty() { - let span = match &bounds[..] { - [] => unreachable!(), - [single] => single.span(), - [first, .., last] => first.span().to(last.span()), - }; - let mut err = struct_span_err!( - self.session, - span, - E0568, - "auto traits cannot have super traits" - ); - err.span_label(item.ident.span, "auto trait cannot have super traits"); - err.span_suggestion( - span, - "remove the super traits", - String::new(), - Applicability::MachineApplicable, - ); - err.emit(); - } - if !trait_items.is_empty() { - let spans: Vec<_> = trait_items.iter().map(|i| i.ident.span).collect(); - struct_span_err!( - self.session, - spans, - E0380, - "auto traits cannot have methods or associated items" - ) - .span_label(item.ident.span, "auto trait cannot have items") - .emit(); - } + self.deny_generic_params(generics, item.ident.span); + self.deny_super_traits(bounds, item.ident.span); + self.deny_items(trait_items, item.ident.span); } self.no_questions_in_bounds(bounds, "supertraits", true); diff --git a/src/librustc_ast_passes/lib.rs b/src/librustc_ast_passes/lib.rs index 520a7ac3e5665..10081d36754ba 100644 --- a/src/librustc_ast_passes/lib.rs +++ b/src/librustc_ast_passes/lib.rs @@ -1,3 +1,4 @@ +#![feature(bindings_after_at)] //! The `rustc_ast_passes` crate contains passes which validate the AST in `syntax` //! parsed by `rustc_parse` and then lowered, after the passes in this crate, //! by `rustc_ast_lowering`. From bbafe47c1449fd75eb5c4726da9bcdf7f6b3520a Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Wed, 11 Mar 2020 17:28:46 +0100 Subject: [PATCH 27/27] ci: bump webrender in cargotest This bumps the commit of webrender we test to include a fix for a spurious failure we encountered in CI. See #69895 for more context on the spurious failure. --- src/tools/cargotest/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/cargotest/main.rs b/src/tools/cargotest/main.rs index cf00cb1ab8a4b..d29244f077037 100644 --- a/src/tools/cargotest/main.rs +++ b/src/tools/cargotest/main.rs @@ -61,7 +61,7 @@ const TEST_REPOS: &'static [Test] = &[ Test { name: "webrender", repo: "https://github.com/servo/webrender", - sha: "a3d6e6894c5a601fa547c6273eb963ca1321c2bb", + sha: "6f23331299bf47e7e4683b815d10320770e14e21", lock: None, packages: &[], },