From d3f981b144c742c3e886a6234002a716a177bc98 Mon Sep 17 00:00:00 2001 From: Deadbeef Date: Wed, 15 Sep 2021 10:03:03 +0000 Subject: [PATCH 01/14] Move is_const_fn to under TyCtxt --- .../src/const_eval/fn_queries.rs | 19 +----------------- .../src/transform/promote_consts.rs | 6 ++---- compiler/rustc_middle/src/ty/context.rs | 20 +++++++++++++++++++ src/librustdoc/clean/mod.rs | 4 ++-- src/tools/clippy/clippy_utils/src/lib.rs | 1 - .../clippy_utils/src/qualify_min_const_fn.rs | 2 +- 6 files changed, 26 insertions(+), 26 deletions(-) diff --git a/compiler/rustc_const_eval/src/const_eval/fn_queries.rs b/compiler/rustc_const_eval/src/const_eval/fn_queries.rs index 10afd9560fa95..df4cc295fac5f 100644 --- a/compiler/rustc_const_eval/src/const_eval/fn_queries.rs +++ b/compiler/rustc_const_eval/src/const_eval/fn_queries.rs @@ -6,23 +6,6 @@ use rustc_middle::ty::TyCtxt; use rustc_span::symbol::Symbol; use rustc_target::spec::abi::Abi; -/// Whether the `def_id` counts as const fn in your current crate, considering all active -/// feature gates -pub fn is_const_fn(tcx: TyCtxt<'_>, def_id: DefId) -> bool { - tcx.is_const_fn_raw(def_id) - && match is_unstable_const_fn(tcx, def_id) { - Some(feature_name) => { - // has a `rustc_const_unstable` attribute, check whether the user enabled the - // corresponding feature gate. - tcx.features().declared_lib_features.iter().any(|&(sym, _)| sym == feature_name) - } - // functions without const stability are either stable user written - // const fn or the user is using feature gates and we thus don't - // care what they do - None => true, - } -} - /// Whether the `def_id` is an unstable const fn and what feature gate is necessary to enable it pub fn is_unstable_const_fn(tcx: TyCtxt<'_>, def_id: DefId) -> Option { if tcx.is_const_fn_raw(def_id) { @@ -77,7 +60,7 @@ fn is_const_fn_raw(tcx: TyCtxt<'_>, def_id: DefId) -> bool { } fn is_promotable_const_fn(tcx: TyCtxt<'_>, def_id: DefId) -> bool { - is_const_fn(tcx, def_id) + tcx.is_const_fn(def_id) && match tcx.lookup_const_stability(def_id) { Some(stab) => { if cfg!(debug_assertions) && stab.promotable { diff --git a/compiler/rustc_const_eval/src/transform/promote_consts.rs b/compiler/rustc_const_eval/src/transform/promote_consts.rs index 52d04cb4ff1e0..35c6ae201037a 100644 --- a/compiler/rustc_const_eval/src/transform/promote_consts.rs +++ b/compiler/rustc_const_eval/src/transform/promote_consts.rs @@ -26,7 +26,6 @@ use rustc_index::vec::{Idx, IndexVec}; use std::cell::Cell; use std::{cmp, iter, mem}; -use crate::const_eval::{is_const_fn, is_unstable_const_fn}; use crate::transform::check_consts::{is_lang_panic_fn, qualifs, ConstCx}; use crate::transform::MirPass; @@ -656,8 +655,7 @@ impl<'tcx> Validator<'_, 'tcx> { let is_const_fn = match *fn_ty.kind() { ty::FnDef(def_id, _) => { - is_const_fn(self.tcx, def_id) - || is_unstable_const_fn(self.tcx, def_id).is_some() + self.tcx.is_const_fn_raw(def_id) || is_lang_panic_fn(self.tcx, def_id) } _ => false, @@ -1079,7 +1077,7 @@ pub fn is_const_fn_in_array_repeat_expression<'tcx>( if let ty::FnDef(def_id, _) = *literal.ty().kind() { if let Some((destination_place, _)) = destination { if destination_place == place { - if is_const_fn(ccx.tcx, def_id) { + if ccx.tcx.is_const_fn(def_id) { return true; } } diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs index 1f5057d1da22f..2521b34a6b94e 100644 --- a/compiler/rustc_middle/src/ty/context.rs +++ b/compiler/rustc_middle/src/ty/context.rs @@ -2741,6 +2741,26 @@ impl<'tcx> TyCtxt<'tcx> { pub fn lifetime_scope(self, id: HirId) -> Option { self.lifetime_scope_map(id.owner).and_then(|mut map| map.remove(&id.local_id)) } + + /// Whether the `def_id` counts as const fn in the current crate, considering all active + /// feature gates + pub fn is_const_fn(self, def_id: DefId) -> bool { + if self.is_const_fn_raw(def_id) { + match self.lookup_const_stability(def_id) { + Some(stability) if stability.level.is_unstable() => { + // has a `rustc_const_unstable` attribute, check whether the user enabled the + // corresponding feature gate. + self.features().declared_lib_features.iter().any(|&(sym, _)| sym == stability.feature) + } + // functions without const stability are either stable user written + // const fn or the user is using feature gates and we thus don't + // care what they do + _ => true, + } + } else { + false + } + } } impl TyCtxtAt<'tcx> { diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index e281bbc59c255..ae7e818f106a6 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -11,7 +11,7 @@ crate mod utils; use rustc_ast as ast; use rustc_attr as attr; -use rustc_const_eval::const_eval::{is_const_fn, is_unstable_const_fn}; +use rustc_const_eval::const_eval::is_unstable_const_fn; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_hir as hir; use rustc_hir::def::{CtorKind, DefKind, Res}; @@ -787,7 +787,7 @@ fn clean_fn_or_proc_macro( let mut func = (sig, generics, body_id).clean(cx); let def_id = item.def_id.to_def_id(); func.header.constness = - if is_const_fn(cx.tcx, def_id) && is_unstable_const_fn(cx.tcx, def_id).is_none() { + if cx.tcx.is_const_fn(def_id) && is_unstable_const_fn(cx.tcx, def_id).is_none() { hir::Constness::Const } else { hir::Constness::NotConst diff --git a/src/tools/clippy/clippy_utils/src/lib.rs b/src/tools/clippy/clippy_utils/src/lib.rs index 3a94f47298390..beddc6f09be81 100644 --- a/src/tools/clippy/clippy_utils/src/lib.rs +++ b/src/tools/clippy/clippy_utils/src/lib.rs @@ -18,7 +18,6 @@ extern crate rustc_ast; extern crate rustc_ast_pretty; extern crate rustc_attr; -extern crate rustc_const_eval; extern crate rustc_data_structures; extern crate rustc_errors; extern crate rustc_hir; diff --git a/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs b/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs index e9a9895cb746f..67dda33e9daaf 100644 --- a/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs +++ b/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs @@ -364,7 +364,7 @@ fn check_terminator( } fn is_const_fn(tcx: TyCtxt<'_>, def_id: DefId, msrv: Option<&RustcVersion>) -> bool { - rustc_const_eval::const_eval::is_const_fn(tcx, def_id) + tcx.is_const_fn(def_id) && tcx.lookup_const_stability(def_id).map_or(true, |const_stab| { if let rustc_attr::StabilityLevel::Stable { since } = const_stab.level { // Checking MSRV is manually necessary because `rustc` has no such concept. This entire From f8aa73d3dd951d69903e962054f2614b083c5b9c Mon Sep 17 00:00:00 2001 From: Deadbeef Date: Wed, 15 Sep 2021 11:28:10 +0000 Subject: [PATCH 02/14] Coerce const FnDefs to implement const Fn traits --- .../src/transform/promote_consts.rs | 3 +- compiler/rustc_middle/src/traits/select.rs | 4 ++- compiler/rustc_middle/src/ty/context.rs | 5 ++- .../src/traits/select/candidate_assembly.rs | 6 ++-- .../src/traits/select/confirmation.rs | 2 +- .../src/traits/select/mod.rs | 17 ++++++---- .../const-closures.rs | 31 +++++++++++++++++++ 7 files changed, 55 insertions(+), 13 deletions(-) create mode 100644 src/test/ui/rfc-2632-const-trait-impl/const-closures.rs diff --git a/compiler/rustc_const_eval/src/transform/promote_consts.rs b/compiler/rustc_const_eval/src/transform/promote_consts.rs index 35c6ae201037a..c1cb5326ca35a 100644 --- a/compiler/rustc_const_eval/src/transform/promote_consts.rs +++ b/compiler/rustc_const_eval/src/transform/promote_consts.rs @@ -655,8 +655,7 @@ impl<'tcx> Validator<'_, 'tcx> { let is_const_fn = match *fn_ty.kind() { ty::FnDef(def_id, _) => { - self.tcx.is_const_fn_raw(def_id) - || is_lang_panic_fn(self.tcx, def_id) + self.tcx.is_const_fn_raw(def_id) || is_lang_panic_fn(self.tcx, def_id) } _ => false, }; diff --git a/compiler/rustc_middle/src/traits/select.rs b/compiler/rustc_middle/src/traits/select.rs index 3c0fedb360827..e236c4712c883 100644 --- a/compiler/rustc_middle/src/traits/select.rs +++ b/compiler/rustc_middle/src/traits/select.rs @@ -120,7 +120,9 @@ pub enum SelectionCandidate<'tcx> { /// Implementation of a `Fn`-family trait by one of the anonymous /// types generated for a fn pointer type (e.g., `fn(int) -> int`) - FnPointerCandidate, + FnPointerCandidate { + is_const: bool, + }, /// Builtin implementation of `DiscriminantKind`. DiscriminantKindCandidate, diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs index 2521b34a6b94e..8cbae81f67348 100644 --- a/compiler/rustc_middle/src/ty/context.rs +++ b/compiler/rustc_middle/src/ty/context.rs @@ -2750,7 +2750,10 @@ impl<'tcx> TyCtxt<'tcx> { Some(stability) if stability.level.is_unstable() => { // has a `rustc_const_unstable` attribute, check whether the user enabled the // corresponding feature gate. - self.features().declared_lib_features.iter().any(|&(sym, _)| sym == stability.feature) + self.features() + .declared_lib_features + .iter() + .any(|&(sym, _)| sym == stability.feature) } // functions without const stability are either stable user written // const fn or the user is using feature gates and we thus don't diff --git a/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs b/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs index 6d64dc8254bb4..d31ae216d3a57 100644 --- a/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs +++ b/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs @@ -475,7 +475,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { .. } = self_ty.fn_sig(self.tcx()).skip_binder() { - candidates.vec.push(FnPointerCandidate); + candidates.vec.push(FnPointerCandidate { is_const: false }); } } // Provide an impl for suitable functions, rejecting `#[target_feature]` functions (RFC 2396). @@ -488,7 +488,9 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { } = self_ty.fn_sig(self.tcx()).skip_binder() { if self.tcx().codegen_fn_attrs(def_id).target_features.is_empty() { - candidates.vec.push(FnPointerCandidate); + candidates + .vec + .push(FnPointerCandidate { is_const: self.tcx().is_const_fn(def_id) }); } } } diff --git a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs index 3b6555de912e9..b3ebd1cb1c440 100644 --- a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs +++ b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs @@ -92,7 +92,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { Ok(ImplSource::Generator(vtable_generator)) } - FnPointerCandidate => { + FnPointerCandidate { .. } => { let data = self.confirm_fn_pointer_candidate(obligation)?; Ok(ImplSource::FnPointer(data)) } diff --git a/compiler/rustc_trait_selection/src/traits/select/mod.rs b/compiler/rustc_trait_selection/src/traits/select/mod.rs index 7b948a0939fff..8dfd71e9cfb65 100644 --- a/compiler/rustc_trait_selection/src/traits/select/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/select/mod.rs @@ -1100,6 +1100,8 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { // generator, this will raise error in other places // or ignore error with const_async_blocks feature GeneratorCandidate => {} + // FnDef where the function is const + FnPointerCandidate { is_const: true } => {} ConstDropCandidate => {} _ => { // reject all other types of candidates @@ -1513,6 +1515,9 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { } } + // Drop otherwise equivalent non-const fn pointer candidates + (FnPointerCandidate { .. }, FnPointerCandidate { is_const: false }) => true, + // Global bounds from the where clause should be ignored // here (see issue #50825). Otherwise, we have a where // clause so don't go around looking for impls. @@ -1523,7 +1528,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { ImplCandidate(..) | ClosureCandidate | GeneratorCandidate - | FnPointerCandidate + | FnPointerCandidate { .. } | BuiltinObjectCandidate | BuiltinUnsizeCandidate | TraitUpcastingUnsizeCandidate(_) @@ -1541,7 +1546,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { ImplCandidate(_) | ClosureCandidate | GeneratorCandidate - | FnPointerCandidate + | FnPointerCandidate { .. } | BuiltinObjectCandidate | BuiltinUnsizeCandidate | TraitUpcastingUnsizeCandidate(_) @@ -1571,7 +1576,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { ImplCandidate(..) | ClosureCandidate | GeneratorCandidate - | FnPointerCandidate + | FnPointerCandidate { .. } | BuiltinObjectCandidate | BuiltinUnsizeCandidate | TraitUpcastingUnsizeCandidate(_) @@ -1583,7 +1588,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { ImplCandidate(..) | ClosureCandidate | GeneratorCandidate - | FnPointerCandidate + | FnPointerCandidate { .. } | BuiltinObjectCandidate | BuiltinUnsizeCandidate | TraitUpcastingUnsizeCandidate(_) @@ -1664,7 +1669,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { ImplCandidate(_) | ClosureCandidate | GeneratorCandidate - | FnPointerCandidate + | FnPointerCandidate { .. } | BuiltinObjectCandidate | BuiltinUnsizeCandidate | TraitUpcastingUnsizeCandidate(_) @@ -1673,7 +1678,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { ImplCandidate(_) | ClosureCandidate | GeneratorCandidate - | FnPointerCandidate + | FnPointerCandidate { .. } | BuiltinObjectCandidate | BuiltinUnsizeCandidate | TraitUpcastingUnsizeCandidate(_) diff --git a/src/test/ui/rfc-2632-const-trait-impl/const-closures.rs b/src/test/ui/rfc-2632-const-trait-impl/const-closures.rs new file mode 100644 index 0000000000000..99e608797ff65 --- /dev/null +++ b/src/test/ui/rfc-2632-const-trait-impl/const-closures.rs @@ -0,0 +1,31 @@ +// run-pass + +#![feature(const_trait_impl)] +#![feature(const_fn_trait_bound)] + +const fn answer_p1(f: &F) -> u8 + where + F: ~const FnOnce() -> u8, + F: ~const FnMut() -> u8, + F: ~const Fn() -> u8, +{ + f() * 7 +} + +const fn three() -> u8 { + 3 +} + +const fn answer_p2() -> u8 { + answer_p1(&three) +} + +const fn answer u8>(f: &F) -> u8 { + f() + f() +} + +const ANSWER: u8 = answer(&answer_p2); + +fn main() { + assert_eq!(ANSWER, 42) +} From d8e9db0dcf0fc5cd6585a86145bfed5ea3c55031 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Thu, 16 Sep 2021 13:05:14 -0700 Subject: [PATCH 03/14] feat(rustc_parse): recover from pre-RFC-2000 const generics syntax Fixes #89013 --- compiler/rustc_parse/src/parser/path.rs | 20 ++++++++- .../issue-89013-no-assoc.rs | 16 ++++++++ .../issue-89013-no-assoc.stderr | 8 ++++ .../issue-89013-no-kw.rs | 18 ++++++++ .../issue-89013-no-kw.stderr | 35 ++++++++++++++++ .../parser-error-recovery/issue-89013-type.rs | 17 ++++++++ .../issue-89013-type.stderr | 24 +++++++++++ .../parser-error-recovery/issue-89013.rs | 19 +++++++++ .../parser-error-recovery/issue-89013.stderr | 41 +++++++++++++++++++ 9 files changed, 197 insertions(+), 1 deletion(-) create mode 100644 src/test/ui/const-generics/parser-error-recovery/issue-89013-no-assoc.rs create mode 100644 src/test/ui/const-generics/parser-error-recovery/issue-89013-no-assoc.stderr create mode 100644 src/test/ui/const-generics/parser-error-recovery/issue-89013-no-kw.rs create mode 100644 src/test/ui/const-generics/parser-error-recovery/issue-89013-no-kw.stderr create mode 100644 src/test/ui/const-generics/parser-error-recovery/issue-89013-type.rs create mode 100644 src/test/ui/const-generics/parser-error-recovery/issue-89013-type.stderr create mode 100644 src/test/ui/const-generics/parser-error-recovery/issue-89013.rs create mode 100644 src/test/ui/const-generics/parser-error-recovery/issue-89013.stderr diff --git a/compiler/rustc_parse/src/parser/path.rs b/compiler/rustc_parse/src/parser/path.rs index 953c6915068af..4e4130dfc2306 100644 --- a/compiler/rustc_parse/src/parser/path.rs +++ b/compiler/rustc_parse/src/parser/path.rs @@ -495,11 +495,16 @@ impl<'a> Parser<'a> { None => { let after_eq = eq.shrink_to_hi(); let before_next = self.token.span.shrink_to_lo(); + let the_type_placeholder = if matches!(self.token.kind, token::Comma | token::Gt) { + " TheType" + } else { + " TheType " + }; self.struct_span_err(after_eq.to(before_next), "missing type to the right of `=`") .span_suggestion( self.sess.source_map().next_point(eq).to(before_next), "to constrain the associated type, add a type after `=`", - " TheType".to_string(), + the_type_placeholder.to_string(), Applicability::HasPlaceholders, ) .span_suggestion( @@ -572,6 +577,19 @@ impl<'a> Parser<'a> { return self.recover_const_arg(start, err).map(Some); } } + } else if self.eat_keyword_noexpect(kw::Const) { + // Detect and recover from the old, pre-RFC2000 syntax for const generics. + let mut err = self.struct_span_err( + start, + "expected lifetime, type, or constant, found keyword `const`", + ); + if self.check_const_arg() { + err.emit(); + GenericArg::Const(self.parse_const_arg()?) + } else { + let after_kw_const = self.token.span; + return self.recover_const_arg(after_kw_const, err).map(Some); + } } else { return Ok(None); }; diff --git a/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-assoc.rs b/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-assoc.rs new file mode 100644 index 0000000000000..99d8e9dea910d --- /dev/null +++ b/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-assoc.rs @@ -0,0 +1,16 @@ +trait Foo { + fn do_x(&self) -> [u8; N]; +} + +struct Bar; + +const T: usize = 42; + +impl Foo for Bar { +//~^ERROR expected lifetime, type, or constant, found keyword `const` + fn do_x(&self) -> [u8; 3] { + [0u8; 3] + } +} + +fn main() {} diff --git a/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-assoc.stderr b/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-assoc.stderr new file mode 100644 index 0000000000000..533d381fa3136 --- /dev/null +++ b/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-assoc.stderr @@ -0,0 +1,8 @@ +error: expected lifetime, type, or constant, found keyword `const` + --> $DIR/issue-89013-no-assoc.rs:9:10 + | +LL | impl Foo for Bar { + | ^^^^^ + +error: aborting due to previous error + diff --git a/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-kw.rs b/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-kw.rs new file mode 100644 index 0000000000000..b73b3e2910453 --- /dev/null +++ b/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-kw.rs @@ -0,0 +1,18 @@ +trait Foo { + fn do_x(&self) -> [u8; N]; +} + +struct Bar; + +const T: usize = 42; + +impl Foo for Bar { +//~^ERROR cannot constrain an associated constant to a value +//~^^ERROR this trait takes 1 generic argument but 0 generic arguments +//~^^^ERROR associated type bindings are not allowed here + fn do_x(&self) -> [u8; 3] { + [0u8; 3] + } +} + +fn main() {} diff --git a/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-kw.stderr b/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-kw.stderr new file mode 100644 index 0000000000000..7682815557671 --- /dev/null +++ b/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-kw.stderr @@ -0,0 +1,35 @@ +error: cannot constrain an associated constant to a value + --> $DIR/issue-89013-no-kw.rs:9:10 + | +LL | impl Foo for Bar { + | -^^^- + | | | + | | ...cannot be constrained to this value + | this associated constant... + +error[E0107]: this trait takes 1 generic argument but 0 generic arguments were supplied + --> $DIR/issue-89013-no-kw.rs:9:6 + | +LL | impl Foo for Bar { + | ^^^ expected 1 generic argument + | +note: trait defined here, with 1 generic parameter: `N` + --> $DIR/issue-89013-no-kw.rs:1:7 + | +LL | trait Foo { + | ^^^ - +help: add missing generic argument + | +LL | impl Foo for Bar { + | ++ + +error[E0229]: associated type bindings are not allowed here + --> $DIR/issue-89013-no-kw.rs:9:10 + | +LL | impl Foo for Bar { + | ^^^^^ associated type not allowed here + +error: aborting due to 3 previous errors + +Some errors have detailed explanations: E0107, E0229. +For more information about an error, try `rustc --explain E0107`. diff --git a/src/test/ui/const-generics/parser-error-recovery/issue-89013-type.rs b/src/test/ui/const-generics/parser-error-recovery/issue-89013-type.rs new file mode 100644 index 0000000000000..c34936d197620 --- /dev/null +++ b/src/test/ui/const-generics/parser-error-recovery/issue-89013-type.rs @@ -0,0 +1,17 @@ +trait Foo { + fn do_x(&self) -> [u8; N]; +} + +struct Bar; + +const T: usize = 42; + +impl Foo for Bar { +//~^ERROR missing type to the right of `=` +//~^^ERROR found keyword `type` + fn do_x(&self) -> [u8; 3] { + [0u8; 3] + } +} + +fn main() {} diff --git a/src/test/ui/const-generics/parser-error-recovery/issue-89013-type.stderr b/src/test/ui/const-generics/parser-error-recovery/issue-89013-type.stderr new file mode 100644 index 0000000000000..0f33f4b0df60c --- /dev/null +++ b/src/test/ui/const-generics/parser-error-recovery/issue-89013-type.stderr @@ -0,0 +1,24 @@ +error: missing type to the right of `=` + --> $DIR/issue-89013-type.rs:9:13 + | +LL | impl Foo for Bar { + | ^ + | +help: to constrain the associated type, add a type after `=` + | +LL | impl Foo for Bar { + | +++++++ +help: remove the `=` if `N` is a type + | +LL - impl Foo for Bar { +LL + impl Foo for Bar { + | + +error: expected one of `,`, `>`, a const expression, lifetime, or type, found keyword `type` + --> $DIR/issue-89013-type.rs:9:14 + | +LL | impl Foo for Bar { + | ^^^^ expected one of `,`, `>`, a const expression, lifetime, or type + +error: aborting due to 2 previous errors + diff --git a/src/test/ui/const-generics/parser-error-recovery/issue-89013.rs b/src/test/ui/const-generics/parser-error-recovery/issue-89013.rs new file mode 100644 index 0000000000000..d5ded44188a02 --- /dev/null +++ b/src/test/ui/const-generics/parser-error-recovery/issue-89013.rs @@ -0,0 +1,19 @@ +trait Foo { + fn do_x(&self) -> [u8; N]; +} + +struct Bar; + +const T: usize = 42; + +impl Foo for Bar { +//~^ERROR expected lifetime, type, or constant, found keyword `const` +//~^^ERROR cannot constrain an associated constant to a value +//~^^^ERROR this trait takes 1 generic argument but 0 generic arguments +//~^^^^ERROR associated type bindings are not allowed here + fn do_x(&self) -> [u8; 3] { + [0u8; 3] + } +} + +fn main() {} diff --git a/src/test/ui/const-generics/parser-error-recovery/issue-89013.stderr b/src/test/ui/const-generics/parser-error-recovery/issue-89013.stderr new file mode 100644 index 0000000000000..3df459ce16285 --- /dev/null +++ b/src/test/ui/const-generics/parser-error-recovery/issue-89013.stderr @@ -0,0 +1,41 @@ +error: expected lifetime, type, or constant, found keyword `const` + --> $DIR/issue-89013.rs:9:14 + | +LL | impl Foo for Bar { + | ^^^^^ + +error: cannot constrain an associated constant to a value + --> $DIR/issue-89013.rs:9:10 + | +LL | impl Foo for Bar { + | -^^^^^^^^^- + | | | + | | ...cannot be constrained to this value + | this associated constant... + +error[E0107]: this trait takes 1 generic argument but 0 generic arguments were supplied + --> $DIR/issue-89013.rs:9:6 + | +LL | impl Foo for Bar { + | ^^^ expected 1 generic argument + | +note: trait defined here, with 1 generic parameter: `N` + --> $DIR/issue-89013.rs:1:7 + | +LL | trait Foo { + | ^^^ - +help: add missing generic argument + | +LL | impl Foo for Bar { + | ++ + +error[E0229]: associated type bindings are not allowed here + --> $DIR/issue-89013.rs:9:10 + | +LL | impl Foo for Bar { + | ^^^^^^^^^^^ associated type not allowed here + +error: aborting due to 4 previous errors + +Some errors have detailed explanations: E0107, E0229. +For more information about an error, try `rustc --explain E0107`. From db8af3f9e1707ef06ced6174dbcf0d25a9d1ffd0 Mon Sep 17 00:00:00 2001 From: asquared31415 <34665709+asquared31415@users.noreply.github.com> Date: Mon, 27 Sep 2021 21:46:43 -0400 Subject: [PATCH 04/14] Add support for specifying multiple clobber_abi in `asm!` --- compiler/rustc_ast/src/ast.rs | 4 +- compiler/rustc_ast_lowering/src/asm.rs | 11 +- compiler/rustc_ast_pretty/src/pprust/state.rs | 4 +- compiler/rustc_builtin_macros/src/asm.rs | 21 ++-- src/test/ui/asm/multiple-clobber-abi.rs | 34 ++++++ src/test/ui/asm/x86_64/parse-error.rs | 4 +- src/test/ui/asm/x86_64/parse-error.stderr | 100 ++++++++---------- 7 files changed, 98 insertions(+), 80 deletions(-) create mode 100644 src/test/ui/asm/multiple-clobber-abi.rs diff --git a/compiler/rustc_ast/src/ast.rs b/compiler/rustc_ast/src/ast.rs index c27ab810a4c60..69fdec361caae 100644 --- a/compiler/rustc_ast/src/ast.rs +++ b/compiler/rustc_ast/src/ast.rs @@ -2056,7 +2056,7 @@ pub struct InlineAsm { pub template: Vec, pub template_strs: Box<[(Symbol, Option, Span)]>, pub operands: Vec<(InlineAsmOperand, Span)>, - pub clobber_abi: Option<(Symbol, Span)>, + pub clobber_abis: Vec<(Symbol, Span)>, pub options: InlineAsmOptions, pub line_spans: Vec, } @@ -2744,7 +2744,7 @@ pub enum ItemKind { } #[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] -rustc_data_structures::static_assert_size!(ItemKind, 112); +rustc_data_structures::static_assert_size!(ItemKind, 128); impl ItemKind { pub fn article(&self) -> &str { diff --git a/compiler/rustc_ast_lowering/src/asm.rs b/compiler/rustc_ast_lowering/src/asm.rs index 7165b3bcb9fc1..809d22cc0bfdf 100644 --- a/compiler/rustc_ast_lowering/src/asm.rs +++ b/compiler/rustc_ast_lowering/src/asm.rs @@ -2,6 +2,7 @@ use super::LoweringContext; use rustc_ast::*; use rustc_data_structures::fx::FxHashMap; +use rustc_data_structures::stable_set::FxHashSet; use rustc_errors::struct_span_err; use rustc_hir as hir; use rustc_span::{Span, Symbol}; @@ -27,11 +28,13 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { .emit(); } - let mut clobber_abi = None; + let mut clobber_abis = FxHashSet::default(); if let Some(asm_arch) = asm_arch { - if let Some((abi_name, abi_span)) = asm.clobber_abi { + for &(abi_name, abi_span) in &asm.clobber_abis { match asm::InlineAsmClobberAbi::parse(asm_arch, &self.sess.target, abi_name) { - Ok(abi) => clobber_abi = Some((abi, abi_span)), + Ok(abi) => { + clobber_abis.insert((abi, abi_span)); + } Err(&[]) => { self.sess .struct_span_err( @@ -368,7 +371,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { // If a clobber_abi is specified, add the necessary clobbers to the // operands list. - if let Some((abi, abi_span)) = clobber_abi { + for (abi, abi_span) in clobber_abis { for &clobber in abi.clobbered_regs() { let mut output_used = false; clobber.overlapping_regs(|reg| { diff --git a/compiler/rustc_ast_pretty/src/pprust/state.rs b/compiler/rustc_ast_pretty/src/pprust/state.rs index c24882086e12d..e83ce6b021474 100644 --- a/compiler/rustc_ast_pretty/src/pprust/state.rs +++ b/compiler/rustc_ast_pretty/src/pprust/state.rs @@ -2199,8 +2199,8 @@ impl<'a> State<'a> { let mut args = vec![AsmArg::Template(InlineAsmTemplatePiece::to_string(&asm.template))]; args.extend(asm.operands.iter().map(|(o, _)| AsmArg::Operand(o))); - if let Some((abi, _)) = asm.clobber_abi { - args.push(AsmArg::ClobberAbi(abi)); + for (abi, _) in &asm.clobber_abis { + args.push(AsmArg::ClobberAbi(*abi)); } if !asm.options.is_empty() { args.push(AsmArg::Options(asm.options)); diff --git a/compiler/rustc_builtin_macros/src/asm.rs b/compiler/rustc_builtin_macros/src/asm.rs index c032364c008f3..91a98684223ef 100644 --- a/compiler/rustc_builtin_macros/src/asm.rs +++ b/compiler/rustc_builtin_macros/src/asm.rs @@ -19,7 +19,7 @@ struct AsmArgs { operands: Vec<(ast::InlineAsmOperand, Span)>, named_args: FxHashMap, reg_args: FxHashSet, - clobber_abi: Option<(Symbol, Span)>, + clobber_abis: Vec<(Symbol, Span)>, options: ast::InlineAsmOptions, options_spans: Vec, } @@ -64,7 +64,7 @@ fn parse_args<'a>( operands: vec![], named_args: FxHashMap::default(), reg_args: FxHashSet::default(), - clobber_abi: None, + clobber_abis: Vec::new(), options: ast::InlineAsmOptions::empty(), options_spans: vec![], }; @@ -210,7 +210,7 @@ fn parse_args<'a>( .span_labels(args.options_spans.clone(), "previous options") .span_label(span, "argument") .emit(); - } else if let Some((_, abi_span)) = args.clobber_abi { + } else if let Some(&(_, abi_span)) = args.clobber_abis.last() { ecx.struct_span_err(span, "arguments are not allowed after clobber_abi") .span_label(abi_span, "clobber_abi") .span_label(span, "argument") @@ -322,7 +322,7 @@ fn parse_args<'a>( // Bail out now since this is likely to confuse MIR return Err(err); } - if let Some((_, abi_span)) = args.clobber_abi { + if let Some(&(_, abi_span)) = args.clobber_abis.last() { if is_global_asm { let err = ecx.struct_span_err(abi_span, "`clobber_abi` cannot be used with `global_asm!`"); @@ -453,14 +453,7 @@ fn parse_clobber_abi<'a>( let new_span = span_start.to(p.prev_token.span); - if let Some((_, prev_span)) = args.clobber_abi { - let mut err = p - .sess - .span_diagnostic - .struct_span_err(new_span, "clobber_abi specified multiple times"); - err.span_label(prev_span, "clobber_abi previously specified here"); - return Err(err); - } else if !args.options_spans.is_empty() { + if !args.options_spans.is_empty() { let mut err = p .sess .span_diagnostic @@ -469,7 +462,7 @@ fn parse_clobber_abi<'a>( return Err(err); } - args.clobber_abi = Some((clobber_abi, new_span)); + args.clobber_abis.push((clobber_abi, new_span)); Ok(()) } @@ -770,7 +763,7 @@ fn expand_preparsed_asm(ecx: &mut ExtCtxt<'_>, args: AsmArgs) -> Option i32 { + x + 16 +} + +extern "win64" fn bar(x: i32) -> i32 { + x / 2 +} + +fn main() { + let x = 8; + let y: i32; + // call `foo` with `x` as the input, and then `bar` with the output of `foo` + // and output that to `y` + unsafe { + asm!( + "call {}; mov rcx, rax; call {}", + sym foo, + sym bar, + in("rdi") x, + out("rax") y, + clobber_abi("sysv64"), + clobber_abi("win64") + ); + } + assert_eq!((x, y), (8, 12)); +} diff --git a/src/test/ui/asm/x86_64/parse-error.rs b/src/test/ui/asm/x86_64/parse-error.rs index fa14c52cf0ad7..ad777bad01b7b 100644 --- a/src/test/ui/asm/x86_64/parse-error.rs +++ b/src/test/ui/asm/x86_64/parse-error.rs @@ -50,8 +50,6 @@ fn main() { //~^ ERROR clobber_abi is not allowed after options asm!("{}", options(), clobber_abi("C"), const foo); //~^ ERROR clobber_abi is not allowed after options - asm!("", clobber_abi("C"), clobber_abi("C")); - //~^ ERROR clobber_abi specified multiple times asm!("{a}", a = const foo, a = const bar); //~^ ERROR duplicate argument named `a` //~^^ ERROR argument never used @@ -121,7 +119,7 @@ global_asm!("", options(), clobber_abi("C")); global_asm!("{}", options(), clobber_abi("C"), const FOO); //~^ ERROR clobber_abi is not allowed after options global_asm!("", clobber_abi("C"), clobber_abi("C")); -//~^ ERROR clobber_abi specified multiple times +//~^ ERROR `clobber_abi` cannot be used with `global_asm!` global_asm!("{a}", a = const FOO, a = const BAR); //~^ ERROR duplicate argument named `a` //~^^ ERROR argument never used diff --git a/src/test/ui/asm/x86_64/parse-error.stderr b/src/test/ui/asm/x86_64/parse-error.stderr index 78d342cc1daf7..b959ab341ffc1 100644 --- a/src/test/ui/asm/x86_64/parse-error.stderr +++ b/src/test/ui/asm/x86_64/parse-error.stderr @@ -132,16 +132,8 @@ LL | asm!("{}", options(), clobber_abi("C"), const foo); | | | options -error: clobber_abi specified multiple times - --> $DIR/parse-error.rs:53:36 - | -LL | asm!("", clobber_abi("C"), clobber_abi("C")); - | ---------------- ^^^^^^^^^^^^^^^^ - | | - | clobber_abi previously specified here - error: duplicate argument named `a` - --> $DIR/parse-error.rs:55:36 + --> $DIR/parse-error.rs:53:36 | LL | asm!("{a}", a = const foo, a = const bar); | ------------- ^^^^^^^^^^^^^ duplicate argument @@ -149,7 +141,7 @@ LL | asm!("{a}", a = const foo, a = const bar); | previously here error: argument never used - --> $DIR/parse-error.rs:55:36 + --> $DIR/parse-error.rs:53:36 | LL | asm!("{a}", a = const foo, a = const bar); | ^^^^^^^^^^^^^ argument never used @@ -157,13 +149,13 @@ LL | asm!("{a}", a = const foo, a = const bar); = help: if this argument is intentionally unused, consider using it in an asm comment: `"/* {1} */"` error: explicit register arguments cannot have names - --> $DIR/parse-error.rs:60:18 + --> $DIR/parse-error.rs:58:18 | LL | asm!("", a = in("eax") foo); | ^^^^^^^^^^^^^^^^^ error: named arguments cannot follow explicit register arguments - --> $DIR/parse-error.rs:62:36 + --> $DIR/parse-error.rs:60:36 | LL | asm!("{a}", in("eax") foo, a = const bar); | ------------- ^^^^^^^^^^^^^ named argument @@ -171,7 +163,7 @@ LL | asm!("{a}", in("eax") foo, a = const bar); | explicit register argument error: named arguments cannot follow explicit register arguments - --> $DIR/parse-error.rs:65:36 + --> $DIR/parse-error.rs:63:36 | LL | asm!("{a}", in("eax") foo, a = const bar); | ------------- ^^^^^^^^^^^^^ named argument @@ -179,7 +171,7 @@ LL | asm!("{a}", in("eax") foo, a = const bar); | explicit register argument error: positional arguments cannot follow named arguments or explicit register arguments - --> $DIR/parse-error.rs:68:36 + --> $DIR/parse-error.rs:66:36 | LL | asm!("{1}", in("eax") foo, const bar); | ------------- ^^^^^^^^^ positional argument @@ -187,19 +179,19 @@ LL | asm!("{1}", in("eax") foo, const bar); | explicit register argument error: expected one of `clobber_abi`, `const`, `in`, `inlateout`, `inout`, `lateout`, `options`, `out`, or `sym`, found `""` - --> $DIR/parse-error.rs:71:29 + --> $DIR/parse-error.rs:69:29 | LL | asm!("", options(), ""); | ^^ expected one of 9 possible tokens error: expected one of `clobber_abi`, `const`, `in`, `inlateout`, `inout`, `lateout`, `options`, `out`, or `sym`, found `"{}"` - --> $DIR/parse-error.rs:73:33 + --> $DIR/parse-error.rs:71:33 | LL | asm!("{}", in(reg) foo, "{}", out(reg) foo); | ^^^^ expected one of 9 possible tokens error: asm template must be a string literal - --> $DIR/parse-error.rs:75:14 + --> $DIR/parse-error.rs:73:14 | LL | asm!(format!("{{{}}}", 0), in(reg) foo); | ^^^^^^^^^^^^^^^^^^^^ @@ -207,7 +199,7 @@ LL | asm!(format!("{{{}}}", 0), in(reg) foo); = note: this error originates in the macro `format` (in Nightly builds, run with -Z macro-backtrace for more info) error: asm template must be a string literal - --> $DIR/parse-error.rs:77:21 + --> $DIR/parse-error.rs:75:21 | LL | asm!("{1}", format!("{{{}}}", 0), in(reg) foo, out(reg) bar); | ^^^^^^^^^^^^^^^^^^^^ @@ -215,79 +207,79 @@ LL | asm!("{1}", format!("{{{}}}", 0), in(reg) foo, out(reg) bar); = note: this error originates in the macro `format` (in Nightly builds, run with -Z macro-backtrace for more info) error: _ cannot be used for input operands - --> $DIR/parse-error.rs:79:28 + --> $DIR/parse-error.rs:77:28 | LL | asm!("{}", in(reg) _); | ^ error: _ cannot be used for input operands - --> $DIR/parse-error.rs:81:31 + --> $DIR/parse-error.rs:79:31 | LL | asm!("{}", inout(reg) _); | ^ error: _ cannot be used for input operands - --> $DIR/parse-error.rs:83:35 + --> $DIR/parse-error.rs:81:35 | LL | asm!("{}", inlateout(reg) _); | ^ error: requires at least a template string argument - --> $DIR/parse-error.rs:90:1 + --> $DIR/parse-error.rs:88:1 | LL | global_asm!(); | ^^^^^^^^^^^^^^ error: asm template must be a string literal - --> $DIR/parse-error.rs:92:13 + --> $DIR/parse-error.rs:90:13 | LL | global_asm!(FOO); | ^^^ error: expected token: `,` - --> $DIR/parse-error.rs:94:18 + --> $DIR/parse-error.rs:92:18 | LL | global_asm!("{}" FOO); | ^^^ expected `,` error: expected operand, options, or additional template string - --> $DIR/parse-error.rs:96:19 + --> $DIR/parse-error.rs:94:19 | LL | global_asm!("{}", FOO); | ^^^ expected operand, options, or additional template string error: expected expression, found end of macro arguments - --> $DIR/parse-error.rs:98:24 + --> $DIR/parse-error.rs:96:24 | LL | global_asm!("{}", const); | ^ expected expression error: expected one of `,`, `.`, `?`, or an operator, found `FOO` - --> $DIR/parse-error.rs:100:30 + --> $DIR/parse-error.rs:98:30 | LL | global_asm!("{}", const(reg) FOO); | ^^^ expected one of `,`, `.`, `?`, or an operator error: expected one of `)`, `att_syntax`, or `raw`, found `FOO` - --> $DIR/parse-error.rs:102:25 + --> $DIR/parse-error.rs:100:25 | LL | global_asm!("", options(FOO)); | ^^^ expected one of `)`, `att_syntax`, or `raw` error: expected one of `)`, `att_syntax`, or `raw`, found `nomem` - --> $DIR/parse-error.rs:104:25 + --> $DIR/parse-error.rs:102:25 | LL | global_asm!("", options(nomem FOO)); | ^^^^^ expected one of `)`, `att_syntax`, or `raw` error: expected one of `)`, `att_syntax`, or `raw`, found `nomem` - --> $DIR/parse-error.rs:106:25 + --> $DIR/parse-error.rs:104:25 | LL | global_asm!("", options(nomem, FOO)); | ^^^^^ expected one of `)`, `att_syntax`, or `raw` error: arguments are not allowed after options - --> $DIR/parse-error.rs:108:30 + --> $DIR/parse-error.rs:106:30 | LL | global_asm!("{}", options(), const FOO); | --------- ^^^^^^^^^ argument @@ -295,25 +287,25 @@ LL | global_asm!("{}", options(), const FOO); | previous options error: expected string literal - --> $DIR/parse-error.rs:110:29 + --> $DIR/parse-error.rs:108:29 | LL | global_asm!("", clobber_abi(FOO)); | ^^^ not a string literal error: expected `)`, found `FOO` - --> $DIR/parse-error.rs:112:33 + --> $DIR/parse-error.rs:110:33 | LL | global_asm!("", clobber_abi("C" FOO)); | ^^^ expected `)` error: expected `)`, found `,` - --> $DIR/parse-error.rs:114:32 + --> $DIR/parse-error.rs:112:32 | LL | global_asm!("", clobber_abi("C", FOO)); | ^ expected `)` error: arguments are not allowed after clobber_abi - --> $DIR/parse-error.rs:116:37 + --> $DIR/parse-error.rs:114:37 | LL | global_asm!("{}", clobber_abi("C"), const FOO); | ---------------- ^^^^^^^^^ argument @@ -321,13 +313,13 @@ LL | global_asm!("{}", clobber_abi("C"), const FOO); | clobber_abi error: `clobber_abi` cannot be used with `global_asm!` - --> $DIR/parse-error.rs:116:19 + --> $DIR/parse-error.rs:114:19 | LL | global_asm!("{}", clobber_abi("C"), const FOO); | ^^^^^^^^^^^^^^^^ error: clobber_abi is not allowed after options - --> $DIR/parse-error.rs:119:28 + --> $DIR/parse-error.rs:117:28 | LL | global_asm!("", options(), clobber_abi("C")); | --------- ^^^^^^^^^^^^^^^^ @@ -335,23 +327,21 @@ LL | global_asm!("", options(), clobber_abi("C")); | options error: clobber_abi is not allowed after options - --> $DIR/parse-error.rs:121:30 + --> $DIR/parse-error.rs:119:30 | LL | global_asm!("{}", options(), clobber_abi("C"), const FOO); | --------- ^^^^^^^^^^^^^^^^ | | | options -error: clobber_abi specified multiple times - --> $DIR/parse-error.rs:123:35 +error: `clobber_abi` cannot be used with `global_asm!` + --> $DIR/parse-error.rs:121:35 | LL | global_asm!("", clobber_abi("C"), clobber_abi("C")); - | ---------------- ^^^^^^^^^^^^^^^^ - | | - | clobber_abi previously specified here + | ^^^^^^^^^^^^^^^^ error: duplicate argument named `a` - --> $DIR/parse-error.rs:125:35 + --> $DIR/parse-error.rs:123:35 | LL | global_asm!("{a}", a = const FOO, a = const BAR); | ------------- ^^^^^^^^^^^^^ duplicate argument @@ -359,7 +349,7 @@ LL | global_asm!("{a}", a = const FOO, a = const BAR); | previously here error: argument never used - --> $DIR/parse-error.rs:125:35 + --> $DIR/parse-error.rs:123:35 | LL | global_asm!("{a}", a = const FOO, a = const BAR); | ^^^^^^^^^^^^^ argument never used @@ -367,19 +357,19 @@ LL | global_asm!("{a}", a = const FOO, a = const BAR); = help: if this argument is intentionally unused, consider using it in an asm comment: `"/* {1} */"` error: expected one of `clobber_abi`, `const`, or `options`, found `""` - --> $DIR/parse-error.rs:128:28 + --> $DIR/parse-error.rs:126:28 | LL | global_asm!("", options(), ""); | ^^ expected one of `clobber_abi`, `const`, or `options` error: expected one of `clobber_abi`, `const`, or `options`, found `"{}"` - --> $DIR/parse-error.rs:130:30 + --> $DIR/parse-error.rs:128:30 | LL | global_asm!("{}", const FOO, "{}", const FOO); | ^^^^ expected one of `clobber_abi`, `const`, or `options` error: asm template must be a string literal - --> $DIR/parse-error.rs:132:13 + --> $DIR/parse-error.rs:130:13 | LL | global_asm!(format!("{{{}}}", 0), const FOO); | ^^^^^^^^^^^^^^^^^^^^ @@ -387,7 +377,7 @@ LL | global_asm!(format!("{{{}}}", 0), const FOO); = note: this error originates in the macro `format` (in Nightly builds, run with -Z macro-backtrace for more info) error: asm template must be a string literal - --> $DIR/parse-error.rs:134:20 + --> $DIR/parse-error.rs:132:20 | LL | global_asm!("{1}", format!("{{{}}}", 0), const FOO, const BAR); | ^^^^^^^^^^^^^^^^^^^^ @@ -413,7 +403,7 @@ LL | asm!("{}", clobber_abi("C"), const foo); | ^^^ non-constant value error[E0435]: attempt to use a non-constant value in a constant - --> $DIR/parse-error.rs:55:31 + --> $DIR/parse-error.rs:53:31 | LL | let mut foo = 0; | ---------- help: consider using `const` instead of `let`: `const foo` @@ -422,7 +412,7 @@ LL | asm!("{a}", a = const foo, a = const bar); | ^^^ non-constant value error[E0435]: attempt to use a non-constant value in a constant - --> $DIR/parse-error.rs:55:46 + --> $DIR/parse-error.rs:53:46 | LL | let mut bar = 0; | ---------- help: consider using `const` instead of `let`: `const bar` @@ -431,7 +421,7 @@ LL | asm!("{a}", a = const foo, a = const bar); | ^^^ non-constant value error[E0435]: attempt to use a non-constant value in a constant - --> $DIR/parse-error.rs:62:46 + --> $DIR/parse-error.rs:60:46 | LL | let mut bar = 0; | ---------- help: consider using `const` instead of `let`: `const bar` @@ -440,7 +430,7 @@ LL | asm!("{a}", in("eax") foo, a = const bar); | ^^^ non-constant value error[E0435]: attempt to use a non-constant value in a constant - --> $DIR/parse-error.rs:65:46 + --> $DIR/parse-error.rs:63:46 | LL | let mut bar = 0; | ---------- help: consider using `const` instead of `let`: `const bar` @@ -449,7 +439,7 @@ LL | asm!("{a}", in("eax") foo, a = const bar); | ^^^ non-constant value error[E0435]: attempt to use a non-constant value in a constant - --> $DIR/parse-error.rs:68:42 + --> $DIR/parse-error.rs:66:42 | LL | let mut bar = 0; | ---------- help: consider using `const` instead of `let`: `const bar` @@ -457,6 +447,6 @@ LL | let mut bar = 0; LL | asm!("{1}", in("eax") foo, const bar); | ^^^ non-constant value -error: aborting due to 66 previous errors +error: aborting due to 65 previous errors For more information about this error, try `rustc --explain E0435`. From 42e9dfd75d293e5051c0204eacd773e1453e3365 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Mon, 30 Aug 2021 14:36:24 +0200 Subject: [PATCH 05/14] Reapply "Remove optimization_fuel_crate from Session" --- compiler/rustc_session/src/session.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index 27215556045be..b8b9c03091f0e 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -169,9 +169,6 @@ pub struct Session { /// Data about code being compiled, gathered during compilation. pub code_stats: CodeStats, - /// If `-zfuel=crate=n` is specified, `Some(crate)`. - optimization_fuel_crate: Option, - /// Tracks fuel info if `-zfuel=crate=n` is specified. optimization_fuel: Lock, @@ -896,7 +893,7 @@ impl Session { /// This expends fuel if applicable, and records fuel if applicable. pub fn consider_optimizing String>(&self, crate_name: &str, msg: T) -> bool { let mut ret = true; - if let Some(ref c) = self.optimization_fuel_crate { + if let Some((ref c, _)) = self.opts.debugging_opts.fuel { if c == crate_name { assert_eq!(self.threads(), 1); let mut fuel = self.optimization_fuel.lock(); @@ -1274,7 +1271,6 @@ pub fn build_session( let local_crate_source_file = local_crate_source_file.map(|path| file_path_mapping.map_prefix(path).0); - let optimization_fuel_crate = sopts.debugging_opts.fuel.as_ref().map(|i| i.0.clone()); let optimization_fuel = Lock::new(OptimizationFuel { remaining: sopts.debugging_opts.fuel.as_ref().map_or(0, |i| i.1), out_of_fuel: false, @@ -1326,7 +1322,6 @@ pub fn build_session( normalize_projection_ty: AtomicUsize::new(0), }, code_stats: Default::default(), - optimization_fuel_crate, optimization_fuel, print_fuel, jobserver: jobserver::client(), From 8a7c1306b43895a97c0675c6d854afa468a22aad Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Tue, 28 Sep 2021 10:57:34 -0700 Subject: [PATCH 06/14] Use less verbose syntax for error annotations Co-authored-by: Esteban Kuber --- .../const-generics/parser-error-recovery/issue-89013.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/test/ui/const-generics/parser-error-recovery/issue-89013.rs b/src/test/ui/const-generics/parser-error-recovery/issue-89013.rs index d5ded44188a02..99f3549d9ac34 100644 --- a/src/test/ui/const-generics/parser-error-recovery/issue-89013.rs +++ b/src/test/ui/const-generics/parser-error-recovery/issue-89013.rs @@ -7,10 +7,10 @@ struct Bar; const T: usize = 42; impl Foo for Bar { -//~^ERROR expected lifetime, type, or constant, found keyword `const` -//~^^ERROR cannot constrain an associated constant to a value -//~^^^ERROR this trait takes 1 generic argument but 0 generic arguments -//~^^^^ERROR associated type bindings are not allowed here +//~^ ERROR expected lifetime, type, or constant, found keyword `const` +//~| ERROR cannot constrain an associated constant to a value +//~| ERROR this trait takes 1 generic argument but 0 generic arguments +//~| ERROR associated type bindings are not allowed here fn do_x(&self) -> [u8; 3] { [0u8; 3] } From befdfb5c7113b3d82d973b1acfdcc2b0d9c14ab1 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Tue, 28 Sep 2021 12:48:58 -0700 Subject: [PATCH 07/14] Improve error messages for bad type constraints Co-authored-by: Esteban Kuber --- compiler/rustc_parse/src/parser/path.rs | 31 ++++++++++++------- .../issue-89013-no-assoc.stderr | 6 ++++ .../parser-error-recovery/issue-89013-type.rs | 1 - .../issue-89013-type.stderr | 20 ++---------- .../parser-error-recovery/issue-89013.stderr | 6 ++++ 5 files changed, 34 insertions(+), 30 deletions(-) diff --git a/compiler/rustc_parse/src/parser/path.rs b/compiler/rustc_parse/src/parser/path.rs index 4e4130dfc2306..c7d080a80fe6f 100644 --- a/compiler/rustc_parse/src/parser/path.rs +++ b/compiler/rustc_parse/src/parser/path.rs @@ -495,25 +495,28 @@ impl<'a> Parser<'a> { None => { let after_eq = eq.shrink_to_hi(); let before_next = self.token.span.shrink_to_lo(); - let the_type_placeholder = if matches!(self.token.kind, token::Comma | token::Gt) { - " TheType" - } else { - " TheType " - }; - self.struct_span_err(after_eq.to(before_next), "missing type to the right of `=`") - .span_suggestion( + let mut err = self + .struct_span_err(after_eq.to(before_next), "missing type to the right of `=`"); + if matches!(self.token.kind, token::Comma | token::Gt) { + err.span_suggestion( self.sess.source_map().next_point(eq).to(before_next), "to constrain the associated type, add a type after `=`", - the_type_placeholder.to_string(), + " TheType".to_string(), Applicability::HasPlaceholders, - ) - .span_suggestion( + ); + err.span_suggestion( eq.to(before_next), &format!("remove the `=` if `{}` is a type", ident), String::new(), Applicability::MaybeIncorrect, ) - .emit(); + } else { + err.span_label( + self.token.span, + &format!("expected type, found {}", super::token_descr(&self.token)), + ) + }; + return Err(err); } } Ok(self.mk_ty(span, ast::TyKind::Err)) @@ -584,6 +587,12 @@ impl<'a> Parser<'a> { "expected lifetime, type, or constant, found keyword `const`", ); if self.check_const_arg() { + err.span_suggestion_verbose( + start.until(self.token.span), + "the `const` keyword is only needed in the definition of the type", + String::new(), + Applicability::MaybeIncorrect, + ); err.emit(); GenericArg::Const(self.parse_const_arg()?) } else { diff --git a/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-assoc.stderr b/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-assoc.stderr index 533d381fa3136..ddddd86ab9c06 100644 --- a/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-assoc.stderr +++ b/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-assoc.stderr @@ -3,6 +3,12 @@ error: expected lifetime, type, or constant, found keyword `const` | LL | impl Foo for Bar { | ^^^^^ + | +help: the `const` keyword is only needed in the definition of the type + | +LL - impl Foo for Bar { +LL + impl Foo<3> for Bar { + | error: aborting due to previous error diff --git a/src/test/ui/const-generics/parser-error-recovery/issue-89013-type.rs b/src/test/ui/const-generics/parser-error-recovery/issue-89013-type.rs index c34936d197620..0ec6762b6e282 100644 --- a/src/test/ui/const-generics/parser-error-recovery/issue-89013-type.rs +++ b/src/test/ui/const-generics/parser-error-recovery/issue-89013-type.rs @@ -8,7 +8,6 @@ const T: usize = 42; impl Foo for Bar { //~^ERROR missing type to the right of `=` -//~^^ERROR found keyword `type` fn do_x(&self) -> [u8; 3] { [0u8; 3] } diff --git a/src/test/ui/const-generics/parser-error-recovery/issue-89013-type.stderr b/src/test/ui/const-generics/parser-error-recovery/issue-89013-type.stderr index 0f33f4b0df60c..f0d0d90c774f8 100644 --- a/src/test/ui/const-generics/parser-error-recovery/issue-89013-type.stderr +++ b/src/test/ui/const-generics/parser-error-recovery/issue-89013-type.stderr @@ -2,23 +2,7 @@ error: missing type to the right of `=` --> $DIR/issue-89013-type.rs:9:13 | LL | impl Foo for Bar { - | ^ - | -help: to constrain the associated type, add a type after `=` - | -LL | impl Foo for Bar { - | +++++++ -help: remove the `=` if `N` is a type - | -LL - impl Foo for Bar { -LL + impl Foo for Bar { - | - -error: expected one of `,`, `>`, a const expression, lifetime, or type, found keyword `type` - --> $DIR/issue-89013-type.rs:9:14 - | -LL | impl Foo for Bar { - | ^^^^ expected one of `,`, `>`, a const expression, lifetime, or type + | ^---- expected type, found keyword `type` -error: aborting due to 2 previous errors +error: aborting due to previous error diff --git a/src/test/ui/const-generics/parser-error-recovery/issue-89013.stderr b/src/test/ui/const-generics/parser-error-recovery/issue-89013.stderr index 3df459ce16285..a60e17aef589a 100644 --- a/src/test/ui/const-generics/parser-error-recovery/issue-89013.stderr +++ b/src/test/ui/const-generics/parser-error-recovery/issue-89013.stderr @@ -3,6 +3,12 @@ error: expected lifetime, type, or constant, found keyword `const` | LL | impl Foo for Bar { | ^^^^^ + | +help: the `const` keyword is only needed in the definition of the type + | +LL - impl Foo for Bar { +LL + impl Foo for Bar { + | error: cannot constrain an associated constant to a value --> $DIR/issue-89013.rs:9:10 From 105b60f2501fac061284ea7f5fbb8fb3f8134a92 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Tue, 28 Sep 2021 15:56:45 -0700 Subject: [PATCH 08/14] feat(rustc_typeck): avoid erroring with "wrong number of generics" if there's other problems As shown in the two test requirements that got updated, if there's other problems, then those other problems are probably the root cause of the incorrect generics count. --- compiler/rustc_hir/src/hir.rs | 10 +++++++++ compiler/rustc_typeck/src/astconv/generics.rs | 2 +- .../issue-89013-no-kw.rs | 5 ++--- .../issue-89013-no-kw.stderr | 21 ++----------------- .../parser-error-recovery/issue-89013.rs | 1 - .../parser-error-recovery/issue-89013.stderr | 21 ++----------------- 6 files changed, 17 insertions(+), 43 deletions(-) diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index 5655c4c0e973e..d952d5a544d7b 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -384,6 +384,16 @@ impl GenericArgs<'_> { self.args.iter().any(|arg| matches!(arg, GenericArg::Type(_))) } + pub fn has_err(&self) -> bool { + self.args.iter().any(|arg| match arg { + GenericArg::Type(ty) => matches!(ty.kind, TyKind::Err), + _ => false, + }) || self.bindings.iter().any(|arg| match arg.kind { + TypeBindingKind::Equality { ty } => matches!(ty.kind, TyKind::Err), + _ => false, + }) + } + #[inline] pub fn num_type_params(&self) -> usize { self.args.iter().filter(|arg| matches!(arg, GenericArg::Type(_))).count() diff --git a/compiler/rustc_typeck/src/astconv/generics.rs b/compiler/rustc_typeck/src/astconv/generics.rs index fd0544a47bb7c..92fc18854ee77 100644 --- a/compiler/rustc_typeck/src/astconv/generics.rs +++ b/compiler/rustc_typeck/src/astconv/generics.rs @@ -601,7 +601,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { def_id, ) .diagnostic() - .emit(); + .emit_unless(gen_args.has_err()); false }; diff --git a/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-kw.rs b/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-kw.rs index b73b3e2910453..19e0f38d320c4 100644 --- a/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-kw.rs +++ b/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-kw.rs @@ -7,9 +7,8 @@ struct Bar; const T: usize = 42; impl Foo for Bar { -//~^ERROR cannot constrain an associated constant to a value -//~^^ERROR this trait takes 1 generic argument but 0 generic arguments -//~^^^ERROR associated type bindings are not allowed here +//~^ ERROR cannot constrain an associated constant to a value +//~| ERROR associated type bindings are not allowed here fn do_x(&self) -> [u8; 3] { [0u8; 3] } diff --git a/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-kw.stderr b/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-kw.stderr index 7682815557671..bbca92ad63a91 100644 --- a/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-kw.stderr +++ b/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-kw.stderr @@ -7,29 +7,12 @@ LL | impl Foo for Bar { | | ...cannot be constrained to this value | this associated constant... -error[E0107]: this trait takes 1 generic argument but 0 generic arguments were supplied - --> $DIR/issue-89013-no-kw.rs:9:6 - | -LL | impl Foo for Bar { - | ^^^ expected 1 generic argument - | -note: trait defined here, with 1 generic parameter: `N` - --> $DIR/issue-89013-no-kw.rs:1:7 - | -LL | trait Foo { - | ^^^ - -help: add missing generic argument - | -LL | impl Foo for Bar { - | ++ - error[E0229]: associated type bindings are not allowed here --> $DIR/issue-89013-no-kw.rs:9:10 | LL | impl Foo for Bar { | ^^^^^ associated type not allowed here -error: aborting due to 3 previous errors +error: aborting due to 2 previous errors -Some errors have detailed explanations: E0107, E0229. -For more information about an error, try `rustc --explain E0107`. +For more information about this error, try `rustc --explain E0229`. diff --git a/src/test/ui/const-generics/parser-error-recovery/issue-89013.rs b/src/test/ui/const-generics/parser-error-recovery/issue-89013.rs index 99f3549d9ac34..ca1158a2f6d16 100644 --- a/src/test/ui/const-generics/parser-error-recovery/issue-89013.rs +++ b/src/test/ui/const-generics/parser-error-recovery/issue-89013.rs @@ -9,7 +9,6 @@ const T: usize = 42; impl Foo for Bar { //~^ ERROR expected lifetime, type, or constant, found keyword `const` //~| ERROR cannot constrain an associated constant to a value -//~| ERROR this trait takes 1 generic argument but 0 generic arguments //~| ERROR associated type bindings are not allowed here fn do_x(&self) -> [u8; 3] { [0u8; 3] diff --git a/src/test/ui/const-generics/parser-error-recovery/issue-89013.stderr b/src/test/ui/const-generics/parser-error-recovery/issue-89013.stderr index a60e17aef589a..85379d3f06e4b 100644 --- a/src/test/ui/const-generics/parser-error-recovery/issue-89013.stderr +++ b/src/test/ui/const-generics/parser-error-recovery/issue-89013.stderr @@ -19,29 +19,12 @@ LL | impl Foo for Bar { | | ...cannot be constrained to this value | this associated constant... -error[E0107]: this trait takes 1 generic argument but 0 generic arguments were supplied - --> $DIR/issue-89013.rs:9:6 - | -LL | impl Foo for Bar { - | ^^^ expected 1 generic argument - | -note: trait defined here, with 1 generic parameter: `N` - --> $DIR/issue-89013.rs:1:7 - | -LL | trait Foo { - | ^^^ - -help: add missing generic argument - | -LL | impl Foo for Bar { - | ++ - error[E0229]: associated type bindings are not allowed here --> $DIR/issue-89013.rs:9:10 | LL | impl Foo for Bar { | ^^^^^^^^^^^ associated type not allowed here -error: aborting due to 4 previous errors +error: aborting due to 3 previous errors -Some errors have detailed explanations: E0107, E0229. -For more information about an error, try `rustc --explain E0107`. +For more information about this error, try `rustc --explain E0229`. From 6490ed30e1025fbb85b3a2dd4849a249c072fb30 Mon Sep 17 00:00:00 2001 From: Fabian Wolff Date: Wed, 29 Sep 2021 00:11:55 +0200 Subject: [PATCH 09/14] Improve error message for `printf`-style format strings --- compiler/rustc_builtin_macros/src/format.rs | 26 ++++++-- .../src/format_foreign.rs | 65 ++++++++++++++----- .../src/format_foreign/printf/tests.rs | 4 +- .../src/format_foreign/shell/tests.rs | 4 +- src/test/ui/fmt/issue-89173.rs | 14 ++++ src/test/ui/fmt/issue-89173.stderr | 18 +++++ 6 files changed, 105 insertions(+), 26 deletions(-) create mode 100644 src/test/ui/fmt/issue-89173.rs create mode 100644 src/test/ui/fmt/issue-89173.stderr diff --git a/compiler/rustc_builtin_macros/src/format.rs b/compiler/rustc_builtin_macros/src/format.rs index 1c9c6834c100c..f0056cb79766a 100644 --- a/compiler/rustc_builtin_macros/src/format.rs +++ b/compiler/rustc_builtin_macros/src/format.rs @@ -1154,11 +1154,12 @@ pub fn expand_preparsed_format_args( // account for `"` and account for raw strings `r#` let padding = str_style.map(|i| i + 2).unwrap_or(1); for sub in foreign::$kind::iter_subs(fmt_str, padding) { - let trn = match sub.translate() { - Some(trn) => trn, + let (trn, success) = match sub.translate() { + Ok(trn) => (trn, true), + Err(Some(msg)) => (msg, false), // If it has no translation, don't call it out specifically. - None => continue, + _ => continue, }; let pos = sub.position(); @@ -1175,9 +1176,24 @@ pub fn expand_preparsed_format_args( if let Some(inner_sp) = pos { let sp = fmt_sp.from_inner(inner_sp); - suggestions.push((sp, trn)); + + if success { + suggestions.push((sp, trn)); + } else { + diag.span_note( + sp, + &format!("format specifiers use curly braces, and {}", trn), + ); + } } else { - diag.help(&format!("`{}` should be written as `{}`", sub, trn)); + if success { + diag.help(&format!("`{}` should be written as `{}`", sub, trn)); + } else { + diag.note(&format!( + "`{}` should use curly braces, and {}", + sub, trn + )); + } } } diff --git a/compiler/rustc_builtin_macros/src/format_foreign.rs b/compiler/rustc_builtin_macros/src/format_foreign.rs index 0cc520e5bd1f0..bfddd7073ff2c 100644 --- a/compiler/rustc_builtin_macros/src/format_foreign.rs +++ b/compiler/rustc_builtin_macros/src/format_foreign.rs @@ -1,4 +1,4 @@ -pub mod printf { +pub(crate) mod printf { use super::strcursor::StrCursor as Cur; use rustc_span::InnerSpan; @@ -36,10 +36,10 @@ pub mod printf { /// /// This ignores cases where the substitution does not have an exact equivalent, or where /// the substitution would be unnecessary. - pub fn translate(&self) -> Option { + pub fn translate(&self) -> Result> { match *self { Substitution::Format(ref fmt) => fmt.translate(), - Substitution::Escape => None, + Substitution::Escape => Err(None), } } } @@ -68,9 +68,9 @@ pub mod printf { impl Format<'_> { /// Translate this directive into an equivalent Rust formatting directive. /// - /// Returns `None` in cases where the `printf` directive does not have an exact Rust + /// Returns `Err` in cases where the `printf` directive does not have an exact Rust /// equivalent, rather than guessing. - pub fn translate(&self) -> Option { + pub fn translate(&self) -> Result> { use std::fmt::Write; let (c_alt, c_zero, c_left, c_plus) = { @@ -84,7 +84,12 @@ pub mod printf { '0' => c_zero = true, '-' => c_left = true, '+' => c_plus = true, - _ => return None, + _ => { + return Err(Some(format!( + "the flag `{}` is unknown or unsupported", + c + ))); + } } } (c_alt, c_zero, c_left, c_plus) @@ -104,7 +109,9 @@ pub mod printf { let width = match self.width { Some(Num::Next) => { // NOTE: Rust doesn't support this. - return None; + return Err(Some( + "you have to use a positional or named parameter for the width".to_string(), + )); } w @ Some(Num::Arg(_)) => w, w @ Some(Num::Num(_)) => w, @@ -125,13 +132,21 @@ pub mod printf { "p" => (Some(self.type_), false, true), "g" => (Some("e"), true, false), "G" => (Some("E"), true, false), - _ => return None, + _ => { + return Err(Some(format!( + "the conversion specifier `{}` is unknown or unsupported", + self.type_ + ))); + } }; let (fill, width, precision) = match (is_int, width, precision) { (true, Some(_), Some(_)) => { // Rust can't duplicate this insanity. - return None; + return Err(Some( + "width and precision cannot both be specified for integer conversions" + .to_string(), + )); } (true, None, Some(p)) => (Some("0"), Some(p), None), (true, w, None) => (fill, w, None), @@ -169,7 +184,17 @@ pub mod printf { s.push('{'); if let Some(arg) = self.parameter { - write!(s, "{}", arg.checked_sub(1)?).ok()?; + match write!( + s, + "{}", + match arg.checked_sub(1) { + Some(a) => a, + None => return Err(None), + } + ) { + Err(_) => return Err(None), + _ => {} + } } if has_options { @@ -199,12 +224,18 @@ pub mod printf { } if let Some(width) = width { - width.translate(&mut s).ok()?; + match width.translate(&mut s) { + Err(_) => return Err(None), + _ => {} + } } if let Some(precision) = precision { s.push('.'); - precision.translate(&mut s).ok()?; + match precision.translate(&mut s) { + Err(_) => return Err(None), + _ => {} + } } if let Some(type_) = type_ { @@ -213,7 +244,7 @@ pub mod printf { } s.push('}'); - Some(s) + Ok(s) } } @@ -623,11 +654,11 @@ pub mod shell { } } - pub fn translate(&self) -> Option { + pub fn translate(&self) -> Result> { match *self { - Substitution::Ordinal(n, _) => Some(format!("{{{}}}", n)), - Substitution::Name(n, _) => Some(format!("{{{}}}", n)), - Substitution::Escape(_) => None, + Substitution::Ordinal(n, _) => Ok(format!("{{{}}}", n)), + Substitution::Name(n, _) => Ok(format!("{{{}}}", n)), + Substitution::Escape(_) => Err(None), } } } diff --git a/compiler/rustc_builtin_macros/src/format_foreign/printf/tests.rs b/compiler/rustc_builtin_macros/src/format_foreign/printf/tests.rs index 33c54c9cee001..1336aab731674 100644 --- a/compiler/rustc_builtin_macros/src/format_foreign/printf/tests.rs +++ b/compiler/rustc_builtin_macros/src/format_foreign/printf/tests.rs @@ -3,7 +3,7 @@ use super::{iter_subs, parse_next_substitution as pns, Format as F, Num as N, Su macro_rules! assert_eq_pnsat { ($lhs:expr, $rhs:expr) => { assert_eq!( - pns($lhs).and_then(|(s, _)| s.translate()), + pns($lhs).and_then(|(s, _)| s.translate().ok()), $rhs.map(>::from) ) }; @@ -98,7 +98,7 @@ fn test_parse() { #[test] fn test_iter() { let s = "The %d'th word %% is: `%.*s` %!\n"; - let subs: Vec<_> = iter_subs(s, 0).map(|sub| sub.translate()).collect(); + let subs: Vec<_> = iter_subs(s, 0).map(|sub| sub.translate().ok()).collect(); assert_eq!( subs.iter().map(|ms| ms.as_ref().map(|s| &s[..])).collect::>(), vec![Some("{}"), None, Some("{:.*}"), None] diff --git a/compiler/rustc_builtin_macros/src/format_foreign/shell/tests.rs b/compiler/rustc_builtin_macros/src/format_foreign/shell/tests.rs index ed8fe81dfcdd8..f5f82732f2034 100644 --- a/compiler/rustc_builtin_macros/src/format_foreign/shell/tests.rs +++ b/compiler/rustc_builtin_macros/src/format_foreign/shell/tests.rs @@ -3,7 +3,7 @@ use super::{parse_next_substitution as pns, Substitution as S}; macro_rules! assert_eq_pnsat { ($lhs:expr, $rhs:expr) => { assert_eq!( - pns($lhs).and_then(|(f, _)| f.translate()), + pns($lhs).and_then(|(f, _)| f.translate().ok()), $rhs.map(>::from) ) }; @@ -37,7 +37,7 @@ fn test_parse() { fn test_iter() { use super::iter_subs; let s = "The $0'th word $$ is: `$WORD` $!\n"; - let subs: Vec<_> = iter_subs(s, 0).map(|sub| sub.translate()).collect(); + let subs: Vec<_> = iter_subs(s, 0).map(|sub| sub.translate().ok()).collect(); assert_eq!( subs.iter().map(|ms| ms.as_ref().map(|s| &s[..])).collect::>(), vec![Some("{0}"), None, Some("{WORD}")] diff --git a/src/test/ui/fmt/issue-89173.rs b/src/test/ui/fmt/issue-89173.rs new file mode 100644 index 0000000000000..96277d4d0d9d7 --- /dev/null +++ b/src/test/ui/fmt/issue-89173.rs @@ -0,0 +1,14 @@ +// Regression test for #89173: Make sure a helpful note is issued for +// printf-style format strings using `*` to specify the width. + +fn main() { + let num = 0x0abcde; + let width = 6; + print!("%0*x", width, num); + //~^ ERROR: multiple unused formatting arguments + //~| NOTE: multiple missing formatting specifiers + //~| NOTE: argument never used + //~| NOTE: argument never used + //~| NOTE: format specifiers use curly braces, and you have to use a positional or named parameter for the width + //~| NOTE: printf formatting not supported +} diff --git a/src/test/ui/fmt/issue-89173.stderr b/src/test/ui/fmt/issue-89173.stderr new file mode 100644 index 0000000000000..7b21e0a4fc896 --- /dev/null +++ b/src/test/ui/fmt/issue-89173.stderr @@ -0,0 +1,18 @@ +error: multiple unused formatting arguments + --> $DIR/issue-89173.rs:7:20 + | +LL | print!("%0*x", width, num); + | ------ ^^^^^ ^^^ argument never used + | | | + | | argument never used + | multiple missing formatting specifiers + | +note: format specifiers use curly braces, and you have to use a positional or named parameter for the width + --> $DIR/issue-89173.rs:7:13 + | +LL | print!("%0*x", width, num); + | ^^^^ + = note: printf formatting not supported; see the documentation for `std::fmt` + +error: aborting due to previous error + From 4425fe4a26972948d76a6bb5966afcf47bc4719b Mon Sep 17 00:00:00 2001 From: asquared31415 <34665709+asquared31415@users.noreply.github.com> Date: Wed, 29 Sep 2021 00:48:27 -0400 Subject: [PATCH 10/14] More spans, don't duplicate clobbers --- compiler/rustc_ast/src/ast.rs | 4 ++-- compiler/rustc_ast_lowering/src/asm.rs | 11 +++++++++-- compiler/rustc_builtin_macros/src/asm.rs | 15 ++++++++++----- src/test/ui/asm/x86_64/bad-options.rs | 2 ++ src/test/ui/asm/x86_64/bad-options.stderr | 23 ++++++++++++++++------- src/test/ui/asm/x86_64/parse-error.stderr | 4 ++-- 6 files changed, 41 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_ast/src/ast.rs b/compiler/rustc_ast/src/ast.rs index 69fdec361caae..41a5f5560b16c 100644 --- a/compiler/rustc_ast/src/ast.rs +++ b/compiler/rustc_ast/src/ast.rs @@ -2705,7 +2705,7 @@ pub enum ItemKind { /// E.g., `extern {}` or `extern "C" {}`. ForeignMod(ForeignMod), /// Module-level inline assembly (from `global_asm!()`). - GlobalAsm(InlineAsm), + GlobalAsm(Box), /// A type alias (`type`). /// /// E.g., `type Foo = Bar;`. @@ -2744,7 +2744,7 @@ pub enum ItemKind { } #[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] -rustc_data_structures::static_assert_size!(ItemKind, 128); +rustc_data_structures::static_assert_size!(ItemKind, 112); impl ItemKind { pub fn article(&self) -> &str { diff --git a/compiler/rustc_ast_lowering/src/asm.rs b/compiler/rustc_ast_lowering/src/asm.rs index 809d22cc0bfdf..3ee19e93e5b7c 100644 --- a/compiler/rustc_ast_lowering/src/asm.rs +++ b/compiler/rustc_ast_lowering/src/asm.rs @@ -28,12 +28,12 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { .emit(); } - let mut clobber_abis = FxHashSet::default(); + let mut clobber_abis = FxHashMap::default(); if let Some(asm_arch) = asm_arch { for &(abi_name, abi_span) in &asm.clobber_abis { match asm::InlineAsmClobberAbi::parse(asm_arch, &self.sess.target, abi_name) { Ok(abi) => { - clobber_abis.insert((abi, abi_span)); + clobber_abis.insert(abi, abi_span); } Err(&[]) => { self.sess @@ -371,8 +371,14 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { // If a clobber_abi is specified, add the necessary clobbers to the // operands list. + let mut clobbered = FxHashSet::default(); for (abi, abi_span) in clobber_abis { for &clobber in abi.clobbered_regs() { + // Don't emit a clobber for a register already clobbered + if clobbered.contains(&clobber) { + continue; + } + let mut output_used = false; clobber.overlapping_regs(|reg| { if used_output_regs.contains_key(®) { @@ -389,6 +395,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { }, self.lower_span(abi_span), )); + clobbered.insert(clobber); } } } diff --git a/compiler/rustc_builtin_macros/src/asm.rs b/compiler/rustc_builtin_macros/src/asm.rs index 91a98684223ef..976a17e203e35 100644 --- a/compiler/rustc_builtin_macros/src/asm.rs +++ b/compiler/rustc_builtin_macros/src/asm.rs @@ -322,10 +322,12 @@ fn parse_args<'a>( // Bail out now since this is likely to confuse MIR return Err(err); } - if let Some(&(_, abi_span)) = args.clobber_abis.last() { + if args.clobber_abis.len() > 0 { if is_global_asm { - let err = - ecx.struct_span_err(abi_span, "`clobber_abi` cannot be used with `global_asm!`"); + let err = ecx.struct_span_err( + args.clobber_abis.iter().map(|(_, sp)| *sp).collect::>(), + "`clobber_abi` cannot be used with `global_asm!`", + ); // Bail out now since this is likely to confuse later stages return Err(err); @@ -335,7 +337,10 @@ fn parse_args<'a>( regclass_outputs.clone(), "asm with `clobber_abi` must specify explicit registers for outputs", ) - .span_label(abi_span, "clobber_abi") + .span_labels( + args.clobber_abis.iter().map(|(_, sp)| *sp).collect::>(), + "clobber_abi", + ) .span_labels(regclass_outputs, "generic outputs") .emit(); } @@ -808,7 +813,7 @@ pub fn expand_global_asm<'cx>( ident: Ident::invalid(), attrs: Vec::new(), id: ast::DUMMY_NODE_ID, - kind: ast::ItemKind::GlobalAsm(inline_asm), + kind: ast::ItemKind::GlobalAsm(Box::new(inline_asm)), vis: ast::Visibility { span: sp.shrink_to_lo(), kind: ast::VisibilityKind::Inherited, diff --git a/src/test/ui/asm/x86_64/bad-options.rs b/src/test/ui/asm/x86_64/bad-options.rs index dc61d1612e8d6..285caa458991d 100644 --- a/src/test/ui/asm/x86_64/bad-options.rs +++ b/src/test/ui/asm/x86_64/bad-options.rs @@ -21,6 +21,8 @@ fn main() { //~^ ERROR invalid ABI for `clobber_abi` asm!("{}", out(reg) foo, clobber_abi("C")); //~^ ERROR asm with `clobber_abi` must specify explicit registers for outputs + asm!("{}", out(reg) foo, clobber_abi("C"), clobber_abi("C")); + //~^ ERROR asm with `clobber_abi` must specify explicit registers for outputs asm!("", out("eax") foo, clobber_abi("C")); } } diff --git a/src/test/ui/asm/x86_64/bad-options.stderr b/src/test/ui/asm/x86_64/bad-options.stderr index 8cfd450ab02a5..b69c4b55587d3 100644 --- a/src/test/ui/asm/x86_64/bad-options.stderr +++ b/src/test/ui/asm/x86_64/bad-options.stderr @@ -36,38 +36,47 @@ LL | asm!("{}", out(reg) foo, clobber_abi("C")); | | | generic outputs +error: asm with `clobber_abi` must specify explicit registers for outputs + --> $DIR/bad-options.rs:24:20 + | +LL | asm!("{}", out(reg) foo, clobber_abi("C"), clobber_abi("C")); + | ^^^^^^^^^^^^ ---------------- ---------------- clobber_abi + | | | + | | clobber_abi + | generic outputs + error: expected one of `)`, `att_syntax`, or `raw`, found `nomem` - --> $DIR/bad-options.rs:28:25 + --> $DIR/bad-options.rs:30:25 | LL | global_asm!("", options(nomem)); | ^^^^^ expected one of `)`, `att_syntax`, or `raw` error: expected one of `)`, `att_syntax`, or `raw`, found `readonly` - --> $DIR/bad-options.rs:30:25 + --> $DIR/bad-options.rs:32:25 | LL | global_asm!("", options(readonly)); | ^^^^^^^^ expected one of `)`, `att_syntax`, or `raw` error: expected one of `)`, `att_syntax`, or `raw`, found `noreturn` - --> $DIR/bad-options.rs:32:25 + --> $DIR/bad-options.rs:34:25 | LL | global_asm!("", options(noreturn)); | ^^^^^^^^ expected one of `)`, `att_syntax`, or `raw` error: expected one of `)`, `att_syntax`, or `raw`, found `pure` - --> $DIR/bad-options.rs:34:25 + --> $DIR/bad-options.rs:36:25 | LL | global_asm!("", options(pure)); | ^^^^ expected one of `)`, `att_syntax`, or `raw` error: expected one of `)`, `att_syntax`, or `raw`, found `nostack` - --> $DIR/bad-options.rs:36:25 + --> $DIR/bad-options.rs:38:25 | LL | global_asm!("", options(nostack)); | ^^^^^^^ expected one of `)`, `att_syntax`, or `raw` error: expected one of `)`, `att_syntax`, or `raw`, found `preserves_flags` - --> $DIR/bad-options.rs:38:25 + --> $DIR/bad-options.rs:40:25 | LL | global_asm!("", options(preserves_flags)); | ^^^^^^^^^^^^^^^ expected one of `)`, `att_syntax`, or `raw` @@ -80,5 +89,5 @@ LL | asm!("", clobber_abi("foo")); | = note: the following ABIs are supported on this target: `C`, `system`, `efiapi`, `win64`, `sysv64` -error: aborting due to 13 previous errors +error: aborting due to 14 previous errors diff --git a/src/test/ui/asm/x86_64/parse-error.stderr b/src/test/ui/asm/x86_64/parse-error.stderr index b959ab341ffc1..dbabee4597b27 100644 --- a/src/test/ui/asm/x86_64/parse-error.stderr +++ b/src/test/ui/asm/x86_64/parse-error.stderr @@ -335,10 +335,10 @@ LL | global_asm!("{}", options(), clobber_abi("C"), const FOO); | options error: `clobber_abi` cannot be used with `global_asm!` - --> $DIR/parse-error.rs:121:35 + --> $DIR/parse-error.rs:121:17 | LL | global_asm!("", clobber_abi("C"), clobber_abi("C")); - | ^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^ error: duplicate argument named `a` --> $DIR/parse-error.rs:123:35 From d90934ce87352e7478f8c872a5a65363206082a0 Mon Sep 17 00:00:00 2001 From: Andreas Jonson Date: Wed, 29 Sep 2021 22:58:33 +0200 Subject: [PATCH 11/14] Fix use after drop in self-profile with llvm events --- compiler/rustc_codegen_llvm/src/back/write.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/back/write.rs b/compiler/rustc_codegen_llvm/src/back/write.rs index 92199f611bad0..985640fb60e04 100644 --- a/compiler/rustc_codegen_llvm/src/back/write.rs +++ b/compiler/rustc_codegen_llvm/src/back/write.rs @@ -405,13 +405,15 @@ pub(crate) unsafe fn optimize_with_new_llvm_pass_manager( None }; - let llvm_selfprofiler = if cgcx.prof.llvm_recording_enabled() { - let mut llvm_profiler = LlvmSelfProfiler::new(cgcx.prof.get_self_profiler().unwrap()); - &mut llvm_profiler as *mut _ as *mut c_void + let mut llvm_profiler = if cgcx.prof.llvm_recording_enabled() { + Some(LlvmSelfProfiler::new(cgcx.prof.get_self_profiler().unwrap())) } else { - std::ptr::null_mut() + None }; + let llvm_selfprofiler = + llvm_profiler.as_mut().map(|s| s as *mut _ as *mut c_void).unwrap_or(std::ptr::null_mut()); + let extra_passes = config.passes.join(","); // FIXME: NewPM doesn't provide a facility to pass custom InlineParams. From b159d95dec86b5edf621d1e69748df2249fa153e Mon Sep 17 00:00:00 2001 From: asquared31415 <34665709+asquared31415@users.noreply.github.com> Date: Thu, 30 Sep 2021 01:56:56 -0400 Subject: [PATCH 12/14] Update docs --- src/doc/unstable-book/src/library-features/asm.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/doc/unstable-book/src/library-features/asm.md b/src/doc/unstable-book/src/library-features/asm.md index ccaf6e8733e0c..5eaac6d0d8255 100644 --- a/src/doc/unstable-book/src/library-features/asm.md +++ b/src/doc/unstable-book/src/library-features/asm.md @@ -319,7 +319,7 @@ fn call_foo(arg: i32) -> i32 { Note that the `fn` or `static` item does not need to be public or `#[no_mangle]`: the compiler will automatically insert the appropriate mangled symbol name into the assembly code. -By default, `asm!` assumes that any register not specified as an output will have its contents preserved by the assembly code. The [`clobber_abi`](#abi-clobbers) argument to `asm!` tells the compiler to automatically insert the necessary clobber operands according to the given calling convention ABI: any register which is not fully preserved in that ABI will be treated as clobbered. +By default, `asm!` assumes that any register not specified as an output will have its contents preserved by the assembly code. The [`clobber_abi`](#abi-clobbers) argument to `asm!` tells the compiler to automatically insert the necessary clobber operands according to the given calling convention ABI: any register which is not fully preserved in that ABI will be treated as clobbered. Multiple `clobber_abi` arguments may be provided and all clobbers from all specified ABIs will be inserted. ## Register template modifiers @@ -456,7 +456,7 @@ operand := reg_operand / "const" const_expr / "sym" path clobber_abi := "clobber_abi(" ")" option := "pure" / "nomem" / "readonly" / "preserves_flags" / "noreturn" / "nostack" / "att_syntax" / "raw" options := "options(" option *["," option] [","] ")" -asm := "asm!(" format_string *("," format_string) *("," [ident "="] operand) ["," clobber_abi] ["," options] [","] ")" +asm := "asm!(" format_string *("," format_string) *("," [ident "="] operand) *("," clobber_abi) ["," options] [","] ")" ``` Inline assembly is currently supported on the following architectures: @@ -799,6 +799,8 @@ As stated in the previous section, passing an input value smaller than the regis The `clobber_abi` keyword can be used to apply a default set of clobbers to an `asm` block. This will automatically insert the necessary clobber constraints as needed for calling a function with a particular calling convention: if the calling convention does not fully preserve the value of a register across a call then a `lateout("reg") _` is implicitly added to the operands list. +`clobber_abi` may be specified any number of times. It will insert a clobber for all unique registers in the union of all specified calling conventions. + Generic register class outputs are disallowed by the compiler when `clobber_abi` is used: all outputs must specify an explicit register. Explicit register outputs have precedence over the implicit clobbers inserted by `clobber_abi`: a clobber will only be inserted for a register if that register is not used as an output. The following ABIs can be used with `clobber_abi`: From 09f1542418fb6312e2252b4dead8a16ad0817e30 Mon Sep 17 00:00:00 2001 From: Chase Wilson Date: Sun, 29 Aug 2021 08:55:29 -0700 Subject: [PATCH 13/14] Implemented -Z randomize-layout --- Cargo.lock | 21 ++++++++--- compiler/rustc_middle/Cargo.toml | 2 ++ compiler/rustc_middle/src/ty/layout.rs | 48 +++++++++++++++++++------- compiler/rustc_middle/src/ty/mod.rs | 38 +++++++++++++++++++- compiler/rustc_session/src/options.rs | 2 ++ src/tools/tidy/src/deps.rs | 1 + 6 files changed, 93 insertions(+), 19 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6b3a714920bb4..9eff4dd8f6b14 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1660,7 +1660,7 @@ checksum = "3ca8957e71f04a205cb162508f9326aea04676c8dfd0711220190d6b83664f3f" dependencies = [ "bitmaps", "rand_core 0.5.1", - "rand_xoshiro", + "rand_xoshiro 0.4.0", "sized-chunks", "typenum", "version_check", @@ -2256,7 +2256,7 @@ dependencies = [ "libc", "log", "measureme", - "rand 0.8.3", + "rand 0.8.4", "rustc-workspace-hack", "rustc_version", "shell-escape", @@ -2852,9 +2852,9 @@ dependencies = [ [[package]] name = "rand" -version = "0.8.3" +version = "0.8.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0ef9e7e66b4468674bfcb0c81af8b7fa0bb154fa9f28eb840da5c447baeb8d7e" +checksum = "2e7573632e6454cf6b99d7aac4ccca54be06da05aca2ef7423d22d27d4d4bcd8" dependencies = [ "libc", "rand_chacha 0.3.0", @@ -2945,6 +2945,15 @@ dependencies = [ "rand_core 0.5.1", ] +[[package]] +name = "rand_xoshiro" +version = "0.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6f97cdb2a36ed4183de61b2f824cc45c9f1037f28afe0a322e9fff4c108b5aaa" +dependencies = [ + "rand_core 0.6.2", +] + [[package]] name = "rayon" version = "1.3.1" @@ -4086,6 +4095,8 @@ dependencies = [ "either", "gsgdt", "polonius-engine", + "rand 0.8.4", + "rand_xoshiro 0.6.0", "rustc-rayon-core", "rustc_apfloat", "rustc_arena", @@ -5096,7 +5107,7 @@ checksum = "dac1c663cfc93810f88aed9b8941d48cabf856a1b111c29a40439018d870eb22" dependencies = [ "cfg-if 1.0.0", "libc", - "rand 0.8.3", + "rand 0.8.4", "redox_syscall", "remove_dir_all", "winapi", diff --git a/compiler/rustc_middle/Cargo.toml b/compiler/rustc_middle/Cargo.toml index b1fcc34bee1d7..d06c593d39481 100644 --- a/compiler/rustc_middle/Cargo.toml +++ b/compiler/rustc_middle/Cargo.toml @@ -32,3 +32,5 @@ chalk-ir = "0.55.0" smallvec = { version = "1.6.1", features = ["union", "may_dangle"] } rustc_session = { path = "../rustc_session" } rustc_type_ir = { path = "../rustc_type_ir" } +rand = "0.8.4" +rand_xoshiro = "0.6.0" diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs index cfbbec374a172..b6aeb9122c3d3 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -24,6 +24,9 @@ use std::iter; use std::num::NonZeroUsize; use std::ops::Bound; +use rand::{seq::SliceRandom, SeedableRng}; +use rand_xoshiro::Xoshiro128StarStar; + pub fn provide(providers: &mut ty::query::Providers) { *providers = ty::query::Providers { layout_of, fn_abi_of_fn_ptr, fn_abi_of_instance, ..*providers }; @@ -324,6 +327,10 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { let mut inverse_memory_index: Vec = (0..fields.len() as u32).collect(); + // `ReprOptions.layout_seed` is a deterministic seed that we can use to + // randomize field ordering with + let mut rng = Xoshiro128StarStar::seed_from_u64(repr.field_shuffle_seed); + let optimize = !repr.inhibit_struct_field_reordering_opt(); if optimize { let end = @@ -332,20 +339,35 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { let field_align = |f: &TyAndLayout<'_>| { if let Some(pack) = pack { f.align.abi.min(pack) } else { f.align.abi } }; - match kind { - StructKind::AlwaysSized | StructKind::MaybeUnsized => { - optimizing.sort_by_key(|&x| { - // Place ZSTs first to avoid "interesting offsets", - // especially with only one or two non-ZST fields. - let f = &fields[x as usize]; - (!f.is_zst(), cmp::Reverse(field_align(f))) - }); - } - StructKind::Prefixed(..) => { - // Sort in ascending alignment so that the layout stay optimal - // regardless of the prefix - optimizing.sort_by_key(|&x| field_align(&fields[x as usize])); + + // If `-Z randomize-layout` was enabled for the type definition we can shuffle + // the field ordering to try and catch some code making assumptions about layouts + // we don't guarantee + if repr.can_randomize_type_layout() { + // Shuffle the ordering of the fields + optimizing.shuffle(&mut rng); + + // Otherwise we just leave things alone and actually optimize the type's fields + } else { + match kind { + StructKind::AlwaysSized | StructKind::MaybeUnsized => { + optimizing.sort_by_key(|&x| { + // Place ZSTs first to avoid "interesting offsets", + // especially with only one or two non-ZST fields. + let f = &fields[x as usize]; + (!f.is_zst(), cmp::Reverse(field_align(f))) + }); + } + + StructKind::Prefixed(..) => { + // Sort in ascending alignment so that the layout stays optimal + // regardless of the prefix + optimizing.sort_by_key(|&x| field_align(&fields[x as usize])); + } } + + // FIXME(Kixiron): We can always shuffle fields within a given alignment class + // regardless of the status of `-Z randomize-layout` } } diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index b3ae76d987167..d04d1565fead7 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -1491,6 +1491,9 @@ bitflags! { const IS_LINEAR = 1 << 3; // If true, don't expose any niche to type's context. const HIDE_NICHE = 1 << 4; + // If true, the type's layout can be randomized using + // the seed stored in `ReprOptions.layout_seed` + const RANDOMIZE_LAYOUT = 1 << 5; // Any of these flags being set prevent field reordering optimisation. const IS_UNOPTIMISABLE = ReprFlags::IS_C.bits | ReprFlags::IS_SIMD.bits | @@ -1505,6 +1508,14 @@ pub struct ReprOptions { pub align: Option, pub pack: Option, pub flags: ReprFlags, + /// The seed to be used for randomizing a type's layout + /// + /// Note: This could technically be a `[u8; 16]` (a `u128`) which would + /// be the "most accurate" hash as it'd encompass the item and crate + /// hash without loss, but it does pay the price of being larger. + /// Everything's a tradeoff, a `u64` seed should be sufficient for our + /// purposes (primarily `-Z randomize-layout`) + pub field_shuffle_seed: u64, } impl ReprOptions { @@ -1513,6 +1524,11 @@ impl ReprOptions { let mut size = None; let mut max_align: Option = None; let mut min_pack: Option = None; + + // Generate a deterministically-derived seed from the item's path hash + // to allow for cross-crate compilation to actually work + let field_shuffle_seed = tcx.def_path_hash(did).0.to_smaller_hash(); + for attr in tcx.get_attrs(did).iter() { for r in attr::find_repr_attrs(&tcx.sess, attr) { flags.insert(match r { @@ -1541,33 +1557,45 @@ impl ReprOptions { } } + // If `-Z randomize-layout` was enabled for the type definition then we can + // consider performing layout randomization + if tcx.sess.opts.debugging_opts.randomize_layout { + flags.insert(ReprFlags::RANDOMIZE_LAYOUT); + } + // This is here instead of layout because the choice must make it into metadata. if !tcx.consider_optimizing(|| format!("Reorder fields of {:?}", tcx.def_path_str(did))) { flags.insert(ReprFlags::IS_LINEAR); } - ReprOptions { int: size, align: max_align, pack: min_pack, flags } + + Self { int: size, align: max_align, pack: min_pack, flags, field_shuffle_seed } } #[inline] pub fn simd(&self) -> bool { self.flags.contains(ReprFlags::IS_SIMD) } + #[inline] pub fn c(&self) -> bool { self.flags.contains(ReprFlags::IS_C) } + #[inline] pub fn packed(&self) -> bool { self.pack.is_some() } + #[inline] pub fn transparent(&self) -> bool { self.flags.contains(ReprFlags::IS_TRANSPARENT) } + #[inline] pub fn linear(&self) -> bool { self.flags.contains(ReprFlags::IS_LINEAR) } + #[inline] pub fn hide_niche(&self) -> bool { self.flags.contains(ReprFlags::HIDE_NICHE) @@ -1594,9 +1622,17 @@ impl ReprOptions { return true; } } + self.flags.intersects(ReprFlags::IS_UNOPTIMISABLE) || self.int.is_some() } + /// Returns `true` if this type is valid for reordering and `-Z randomize-layout` + /// was enabled for its declaration crate + pub fn can_randomize_type_layout(&self) -> bool { + !self.inhibit_struct_field_reordering_opt() + && self.flags.contains(ReprFlags::RANDOMIZE_LAYOUT) + } + /// Returns `true` if this `#[repr()]` should inhibit union ABI optimisations. pub fn inhibit_union_abi_opt(&self) -> bool { self.c() diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index 8110afe75fa92..8ecb7a031ad81 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -1246,6 +1246,8 @@ options! { "enable queries of the dependency graph for regression testing (default: no)"), query_stats: bool = (false, parse_bool, [UNTRACKED], "print some statistics about the query system (default: no)"), + randomize_layout: bool = (false, parse_bool, [TRACKED], + "randomize the layout of types (default: no)"), relax_elf_relocations: Option = (None, parse_opt_bool, [TRACKED], "whether ELF relocations can be relaxed"), relro_level: Option = (None, parse_relro_level, [TRACKED], diff --git a/src/tools/tidy/src/deps.rs b/src/tools/tidy/src/deps.rs index 60703384e9e85..c1719a9ffe80c 100644 --- a/src/tools/tidy/src/deps.rs +++ b/src/tools/tidy/src/deps.rs @@ -167,6 +167,7 @@ const PERMITTED_DEPENDENCIES: &[&str] = &[ "rand_hc", "rand_pcg", "rand_xorshift", + "rand_xoshiro", "redox_syscall", "regex", "regex-automata", From 198d90786b9fb429928d70c423bad5d65374a532 Mon Sep 17 00:00:00 2001 From: Marcel Hlopko Date: Fri, 10 Sep 2021 15:11:56 +0200 Subject: [PATCH 14/14] Add `pie` as another `relocation-model` value --- compiler/rustc_codegen_llvm/src/back/write.rs | 3 +- compiler/rustc_codegen_llvm/src/context.rs | 7 +++- compiler/rustc_codegen_llvm/src/lib.rs | 13 +++++-- compiler/rustc_codegen_llvm/src/mono_item.rs | 6 +++ compiler/rustc_codegen_ssa/src/back/link.rs | 8 +++- compiler/rustc_target/src/spec/mod.rs | 3 ++ src/doc/rustc/src/codegen-options/index.md | 4 ++ src/test/assembly/pic-relocation-model.rs | 35 +++++++++++++++++ src/test/assembly/pie-relocation-model.rs | 38 +++++++++++++++++++ src/test/codegen/pic-relocation-model.rs | 16 ++++++++ src/test/codegen/pie-relocation-model.rs | 22 +++++++++++ 11 files changed, 147 insertions(+), 8 deletions(-) create mode 100644 src/test/assembly/pic-relocation-model.rs create mode 100644 src/test/assembly/pie-relocation-model.rs create mode 100644 src/test/codegen/pic-relocation-model.rs create mode 100644 src/test/codegen/pie-relocation-model.rs diff --git a/compiler/rustc_codegen_llvm/src/back/write.rs b/compiler/rustc_codegen_llvm/src/back/write.rs index 791604a18273d..e8484c24df9db 100644 --- a/compiler/rustc_codegen_llvm/src/back/write.rs +++ b/compiler/rustc_codegen_llvm/src/back/write.rs @@ -129,7 +129,8 @@ fn to_pass_builder_opt_level(cfg: config::OptLevel) -> llvm::PassBuilderOptLevel fn to_llvm_relocation_model(relocation_model: RelocModel) -> llvm::RelocModel { match relocation_model { RelocModel::Static => llvm::RelocModel::Static, - RelocModel::Pic => llvm::RelocModel::PIC, + // LLVM doesn't have a PIE relocation model, it represents PIE as PIC with an extra attribute. + RelocModel::Pic | RelocModel::Pie => llvm::RelocModel::PIC, RelocModel::DynamicNoPic => llvm::RelocModel::DynamicNoPic, RelocModel::Ropi => llvm::RelocModel::ROPI, RelocModel::Rwpi => llvm::RelocModel::RWPI, diff --git a/compiler/rustc_codegen_llvm/src/context.rs b/compiler/rustc_codegen_llvm/src/context.rs index 2d397dc583534..e69d42c948285 100644 --- a/compiler/rustc_codegen_llvm/src/context.rs +++ b/compiler/rustc_codegen_llvm/src/context.rs @@ -190,11 +190,14 @@ pub unsafe fn create_module( let llvm_target = SmallCStr::new(&sess.target.llvm_target); llvm::LLVMRustSetNormalizedTarget(llmod, llvm_target.as_ptr()); - if sess.relocation_model() == RelocModel::Pic { + let reloc_model = sess.relocation_model(); + if matches!(reloc_model, RelocModel::Pic | RelocModel::Pie) { llvm::LLVMRustSetModulePICLevel(llmod); // PIE is potentially more effective than PIC, but can only be used in executables. // If all our outputs are executables, then we can relax PIC to PIE. - if sess.crate_types().iter().all(|ty| *ty == CrateType::Executable) { + if reloc_model == RelocModel::Pie + || sess.crate_types().iter().all(|ty| *ty == CrateType::Executable) + { llvm::LLVMRustSetModulePIELevel(llmod); } } diff --git a/compiler/rustc_codegen_llvm/src/lib.rs b/compiler/rustc_codegen_llvm/src/lib.rs index 1da14344b1d26..0b06fe8bdcf9c 100644 --- a/compiler/rustc_codegen_llvm/src/lib.rs +++ b/compiler/rustc_codegen_llvm/src/lib.rs @@ -211,9 +211,16 @@ impl CodegenBackend for LlvmCodegenBackend { match req { PrintRequest::RelocationModels => { println!("Available relocation models:"); - for name in - &["static", "pic", "dynamic-no-pic", "ropi", "rwpi", "ropi-rwpi", "default"] - { + for name in &[ + "static", + "pic", + "pie", + "dynamic-no-pic", + "ropi", + "rwpi", + "ropi-rwpi", + "default", + ] { println!(" {}", name); } println!(); diff --git a/compiler/rustc_codegen_llvm/src/mono_item.rs b/compiler/rustc_codegen_llvm/src/mono_item.rs index 8ba3e870fbb71..17bf55bf512fe 100644 --- a/compiler/rustc_codegen_llvm/src/mono_item.rs +++ b/compiler/rustc_codegen_llvm/src/mono_item.rs @@ -144,6 +144,12 @@ impl CodegenCx<'ll, 'tcx> { return true; } + // With pie relocation model calls of functions defined in the translation + // unit can use copy relocations. + if self.tcx.sess.relocation_model() == RelocModel::Pie && !is_declaration { + return true; + } + return false; } } diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs index 4fb51ecc1d347..0dcddb693de75 100644 --- a/compiler/rustc_codegen_ssa/src/back/link.rs +++ b/compiler/rustc_codegen_ssa/src/back/link.rs @@ -1490,9 +1490,13 @@ fn exec_linker( fn link_output_kind(sess: &Session, crate_type: CrateType) -> LinkOutputKind { let kind = match (crate_type, sess.crt_static(Some(crate_type)), sess.relocation_model()) { (CrateType::Executable, _, _) if sess.is_wasi_reactor() => LinkOutputKind::WasiReactorExe, - (CrateType::Executable, false, RelocModel::Pic) => LinkOutputKind::DynamicPicExe, + (CrateType::Executable, false, RelocModel::Pic | RelocModel::Pie) => { + LinkOutputKind::DynamicPicExe + } (CrateType::Executable, false, _) => LinkOutputKind::DynamicNoPicExe, - (CrateType::Executable, true, RelocModel::Pic) => LinkOutputKind::StaticPicExe, + (CrateType::Executable, true, RelocModel::Pic | RelocModel::Pie) => { + LinkOutputKind::StaticPicExe + } (CrateType::Executable, true, _) => LinkOutputKind::StaticNoPicExe, (_, true, _) => LinkOutputKind::StaticDylib, (_, false, _) => LinkOutputKind::DynamicDylib, diff --git a/compiler/rustc_target/src/spec/mod.rs b/compiler/rustc_target/src/spec/mod.rs index 273221360b8b5..44eafde300181 100644 --- a/compiler/rustc_target/src/spec/mod.rs +++ b/compiler/rustc_target/src/spec/mod.rs @@ -287,6 +287,7 @@ impl ToJson for MergeFunctions { pub enum RelocModel { Static, Pic, + Pie, DynamicNoPic, Ropi, Rwpi, @@ -300,6 +301,7 @@ impl FromStr for RelocModel { Ok(match s { "static" => RelocModel::Static, "pic" => RelocModel::Pic, + "pie" => RelocModel::Pie, "dynamic-no-pic" => RelocModel::DynamicNoPic, "ropi" => RelocModel::Ropi, "rwpi" => RelocModel::Rwpi, @@ -314,6 +316,7 @@ impl ToJson for RelocModel { match *self { RelocModel::Static => "static", RelocModel::Pic => "pic", + RelocModel::Pie => "pie", RelocModel::DynamicNoPic => "dynamic-no-pic", RelocModel::Ropi => "ropi", RelocModel::Rwpi => "rwpi", diff --git a/src/doc/rustc/src/codegen-options/index.md b/src/doc/rustc/src/codegen-options/index.md index 05384117ac175..4f8c4c66f8891 100644 --- a/src/doc/rustc/src/codegen-options/index.md +++ b/src/doc/rustc/src/codegen-options/index.md @@ -435,6 +435,10 @@ Equivalent to the "uppercase" `-fPIC` or `-fPIE` options in other compilers, depending on the produced crate types. \ This is the default model for majority of supported targets. +- `pie` - position independent executable, relocatable code but without support for symbol +interpositioning (replacing symbols by name using `LD_PRELOAD` and similar). Equivalent to the "uppercase" `-fPIE` option in other compilers. `pie` +code cannot be linked into shared libraries (you'll get a linking error on attempt to do this). + #### Special relocation models - `dynamic-no-pic` - relocatable external references, non-relocatable code. \ diff --git a/src/test/assembly/pic-relocation-model.rs b/src/test/assembly/pic-relocation-model.rs new file mode 100644 index 0000000000000..72471ffcdb0cb --- /dev/null +++ b/src/test/assembly/pic-relocation-model.rs @@ -0,0 +1,35 @@ +// revisions: x64 +// assembly-output: emit-asm +// [x64] compile-flags: --target x86_64-unknown-linux-gnu -Crelocation-model=pic +// [x64] needs-llvm-components: x86 + + +#![feature(no_core, lang_items)] +#![no_core] +#![crate_type="rlib"] + +#[lang = "sized"] +trait Sized {} +#[lang = "copy"] +trait Copy {} + +// CHECK-LABEL: call_other_fn: +// CHECK: {{(jmpq|callq)}} *other_fn@GOTPCREL(%rip) +#[no_mangle] +pub fn call_other_fn() -> u8 { + unsafe { + other_fn() + } +} + +// CHECK-LABEL: other_fn: +// CHECK: callq *foreign_fn@GOTPCREL(%rip) +#[no_mangle] +#[inline(never)] +pub fn other_fn() -> u8 { + unsafe { + foreign_fn() + } +} + +extern "C" {fn foreign_fn() -> u8;} diff --git a/src/test/assembly/pie-relocation-model.rs b/src/test/assembly/pie-relocation-model.rs new file mode 100644 index 0000000000000..e40797e038d4b --- /dev/null +++ b/src/test/assembly/pie-relocation-model.rs @@ -0,0 +1,38 @@ +// revisions: x64 +// assembly-output: emit-asm +// [x64] compile-flags: --target x86_64-unknown-linux-gnu -Crelocation-model=pie +// [x64] needs-llvm-components: x86 + + +#![feature(no_core, lang_items)] +#![no_core] +#![crate_type="rlib"] + +#[lang = "sized"] +trait Sized {} +#[lang = "copy"] +trait Copy {} + +// CHECK-LABEL: call_other_fn: +// With PIE local functions are called "directly". +// CHECK: {{(jmp|callq)}} other_fn +#[no_mangle] +pub fn call_other_fn() -> u8 { + unsafe { + other_fn() + } +} + +// CHECK-LABEL: other_fn: +// External functions are still called through GOT, since we don't know if the symbol +// is defined in the binary or in the shared library. +// CHECK: callq *foreign_fn@GOTPCREL(%rip) +#[no_mangle] +#[inline(never)] +pub fn other_fn() -> u8 { + unsafe { + foreign_fn() + } +} + +extern "C" {fn foreign_fn() -> u8;} diff --git a/src/test/codegen/pic-relocation-model.rs b/src/test/codegen/pic-relocation-model.rs new file mode 100644 index 0000000000000..6e1d5a6c3f271 --- /dev/null +++ b/src/test/codegen/pic-relocation-model.rs @@ -0,0 +1,16 @@ +// compile-flags: -C relocation-model=pic + +#![crate_type = "rlib"] + +// CHECK: define i8 @call_foreign_fn() +#[no_mangle] +pub fn call_foreign_fn() -> u8 { + unsafe { + foreign_fn() + } +} + +// CHECK: declare zeroext i8 @foreign_fn() +extern "C" {fn foreign_fn() -> u8;} + +// CHECK: !{i32 7, !"PIC Level", i32 2} diff --git a/src/test/codegen/pie-relocation-model.rs b/src/test/codegen/pie-relocation-model.rs new file mode 100644 index 0000000000000..a843202a94f82 --- /dev/null +++ b/src/test/codegen/pie-relocation-model.rs @@ -0,0 +1,22 @@ +// compile-flags: -C relocation-model=pie +// only-x86_64-unknown-linux-gnu + +#![crate_type = "rlib"] + +// With PIE we know local functions cannot be interpositioned, we can mark them +// as dso_local. +// CHECK: define dso_local i8 @call_foreign_fn() +#[no_mangle] +pub fn call_foreign_fn() -> u8 { + unsafe { + foreign_fn() + } +} + +// External functions are still marked as non-dso_local, since we don't know if the symbol +// is defined in the binary or in the shared library. +// CHECK: declare zeroext i8 @foreign_fn() +extern "C" {fn foreign_fn() -> u8;} + +// CHECK: !{i32 7, !"PIC Level", i32 2} +// CHECK: !{i32 7, !"PIE Level", i32 2}