mathieu/granite-rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Auto merge of #56392 - petrochenkov:regensym, r=oli-obk

Browse source 
Delay gensym creation for "underscore items" (`use foo as _`/`const _`) until name resolution

So they cannot be cloned by macros. See https://github.com/rust-lang/rust/pull/56303 for the discussion.

Mostly fix cross-crate use of underscore items by inverting the "gensyms are lost in metadata" bug as described in https://github.com/rust-lang/rust/pull/56303#issuecomment-442464695.
Fix unused import warnings for single-segment imports (first commit) and `use crate_name as _` imports (as specified in https://github.com/rust-lang/rust/pull/56303#issuecomment-442274118).
Prohibit accidentally implemented `static _: TYPE = EXPR;` (cc https://github.com/rust-lang/rust/pull/55983).
Add more tests for `use foo as _` imports.
This commit is contained in:
bors 2018-12-06 15:08:11 +00:00
commit 4bb5d35659
19 changed files with 212 additions and 70 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

19
src/librustc_resolve/build_reduced_graph.rs
View file

@ -157,7 +157,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
};
match use_tree.kind {
ast::UseTreeKind::Simple(rename, ..) => {
let mut ident = use_tree.ident();
let mut ident = use_tree.ident().gensym_if_underscore();
let mut module_path = prefix;
let mut source = module_path.pop().unwrap();
let mut type_ns_only = false;
@ -230,13 +230,18 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
}
let subclass = SingleImport {
target: ident,
source: source.ident,
result: PerNS {
target: ident,
source_bindings: PerNS {
type_ns: Cell::new(Err(Undetermined)),
value_ns: Cell::new(Err(Undetermined)),
macro_ns: Cell::new(Err(Undetermined)),
},
target_bindings: PerNS {
type_ns: Cell::new(None),
value_ns: Cell::new(None),
macro_ns: Cell::new(None),
},
type_ns_only,
};
self.add_import_directive(
@ -329,7 +334,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
fn build_reduced_graph_for_item(&mut self, item: &Item, parent_scope: ParentScope<'a>) {
let parent = parent_scope.module;
let expansion = parent_scope.expansion;
let ident = item.ident;
let ident = item.ident.gensym_if_underscore();
let sp = item.span;
let vis = self.resolve_visibility(&item.vis);
@ -623,7 +628,11 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
/// Builds the reduced graph for a single item in an external crate.
fn build_reduced_graph_for_external_crate_def(&mut self, parent: Module<'a>, child: Export) {
let Export { ident, def, vis, span, .. } = child;
let Export { ident, def, vis, span } = child;
// FIXME: We shouldn't create the gensym here, it should come from metadata,
// but metadata cannot encode gensyms currently, so we create it here.
// This is only a guess, two equivalent idents may incorrectly get different gensyms here.
let ident = ident.gensym_if_underscore();
let def_id = def.def_id();
let expansion = Mark::root(); // FIXME(jseyfried) intercrate hygiene
match def {

7
src/librustc_resolve/lib.rs
View file

@ -1519,8 +1519,12 @@ pub struct Resolver<'a, 'b: 'a> {
/// The current self item if inside an ADT (used for better errors).
current_self_item: Option<NodeId>,
/// FIXME: Refactor things so that this is passed through arguments and not resolver.
/// FIXME: Refactor things so that these fields are passed through arguments and not resolver.
/// We are resolving a last import segment during import validation.
last_import_segment: bool,
/// This binding should be ignored during in-module resolution, so that we don't get
/// "self-confirming" import resolutions during import validation.
blacklisted_binding: Option<&'a NameBinding<'a>>,
/// The idents for the primitive types.
primitive_type_table: PrimitiveTypeTable,
@ -1871,6 +1875,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
current_self_type: None,
current_self_item: None,
last_import_segment: false,
blacklisted_binding: None,
primitive_type_table: PrimitiveTypeTable::new(),

8
src/librustc_resolve/macros.rs
View file

@ -977,12 +977,14 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
let what = self.binding_description(binding, ident,
flags.contains(Flags::MISC_FROM_PRELUDE));
let note_msg = format!("this import refers to {what}", what = what);
if binding.span.is_dummy() {
let label_span = if binding.span.is_dummy() {
err.note(&note_msg);
ident.span
} else {
err.span_note(binding.span, &note_msg);
err.span_label(binding.span, "not an extern crate passed with `--extern`");
}
binding.span
};
err.span_label(label_span, "not an extern crate passed with `--extern`");
err.emit();
}

61
src/librustc_resolve/resolve_imports.rs
View file

@ -42,9 +42,15 @@ use std::{mem, ptr};
#[derive(Clone, Debug)]
pub enum ImportDirectiveSubclass<'a> {
SingleImport {
target: Ident,
/// `source` in `use prefix::source as target`.
source: Ident,
result: PerNS<Cell<Result<&'a NameBinding<'a>, Determinacy>>>,
/// `target` in `use prefix::source as target`.
target: Ident,
/// Bindings to which `source` refers to.
source_bindings: PerNS<Cell<Result<&'a NameBinding<'a>, Determinacy>>>,
/// Bindings introduced by `target`.
target_bindings: PerNS<Cell<Option<&'a NameBinding<'a>>>>,
/// `true` for `...::{self [as target]}` imports, `false` otherwise.
type_ns_only: bool,
},
GlobImport {
@ -227,6 +233,11 @@ impl<'a, 'crateloader> Resolver<'a, 'crateloader> {
}
let check_usable = |this: &mut Self, binding: &'a NameBinding<'a>| {
if let Some(blacklisted_binding) = this.blacklisted_binding {
if ptr::eq(binding, blacklisted_binding) {
return Err((Determined, Weak::No));
}
}
// `extern crate` are always usable for backwards compatibility, see issue #37020,
// remove this together with `PUB_USE_OF_PRIVATE_EXTERN_CRATE`.
let usable = this.is_accessible(binding.vis) || binding.is_extern_crate();
@ -642,10 +653,10 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
if let Some((span, err, note)) = self.finalize_import(import) {
errors = true;
if let SingleImport { source, ref result, .. } = import.subclass {
if let SingleImport { source, ref source_bindings, .. } = import.subclass {
if source.name == "self" {
// Silence `unresolved import` error if E0429 is already emitted
if let Err(Determined) = result.value_ns.get() {
if let Err(Determined) = source_bindings.value_ns.get() {
continue;
}
}
@ -765,9 +776,11 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
};
directive.imported_module.set(Some(module));
let (source, target, result, type_ns_only) = match directive.subclass {
SingleImport { source, target, ref result, type_ns_only } =>
(source, target, result, type_ns_only),
let (source, target, source_bindings, target_bindings, type_ns_only) =
match directive.subclass {
SingleImport { source, target, ref source_bindings,
ref target_bindings, type_ns_only } =>
(source, target, source_bindings, target_bindings, type_ns_only),
GlobImport { .. } => {
self.resolve_glob_import(directive);
return true;
@ -777,7 +790,7 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
let mut indeterminate = false;
self.per_ns(|this, ns| if !type_ns_only || ns == TypeNS {
if let Err(Undetermined) = result[ns].get() {
if let Err(Undetermined) = source_bindings[ns].get() {
// For better failure detection, pretend that the import will
// not define any names while resolving its module path.
let orig_vis = directive.vis.replace(ty::Visibility::Invisible);
@ -786,13 +799,13 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
);
directive.vis.set(orig_vis);
result[ns].set(binding);
source_bindings[ns].set(binding);
} else {
return
};
let parent = directive.parent_scope.module;
match result[ns].get() {
match source_bindings[ns].get() {
Err(Undetermined) => indeterminate = true,
Err(Determined) => {
this.update_resolution(parent, target, ns, |_, resolution| {
@ -810,6 +823,7 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
}
Ok(binding) => {
let imported_binding = this.import(binding, directive);
target_bindings[ns].set(Some(imported_binding));
let conflict = this.try_define(parent, target, ns, imported_binding);
if let Err(old_binding) = conflict {
this.report_conflict(parent, target, ns, imported_binding, old_binding);
@ -879,8 +893,11 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
PathResult::Indeterminate | PathResult::NonModule(..) => unreachable!(),
};
let (ident, result, type_ns_only) = match directive.subclass {
SingleImport { source, ref result, type_ns_only, .. } => (source, result, type_ns_only),
let (ident, target, source_bindings, target_bindings, type_ns_only) =
match directive.subclass {
SingleImport { source, target, ref source_bindings,
ref target_bindings, type_ns_only } =>
(source, target, source_bindings, target_bindings, type_ns_only),
GlobImport { is_prelude, ref max_vis } => {
if directive.module_path.len() <= 1 {
// HACK(eddyb) `lint_if_path_starts_with_module` needs at least
@ -919,20 +936,28 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
let mut all_ns_err = true;
self.per_ns(|this, ns| if !type_ns_only || ns == TypeNS {
let orig_vis = directive.vis.replace(ty::Visibility::Invisible);
let orig_blacklisted_binding =
mem::replace(&mut this.blacklisted_binding, target_bindings[ns].get());
let orig_last_import_segment = mem::replace(&mut this.last_import_segment, true);
let binding = this.resolve_ident_in_module(
module, ident, ns, Some(&directive.parent_scope), true, directive.span
);
this.last_import_segment = orig_last_import_segment;
this.blacklisted_binding = orig_blacklisted_binding;
directive.vis.set(orig_vis);
match binding {
Ok(binding) => {
// Consistency checks, analogous to `finalize_current_module_macro_resolutions`.
let initial_def = result[ns].get().map(|initial_binding| {
let initial_def = source_bindings[ns].get().map(|initial_binding| {
all_ns_err = false;
this.record_use(ident, ns, initial_binding,
directive.module_path.is_empty());
if let Some(target_binding) = target_bindings[ns].get() {
if target.name == "_" &&
initial_binding.is_extern_crate() && !initial_binding.is_import() {
this.record_use(ident, ns, target_binding,
directive.module_path.is_empty());
}
}
initial_binding.def_ignoring_ambiguity()
});
let def = binding.def_ignoring_ambiguity();
@ -1034,7 +1059,7 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
let mut reexport_error = None;
let mut any_successful_reexport = false;
self.per_ns(|this, ns| {
if let Ok(binding) = result[ns].get() {
if let Ok(binding) = source_bindings[ns].get() {
let vis = directive.vis.get();
if !binding.pseudo_vis().is_at_least(vis, &*this) {
reexport_error = Some((ns, binding));
@ -1078,7 +1103,7 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
let mut full_path = directive.module_path.clone();
full_path.push(Segment::from_ident(ident));
self.per_ns(|this, ns| {
if let Ok(binding) = result[ns].get() {
if let Ok(binding) = source_bindings[ns].get() {
this.lint_if_path_starts_with_module(
directive.crate_lint(),
&full_path,
@ -1092,7 +1117,7 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
// Record what this import resolves to for later uses in documentation,
// this may resolve to either a value or a type, but for documentation
// purposes it's good enough to just favor one over the other.
self.per_ns(|this, ns| if let Some(binding) = result[ns].get().ok() {
self.per_ns(|this, ns| if let Some(binding) = source_bindings[ns].get().ok() {
let mut def = binding.def();
if let Def::Macro(def_id, _) = def {
// `DefId`s from the "built-in macro crate" should not leak from resolve because

27
src/libsyntax/parse/parser.rs
View file

@ -2017,6 +2017,17 @@ impl<'a> Parser<'a> {
}
}
fn parse_ident_or_underscore(&mut self) -> PResult<'a, ast::Ident> {
match self.token {
token::Ident(ident, false) if ident.name == keywords::Underscore.name() => {
let span = self.span;
self.bump();
Ok(Ident::new(ident.name, span))
}
_ => self.parse_ident(),
}
}
/// Parses qualified path.
/// Assumes that the leading `<` has been parsed already.
///
@ -6434,13 +6445,7 @@ impl<'a> Parser<'a> {
}
fn parse_item_const(&mut self, m: Option<Mutability>) -> PResult<'a, ItemInfo> {
let id = match self.token {
token::Ident(ident, false) if ident.name == keywords::Underscore.name() => {
self.bump(); // `_`
ident.gensym()
},
_ => self.parse_ident()?,
};
let id = if m.is_none() { self.parse_ident_or_underscore() } else { self.parse_ident() }?;
self.expect(&token::Colon)?;
let ty = self.parse_ty()?;
self.expect(&token::Eq)?;
@ -7725,13 +7730,7 @@ impl<'a> Parser<'a> {
fn parse_rename(&mut self) -> PResult<'a, Option<Ident>> {
if self.eat_keyword(keywords::As) {
match self.token {
token::Ident(ident, false) if ident.name == keywords::Underscore.name() => {
self.bump(); // `_`
Ok(Some(ident.gensym()))
}
_ => self.parse_ident().map(Some),
}
self.parse_ident_or_underscore().map(Some)
} else {
Ok(None)
}

6
src/libsyntax_pos/symbol.rs
View file

@ -80,6 +80,10 @@ impl Ident {
Ident::new(self.name.gensymed(), self.span)
}
pub fn gensym_if_underscore(self) -> Ident {
if self.name == keywords::Underscore.name() { self.gensym() } else { self }
}
pub fn as_str(self) -> LocalInternedString {
self.name.as_str()
}
@ -473,7 +477,7 @@ impl Ident {
// We see this identifier in a normal identifier position, like variable name or a type.
// How was it written originally? Did it use the raw form? Let's try to guess.
pub fn is_raw_guess(self) -> bool {
self.name != keywords::Invalid.name() &&
self.name != keywords::Invalid.name() && self.name != keywords::Underscore.name() &&
self.is_reserved() && !self.is_path_segment_keyword()
}
}

18
src/test/ui/feature-gates/feature-gate-uniform-paths.stderr
View file

@ -25,11 +25,7 @@ LL | use inline; //~ ERROR imports can only refer to extern crate names
| ^^^^^^ not an extern crate passed with `--extern`
|
= help: add #![feature(uniform_paths)] to the crate attributes to enable
note: this import refers to the built-in attribute imported here
--> $DIR/feature-gate-uniform-paths.rs:21:5
|
LL | use inline; //~ ERROR imports can only refer to extern crate names
| ^^^^^^
= note: this import refers to a built-in attribute
error[E0658]: imports can only refer to extern crate names passed with `--extern` on stable channel (see issue #53130)
--> $DIR/feature-gate-uniform-paths.rs:23:5
@ -38,11 +34,7 @@ LL | use Vec; //~ ERROR imports can only refer to extern crate names
| ^^^ not an extern crate passed with `--extern`
|
= help: add #![feature(uniform_paths)] to the crate attributes to enable
note: this import refers to the struct imported here
--> $DIR/feature-gate-uniform-paths.rs:23:5
|
LL | use Vec; //~ ERROR imports can only refer to extern crate names
| ^^^
= note: this import refers to a struct from prelude
error[E0658]: imports can only refer to extern crate names passed with `--extern` on stable channel (see issue #53130)
--> $DIR/feature-gate-uniform-paths.rs:25:5
@ -51,11 +43,7 @@ LL | use vec; //~ ERROR imports can only refer to extern crate names
| ^^^ not an extern crate passed with `--extern`
|
= help: add #![feature(uniform_paths)] to the crate attributes to enable
note: this import refers to the macro imported here
--> $DIR/feature-gate-uniform-paths.rs:25:5
|
LL | use vec; //~ ERROR imports can only refer to extern crate names
| ^^^
= note: this import refers to a macro from prelude
error: aborting due to 4 previous errors

1
src/test/ui/underscore_const_names_feature_gate.rs → src/test/ui/feature-gates/underscore_const_names_feature_gate.rs
View file

@ -9,6 +9,5 @@
// except according to those terms.
const _: () = (); //~ ERROR is unstable
static _: () = (); //~ ERROR is unstable
fn main() {}

10
src/test/ui/underscore_const_names_feature_gate.stderr → src/test/ui/feature-gates/underscore_const_names_feature_gate.stderr
View file

@ -6,14 +6,6 @@ LL | const _: () = (); //~ ERROR is unstable
|
= help: add #![feature(underscore_const_names)] to the crate attributes to enable
error[E0658]: naming constants with `_` is unstable (see issue #54912)
--> $DIR/underscore_const_names_feature_gate.rs:12:1
|
LL | static _: () = (); //~ ERROR is unstable
| ^^^^^^^^^^^^^^^^^^
|
= help: add #![feature(underscore_const_names)] to the crate attributes to enable
error: aborting due to 2 previous errors
error: aborting due to previous error
For more information about this error, try `rustc --explain E0658`.

3
src/test/ui/parser/underscore_static.rs Normal file
View file

@ -0,0 +1,3 @@
// compile-flags: -Z parse-only
static _: () = (); //~ ERROR expected identifier, found reserved identifier `_`

8
src/test/ui/parser/underscore_static.stderr Normal file
View file

@ -0,0 +1,8 @@
error: expected identifier, found reserved identifier `_`
--> $DIR/underscore_static.rs:3:8
|
LL | static _: () = (); //~ ERROR expected identifier, found reserved identifier `_`
| ^ expected identifier, found reserved identifier
error: aborting due to previous error

14
src/test/ui/rfc-2166-underscore-imports/auxiliary/duplicate.rs Normal file
View file

@ -0,0 +1,14 @@
// force-host
// no-prefer-dynamic
#![crate_type = "proc-macro"]
extern crate proc_macro;
use proc_macro::*;
#[proc_macro_attribute]
pub fn duplicate(_: TokenStream, input: TokenStream) -> TokenStream {
let clone = input.clone();
input.into_iter().chain(clone.into_iter()).collect()
}

22
src/test/ui/rfc-2166-underscore-imports/auxiliary/underscore-imports.rs Normal file
View file

@ -0,0 +1,22 @@
#![feature(underscore_imports)]
#[macro_export]
macro_rules! do_nothing {
() => ()
}
mod m1 {
pub trait InScope1 {
fn in_scope1(&self) {}
}
impl InScope1 for () {}
}
mod m2 {
pub trait InScope2 {
fn in_scope2(&self) {}
}
impl InScope2 for () {}
}
pub use m1::InScope1 as _;
pub use m2::InScope2 as _;

6
src/test/ui/rfc-2166-underscore-imports/basic.rs
View file

@ -9,10 +9,16 @@
// except according to those terms.
// compile-pass
// aux-build:underscore-imports.rs
#![feature(underscore_imports)]
#![warn(unused_imports, unused_extern_crates)]
#[macro_use]
extern crate underscore_imports as _;
do_nothing!(); // OK
struct S;
mod m {

6
src/test/ui/rfc-2166-underscore-imports/basic.stderr
View file

@ -1,17 +1,17 @@
warning: unused import: `m::Tr1 as _`
--> $DIR/basic.rs:31:9
--> $DIR/basic.rs:37:9
|
LL | use m::Tr1 as _; //~ WARN unused import
| ^^^^^^^^^^^
|
note: lint level defined here
--> $DIR/basic.rs:14:9
--> $DIR/basic.rs:15:9
|
LL | #![warn(unused_imports, unused_extern_crates)]
| ^^^^^^^^^^^^^^
warning: unused import: `S as _`
--> $DIR/basic.rs:32:9
--> $DIR/basic.rs:38:9
|
LL | use S as _; //~ WARN unused import
| ^^^^^^

17
src/test/ui/rfc-2166-underscore-imports/duplicate.rs Normal file
View file

@ -0,0 +1,17 @@
// compile-pass
// aux-build:duplicate.rs
#![feature(underscore_imports)]
extern crate duplicate;
#[duplicate::duplicate]
use main as _; // OK
macro_rules! duplicate {
($item: item) => { $item $item }
}
duplicate!(use std as _;); // OK
fn main() {}

11
src/test/ui/rfc-2166-underscore-imports/intercrate.rs Normal file
View file

@ -0,0 +1,11 @@
// compile-pass
// aux-build:underscore-imports.rs
extern crate underscore_imports;
use underscore_imports::*;
fn main() {
().in_scope1();
().in_scope2();
}

18
src/test/ui/rfc-2166-underscore-imports/unused-2018.rs Normal file
View file

@ -0,0 +1,18 @@
// edition:2018
#![feature(underscore_imports)]
#![deny(unused_imports)]
mod multi_segment {
use core::any; //~ ERROR unused import: `core::any`
}
mod single_segment {
use core; //~ ERROR unused import: `core`
}
mod single_segment_underscore {
use core as _; // OK
}
fn main() {}

20
src/test/ui/rfc-2166-underscore-imports/unused-2018.stderr Normal file
View file

@ -0,0 +1,20 @@
error: unused import: `core::any`
--> $DIR/unused-2018.rs:7:9
|
LL | use core::any; //~ ERROR unused import: `core::any`
| ^^^^^^^^^
|
note: lint level defined here
--> $DIR/unused-2018.rs:4:9
|
LL | #![deny(unused_imports)]
| ^^^^^^^^^^^^^^
error: unused import: `core`
--> $DIR/unused-2018.rs:11:9
|
LL | use core; //~ ERROR unused import: `core`
| ^^^^
error: aborting due to 2 previous errors