From 96eee8aa70054e9a7a5590804ece280fa9774ffc Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Thu, 27 Sep 2018 04:49:40 +0300 Subject: [PATCH 1/3] resolve: Prefer `macro_rules` definitions to in-module macro definitions in some cases --- src/librustc_resolve/lib.rs | 43 +++++++++++++++-- src/librustc_resolve/macros.rs | 5 +- src/librustc_resolve/resolve_imports.rs | 1 + src/test/ui/imports/macros.rs | 2 +- src/test/ui/imports/macros.stderr | 19 +------- .../ui/macros/ambiguity-legacy-vs-modern.rs | 46 +++++++++++++++++++ .../macros/ambiguity-legacy-vs-modern.stderr | 37 +++++++++++++++ 7 files changed, 130 insertions(+), 23 deletions(-) create mode 100644 src/test/ui/macros/ambiguity-legacy-vs-modern.rs create mode 100644 src/test/ui/macros/ambiguity-legacy-vs-modern.stderr diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index e393aa9921845..a19bf6d89cb9d 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -71,11 +71,10 @@ use syntax_pos::{Span, DUMMY_SP, MultiSpan}; use errors::{Applicability, DiagnosticBuilder, DiagnosticId}; use std::cell::{Cell, RefCell}; -use std::cmp; +use std::{cmp, fmt, iter, ptr}; use std::collections::BTreeSet; -use std::fmt; -use std::iter; use std::mem::replace; +use rustc_data_structures::ptr_key::PtrKey; use rustc_data_structures::sync::Lrc; use resolve_imports::{ImportDirective, ImportDirectiveSubclass, NameResolution, ImportResolver}; @@ -1113,6 +1112,17 @@ impl<'a> ModuleData<'a> { fn nearest_item_scope(&'a self) -> Module<'a> { if self.is_trait() { self.parent.unwrap() } else { self } } + + fn is_ancestor_of(&self, mut other: &Self) -> bool { + while !ptr::eq(self, other) { + if let Some(parent) = other.parent { + other = parent; + } else { + return false; + } + } + true + } } impl<'a> fmt::Debug for ModuleData<'a> { @@ -1407,6 +1417,7 @@ pub struct Resolver<'a, 'b: 'a> { block_map: NodeMap>, module_map: FxHashMap>, extern_module_map: FxHashMap<(DefId, bool /* MacrosOnly? */), Module<'a>>, + binding_parent_modules: FxHashMap>, Module<'a>>, pub make_glob_map: bool, /// Maps imports to the names of items actually imported (this actually maps @@ -1709,6 +1720,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> { module_map, block_map: NodeMap(), extern_module_map: FxHashMap(), + binding_parent_modules: FxHashMap(), make_glob_map: make_glob_map == MakeGlobMap::Yes, glob_map: NodeMap(), @@ -4526,6 +4538,31 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> { vis.is_accessible_from(module.normal_ancestor_id, self) } + fn set_binding_parent_module(&mut self, binding: &'a NameBinding<'a>, module: Module<'a>) { + if let Some(old_module) = self.binding_parent_modules.insert(PtrKey(binding), module) { + if !ptr::eq(module, old_module) { + span_bug!(binding.span, "parent module is reset for binding"); + } + } + } + + fn disambiguate_legacy_vs_modern( + &self, + legacy: &'a NameBinding<'a>, + modern: &'a NameBinding<'a>, + ) -> bool { + // Some non-controversial subset of ambiguities "modern macro name" vs "macro_rules" + // is disambiguated to mitigate regressions from macro modularization. + // Scoping for `macro_rules` behaves like scoping for `let` at module level, in general. + match (self.binding_parent_modules.get(&PtrKey(legacy)), + self.binding_parent_modules.get(&PtrKey(modern))) { + (Some(legacy), Some(modern)) => + legacy.normal_ancestor_id == modern.normal_ancestor_id && + modern.is_ancestor_of(legacy), + _ => false, + } + } + fn report_ambiguity_error(&self, ident: Ident, b1: &NameBinding, b2: &NameBinding) { let participle = |is_import: bool| if is_import { "imported" } else { "defined" }; let msg1 = diff --git a/src/librustc_resolve/macros.rs b/src/librustc_resolve/macros.rs index b320ef032f258..6aac4c7945c78 100644 --- a/src/librustc_resolve/macros.rs +++ b/src/librustc_resolve/macros.rs @@ -956,11 +956,13 @@ impl<'a, 'cl> Resolver<'a, 'cl> { }, (Some(legacy_binding), Ok((binding, FromPrelude(from_prelude)))) if legacy_binding.def() != binding.def_ignoring_ambiguity() && - (!from_prelude || + (!from_prelude && + !self.disambiguate_legacy_vs_modern(legacy_binding, binding) || legacy_binding.may_appear_after(parent_scope.expansion, binding)) => { self.report_ambiguity_error(ident, legacy_binding, binding); }, // OK, non-macro-expanded legacy wins over prelude even if defs are different + // Also, non-macro-expanded legacy wins over modern from the same module // Also, legacy and modern can co-exist if their defs are same (Some(legacy_binding), Ok(_)) | // OK, unambiguous resolution @@ -1096,6 +1098,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> { let def = Def::Macro(def_id, MacroKind::Bang); let vis = ty::Visibility::Invisible; // Doesn't matter for legacy bindings let binding = (def, vis, item.span, expansion).to_name_binding(self.arenas); + self.set_binding_parent_module(binding, self.current_module); let legacy_binding = self.arenas.alloc_legacy_binding(LegacyBinding { parent_legacy_scope: *current_legacy_scope, binding, ident }); diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index e689e6d70fdf4..4cf1a0d89356e 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -478,6 +478,7 @@ impl<'a, 'crateloader> Resolver<'a, 'crateloader> { binding: &'a NameBinding<'a>) -> Result<(), &'a NameBinding<'a>> { self.check_reserved_macro_name(ident, ns); + self.set_binding_parent_module(binding, module); self.update_resolution(module, ident, ns, |this, resolution| { if let Some(old_binding) = resolution.binding { if binding.is_glob_import() { diff --git a/src/test/ui/imports/macros.rs b/src/test/ui/imports/macros.rs index 47ab8fc6c2f75..f7dc7ac9eac81 100644 --- a/src/test/ui/imports/macros.rs +++ b/src/test/ui/imports/macros.rs @@ -45,7 +45,7 @@ mod m3 { mod m4 { macro_rules! m { () => {} } use two_macros::m; - m!(); //~ ERROR ambiguous + m!(); } fn main() {} diff --git a/src/test/ui/imports/macros.stderr b/src/test/ui/imports/macros.stderr index c54101fc6e6a8..209d449dfd840 100644 --- a/src/test/ui/imports/macros.stderr +++ b/src/test/ui/imports/macros.stderr @@ -1,20 +1,3 @@ -error[E0659]: `m` is ambiguous - --> $DIR/macros.rs:48:5 - | -LL | m!(); //~ ERROR ambiguous - | ^ ambiguous name - | -note: `m` could refer to the name defined here - --> $DIR/macros.rs:46:5 - | -LL | macro_rules! m { () => {} } - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ -note: `m` could also refer to the name imported here - --> $DIR/macros.rs:47:9 - | -LL | use two_macros::m; - | ^^^^^^^^^^^^^ - error[E0659]: `m` is ambiguous --> $DIR/macros.rs:26:5 | @@ -51,6 +34,6 @@ LL | use two_macros::m; | ^^^^^^^^^^^^^ = note: macro-expanded macro imports do not shadow -error: aborting due to 3 previous errors +error: aborting due to 2 previous errors For more information about this error, try `rustc --explain E0659`. diff --git a/src/test/ui/macros/ambiguity-legacy-vs-modern.rs b/src/test/ui/macros/ambiguity-legacy-vs-modern.rs new file mode 100644 index 0000000000000..216b9dd0526d2 --- /dev/null +++ b/src/test/ui/macros/ambiguity-legacy-vs-modern.rs @@ -0,0 +1,46 @@ +// Some non-controversial subset of ambiguities "modern macro name" vs "macro_rules" +// is disambiguated to mitigate regressions from macro modularization. +// Scoping for `macro_rules` behaves like scoping for `let` at module level, in general. + +#![feature(decl_macro)] + +fn same_unnamed_mod() { + macro m() { 0 } + + macro_rules! m { () => (()) } + + m!() // OK +} + +fn nested_unnamed_mod() { + macro m() { 0 } + + { + macro_rules! m { () => (()) } + + m!() // OK + } +} + +fn nested_unnamed_mod_fail() { + macro_rules! m { () => (()) } + + { + macro m() { 0 } + + m!() //~ ERROR `m` is ambiguous + } +} + +fn nexted_named_mod_fail() { + macro m() { 0 } + + #[macro_use] + mod inner { + macro_rules! m { () => (()) } + } + + m!() //~ ERROR `m` is ambiguous +} + +fn main() {} diff --git a/src/test/ui/macros/ambiguity-legacy-vs-modern.stderr b/src/test/ui/macros/ambiguity-legacy-vs-modern.stderr new file mode 100644 index 0000000000000..b5e1e751737b4 --- /dev/null +++ b/src/test/ui/macros/ambiguity-legacy-vs-modern.stderr @@ -0,0 +1,37 @@ +error[E0659]: `m` is ambiguous + --> $DIR/ambiguity-legacy-vs-modern.rs:31:9 + | +LL | m!() //~ ERROR `m` is ambiguous + | ^ ambiguous name + | +note: `m` could refer to the name defined here + --> $DIR/ambiguity-legacy-vs-modern.rs:26:5 + | +LL | macro_rules! m { () => (()) } + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +note: `m` could also refer to the name defined here + --> $DIR/ambiguity-legacy-vs-modern.rs:29:9 + | +LL | macro m() { 0 } + | ^^^^^^^^^^^^^^^ + +error[E0659]: `m` is ambiguous + --> $DIR/ambiguity-legacy-vs-modern.rs:43:5 + | +LL | m!() //~ ERROR `m` is ambiguous + | ^ ambiguous name + | +note: `m` could refer to the name defined here + --> $DIR/ambiguity-legacy-vs-modern.rs:40:9 + | +LL | macro_rules! m { () => (()) } + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +note: `m` could also refer to the name defined here + --> $DIR/ambiguity-legacy-vs-modern.rs:36:5 + | +LL | macro m() { 0 } + | ^^^^^^^^^^^^^^^ + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0659`. From 310a9210b37b1d59837340e7d44bb19edd76a9ae Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Tue, 25 Sep 2018 17:06:18 +0200 Subject: [PATCH 2/3] incr.comp.: Don't automatically enable -Zshare-generics for incr.comp. builds. --- src/librustc/session/config.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs index 4c0eeba744150..da3d0d5ed3fe7 100644 --- a/src/librustc/session/config.rs +++ b/src/librustc/session/config.rs @@ -646,7 +646,6 @@ impl Options { match self.debugging_opts.share_generics { Some(setting) => setting, None => { - self.incremental.is_some() || match self.optimize { OptLevel::No | OptLevel::Less | From e3f69317a0aab547ec98b5db93130cc69f52e20d Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 2 Oct 2018 20:47:57 +0200 Subject: [PATCH 3/3] do not normalize non-scalar constants to a ConstValue::ScalarPair --- src/librustc_mir/const_eval.rs | 14 ++++++++++++-- src/test/ui/consts/const-eval/union-ice.rs | 2 +- src/test/ui/consts/const-eval/union-ice.stderr | 8 +++++--- src/test/ui/issues/issue-54387.rs | 12 ++++++++++++ 4 files changed, 30 insertions(+), 6 deletions(-) create mode 100644 src/test/ui/issues/issue-54387.rs diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index 82cc1b7f66166..dd6f5fc044618 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -17,7 +17,7 @@ use rustc::hir::{self, def_id::DefId}; use rustc::mir::interpret::ConstEvalErr; use rustc::mir; use rustc::ty::{self, TyCtxt, Instance, query::TyCtxtAt}; -use rustc::ty::layout::{LayoutOf, TyLayout}; +use rustc::ty::layout::{self, LayoutOf, TyLayout}; use rustc::ty::subst::Subst; use rustc_data_structures::indexed_vec::IndexVec; @@ -90,8 +90,18 @@ pub fn eval_promoted<'a, 'mir, 'tcx>( pub fn op_to_const<'tcx>( ecx: &EvalContext<'_, '_, 'tcx, CompileTimeEvaluator>, op: OpTy<'tcx>, - normalize: bool, + may_normalize: bool, ) -> EvalResult<'tcx, &'tcx ty::Const<'tcx>> { + // We do not normalize just any data. Only scalar layout and fat pointers. + let normalize = may_normalize + && match op.layout.abi { + layout::Abi::Scalar(..) => true, + layout::Abi::ScalarPair(..) => { + // Must be a fat pointer + op.layout.ty.builtin_deref(true).is_some() + }, + _ => false, + }; let normalized_op = if normalize { ecx.try_read_value(op)? } else { diff --git a/src/test/ui/consts/const-eval/union-ice.rs b/src/test/ui/consts/const-eval/union-ice.rs index 0cdb78c97803c..5d50004e5549d 100644 --- a/src/test/ui/consts/const-eval/union-ice.rs +++ b/src/test/ui/consts/const-eval/union-ice.rs @@ -22,7 +22,7 @@ const UNION: DummyUnion = DummyUnion { field1: 1065353216 }; const FIELD3: Field3 = unsafe { UNION.field3 }; //~ ERROR this constant cannot be used -const FIELD_PATH: Struct = Struct { //~ ERROR this constant cannot be used +const FIELD_PATH: Struct = Struct { //~ ERROR this constant likely exhibits undefined behavior a: 42, b: unsafe { UNION.field3 }, }; diff --git a/src/test/ui/consts/const-eval/union-ice.stderr b/src/test/ui/consts/const-eval/union-ice.stderr index e8a7b2f500561..ec51802681e0d 100644 --- a/src/test/ui/consts/const-eval/union-ice.stderr +++ b/src/test/ui/consts/const-eval/union-ice.stderr @@ -6,14 +6,16 @@ LL | const FIELD3: Field3 = unsafe { UNION.field3 }; //~ ERROR this constant can | = note: #[deny(const_err)] on by default -error: this constant cannot be used +error[E0080]: this constant likely exhibits undefined behavior --> $DIR/union-ice.rs:25:1 | -LL | / const FIELD_PATH: Struct = Struct { //~ ERROR this constant cannot be used +LL | / const FIELD_PATH: Struct = Struct { //~ ERROR this constant likely exhibits undefined behavior LL | | a: 42, LL | | b: unsafe { UNION.field3 }, LL | | }; - | |__^ attempted to read undefined bytes + | |__^ type validation failed: encountered undefined bytes at .b + | + = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior error[E0080]: this constant likely exhibits undefined behavior --> $DIR/union-ice.rs:35:1 diff --git a/src/test/ui/issues/issue-54387.rs b/src/test/ui/issues/issue-54387.rs new file mode 100644 index 0000000000000..ac1033add0ee4 --- /dev/null +++ b/src/test/ui/issues/issue-54387.rs @@ -0,0 +1,12 @@ +// compile-pass + +pub struct GstRc { + _obj: *const (), + _borrowed: bool, +} + +const FOO: Option = None; + +fn main() { + let _meh = FOO; +}