Skip to content

Commit

Permalink
Auto merge of #56759 - petrochenkov:prestabuni, r=nikomatsakis
Browse files Browse the repository at this point in the history
Stabilize `uniform_paths`

Address all the things described in #56417.

Assign visibilities to `macro_rules` items - `pub` to `macro_export`-ed macros and `pub(crate)` to non-exported macros, these visibilities determine how far these macros can be reexported with `use`.

Prohibit use of reexported inert attributes and "tool modules", after renaming (e.g. `use inline as imported_inline`) their respective tools and even compiler passes won't be able to recognize and properly check them.

Also turn use of uniform paths in 2015 macros into an error, I'd prefer to neither remove nor stabilize them right away because I still want to make some experiments in this area (uniform path fallback was added to 2015 macros used on 2018 edition in #56053 (comment)).

UPDATE: The last commit also stabilizes the feature `uniform_paths`.

Closes #53130
Closes #55618
Closes #56326
Closes #56398
Closes #56417
Closes #56821
Closes #57252
Closes #57422
  • Loading branch information
bors committed Jan 12, 2019
2 parents d6525ef + 250935d commit 75a369c
Show file tree
Hide file tree
Showing 38 changed files with 229 additions and 263 deletions.
2 changes: 1 addition & 1 deletion src/librustc/hir/def.rs
Expand Up @@ -240,7 +240,7 @@ impl CtorKind {
}

impl NonMacroAttrKind {
fn descr(self) -> &'static str {
pub fn descr(self) -> &'static str {
match self {
NonMacroAttrKind::Builtin => "built-in attribute",
NonMacroAttrKind::Tool => "tool attribute",
Expand Down
33 changes: 28 additions & 5 deletions src/librustc_resolve/lib.rs
Expand Up @@ -67,7 +67,7 @@ use syntax_pos::{Span, DUMMY_SP, MultiSpan};
use errors::{Applicability, DiagnosticBuilder, DiagnosticId};

use std::cell::{Cell, RefCell};
use std::{cmp, fmt, iter, ptr};
use std::{cmp, fmt, iter, mem, ptr};
use std::collections::BTreeSet;
use std::mem::replace;
use rustc_data_structures::ptr_key::PtrKey;
Expand Down Expand Up @@ -2375,11 +2375,27 @@ impl<'a> Resolver<'a> {
ast::UseTreeKind::Simple(..) if segments.len() == 1 => &[TypeNS, ValueNS][..],
_ => &[TypeNS],
};
let report_error = |this: &Self, ns| {
let what = if ns == TypeNS { "type parameters" } else { "local variables" };
this.session.span_err(ident.span, &format!("imports cannot refer to {}", what));
};

for &ns in nss {
if let Some(LexicalScopeBinding::Def(..)) =
self.resolve_ident_in_lexical_scope(ident, ns, None, use_tree.prefix.span) {
let what = if ns == TypeNS { "type parameters" } else { "local variables" };
self.session.span_err(ident.span, &format!("imports cannot refer to {}", what));
match self.resolve_ident_in_lexical_scope(ident, ns, None, use_tree.prefix.span) {
Some(LexicalScopeBinding::Def(..)) => {
report_error(self, ns);
}
Some(LexicalScopeBinding::Item(binding)) => {
let orig_blacklisted_binding =
mem::replace(&mut self.blacklisted_binding, Some(binding));
if let Some(LexicalScopeBinding::Def(..)) =
self.resolve_ident_in_lexical_scope(ident, ns, None,
use_tree.prefix.span) {
report_error(self, ns);
}
self.blacklisted_binding = orig_blacklisted_binding;
}
None => {}
}
}
} else if let ast::UseTreeKind::Nested(use_trees) = &use_tree.kind {
Expand Down Expand Up @@ -3874,6 +3890,13 @@ impl<'a> Resolver<'a> {
module = Some(ModuleOrUniformRoot::Module(next_module));
record_segment_def(self, def);
} else if def == Def::ToolMod && i + 1 != path.len() {
if binding.is_import() {
self.session.struct_span_err(
ident.span, "cannot use a tool module through an import"
).span_note(
binding.span, "the tool module imported here"
).emit();
}
let def = Def::NonMacroAttr(NonMacroAttrKind::Tool);
return PathResult::NonModule(PathResolution::new(def));
} else if def == Def::Err {
Expand Down
53 changes: 35 additions & 18 deletions src/librustc_resolve/macros.rs
Expand Up @@ -376,6 +376,7 @@ impl<'a> Resolver<'a> {
.push((path, path_span, kind, parent_scope.clone(), def.ok()));
}

self.prohibit_imported_non_macro_attrs(None, def.ok(), path_span);
def
} else {
let binding = self.early_resolve_ident_in_lexical_scope(
Expand All @@ -390,7 +391,9 @@ impl<'a> Resolver<'a> {
.push((path[0].ident, kind, parent_scope.clone(), binding.ok()));
}

binding.map(|binding| binding.def())
let def = binding.map(|binding| binding.def());
self.prohibit_imported_non_macro_attrs(binding.ok(), def.ok(), path_span);
def
}
}

Expand Down Expand Up @@ -828,27 +831,23 @@ impl<'a> Resolver<'a> {
// but its `Def` should coincide with a crate passed with `--extern`
// (otherwise there would be ambiguity) and we can skip feature error in this case.
'ok: {
if !is_import || self.session.features_untracked().uniform_paths {
if !is_import || !rust_2015 {
break 'ok;
}
if ns == TypeNS && use_prelude && self.extern_prelude_get(ident, true).is_some() {
break 'ok;
}
if rust_2015 {
let root_ident = Ident::new(keywords::PathRoot.name(), orig_ident.span);
let root_module = self.resolve_crate_root(root_ident);
if self.resolve_ident_in_module_ext(ModuleOrUniformRoot::Module(root_module),
orig_ident, ns, None, false, path_span)
.is_ok() {
break 'ok;
}
let root_ident = Ident::new(keywords::PathRoot.name(), orig_ident.span);
let root_module = self.resolve_crate_root(root_ident);
if self.resolve_ident_in_module_ext(ModuleOrUniformRoot::Module(root_module),
orig_ident, ns, None, false, path_span)
.is_ok() {
break 'ok;
}

let msg = "imports can only refer to extern crate names \
passed with `--extern` on stable channel";
let mut err = feature_err(&self.session.parse_sess, "uniform_paths",
ident.span, GateIssue::Language, msg);

let msg = "imports can only refer to extern crate names passed with \
`--extern` in macros originating from 2015 edition";
let mut err = self.session.struct_span_err(ident.span, msg);
let what = self.binding_description(binding, ident,
flags.contains(Flags::MISC_FROM_PRELUDE));
let note_msg = format!("this import refers to {what}", what = what);
Expand Down Expand Up @@ -977,6 +976,20 @@ impl<'a> Resolver<'a> {
}
}

fn prohibit_imported_non_macro_attrs(&self, binding: Option<&'a NameBinding<'a>>,
def: Option<Def>, span: Span) {
if let Some(Def::NonMacroAttr(kind)) = def {
if kind != NonMacroAttrKind::Tool && binding.map_or(true, |b| b.is_import()) {
let msg = format!("cannot use a {} through an import", kind.descr());
let mut err = self.session.struct_span_err(span, &msg);
if let Some(binding) = binding {
err.span_note(binding.span, &format!("the {} imported here", kind.descr()));
}
err.emit();
}
}
}

fn suggest_macro_name(&mut self, name: &str, kind: MacroKind,
err: &mut DiagnosticBuilder<'a>, span: Span) {
// First check if this is a locally-defined bang macro.
Expand Down Expand Up @@ -1073,17 +1086,21 @@ impl<'a> Resolver<'a> {
let ident = ident.modern();
self.macro_names.insert(ident);
let def = Def::Macro(def_id, MacroKind::Bang);
let vis = ty::Visibility::Invisible; // Doesn't matter for legacy bindings
let is_macro_export = attr::contains_name(&item.attrs, "macro_export");
let vis = if is_macro_export {
ty::Visibility::Public
} else {
ty::Visibility::Restricted(DefId::local(CRATE_DEF_INDEX))
};
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
});
*current_legacy_scope = LegacyScope::Binding(legacy_binding);
self.all_macros.insert(ident.name, def);
if attr::contains_name(&item.attrs, "macro_export") {
if is_macro_export {
let module = self.graph_root;
let vis = ty::Visibility::Public;
self.define(module, ident, MacroNS,
(def, vis, item.span, expansion, IsMacroExport));
} else {
Expand Down
5 changes: 5 additions & 0 deletions src/librustc_resolve/resolve_imports.rs
Expand Up @@ -223,6 +223,11 @@ impl<'a> Resolver<'a> {
}

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();
Expand Down
5 changes: 2 additions & 3 deletions src/libsyntax/feature_gate.rs
Expand Up @@ -434,9 +434,6 @@ declare_features! (
// support for arbitrary delimited token streams in non-macro attributes
(active, unrestricted_attribute_tokens, "1.30.0", Some(55208), None),

// Allows `use x::y;` to resolve through `self::x`, not just `::x`.
(active, uniform_paths, "1.30.0", Some(53130), None),

// Allows unsized rvalues at arguments and parameters.
(active, unsized_locals, "1.30.0", Some(48055), None),

Expand Down Expand Up @@ -687,6 +684,8 @@ declare_features! (
(accepted, cfg_attr_multi, "1.33.0", Some(54881), None),
// Top level or-patterns (`p | q`) in `if let` and `while let`.
(accepted, if_while_or_patterns, "1.33.0", Some(48215), None),
// Allows `use x::y;` to search `x` in the current scope.
(accepted, uniform_paths, "1.32.0", Some(53130), None),
);

// If you change this, please modify `src/doc/unstable-book` as well. You must
Expand Down
2 changes: 0 additions & 2 deletions src/test/run-pass/uniform-paths/auxiliary/issue-53691.rs
@@ -1,7 +1,5 @@
// edition:2018

#![feature(uniform_paths)]

mod m { pub fn f() {} }
mod n { pub fn g() {} }

Expand Down
10 changes: 5 additions & 5 deletions src/test/run-pass/uniform-paths/basic-nested.rs
@@ -1,12 +1,12 @@
// run-pass
#![allow(unused_imports)]
#![allow(non_camel_case_types)]
// This test is similar to `basic.rs`, but nested in modules.

// run-pass
// edition:2018

#![feature(decl_macro, uniform_paths)]
#![feature(decl_macro)]

// This test is similar to `basic.rs`, but nested in modules.
#![allow(unused_imports)]
#![allow(non_camel_case_types)]

mod foo {
// Test that ambiguity errors are not emitted between `self::test` and
Expand Down
6 changes: 2 additions & 4 deletions src/test/run-pass/uniform-paths/basic.rs
@@ -1,10 +1,8 @@
// run-pass
#![allow(unused_imports)]
#![allow(non_camel_case_types)]

// edition:2018

#![feature(uniform_paths)]
#![allow(unused_imports)]
#![allow(non_camel_case_types)]

// Test that ambiguity errors are not emitted between `self::test` and
// `::test`, assuming the latter (crate) is not in `extern_prelude`.
Expand Down
8 changes: 3 additions & 5 deletions src/test/run-pass/uniform-paths/macros-nested.rs
@@ -1,11 +1,9 @@
// run-pass
#![allow(non_camel_case_types)]
// This test is similar to `macros.rs`, but nested in modules.

// run-pass
// edition:2018

#![feature(uniform_paths)]

// This test is similar to `macros.rs`, but nested in modules.
#![allow(non_camel_case_types)]

mod foo {
// Test that ambiguity errors are not emitted between `self::test` and
Expand Down
8 changes: 3 additions & 5 deletions src/test/run-pass/uniform-paths/macros.rs
@@ -1,11 +1,9 @@
// run-pass
#![allow(non_camel_case_types)]
// This test is similar to `basic.rs`, but with macros defining local items.

// run-pass
// edition:2018

#![feature(uniform_paths)]

// This test is similar to `basic.rs`, but with macros defining local items.
#![allow(non_camel_case_types)]

// Test that ambiguity errors are not emitted between `self::test` and
// `::test`, assuming the latter (crate) is not in `extern_prelude`.
Expand Down
3 changes: 0 additions & 3 deletions src/test/run-pass/uniform-paths/same-crate.rs
@@ -1,9 +1,6 @@
// run-pass

// edition:2018

#![feature(uniform_paths)]

pub const A: usize = 0;

pub mod foo {
Expand Down
2 changes: 0 additions & 2 deletions src/test/ui/editions/edition-imports-2015.rs
Expand Up @@ -3,8 +3,6 @@
// aux-build:edition-imports-2018.rs
// aux-build:absolute.rs

#![feature(uniform_paths)]

#[macro_use]
extern crate edition_imports_2018;

Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/editions/edition-imports-2015.stderr
@@ -1,5 +1,5 @@
error: cannot glob-import all possible crates
--> $DIR/edition-imports-2015.rs:25:5
--> $DIR/edition-imports-2015.rs:23:5
|
LL | gen_glob!(); //~ ERROR cannot glob-import all possible crates
| ^^^^^^^^^^^^
Expand Down
@@ -1,4 +1,4 @@
error[E0658]: imports can only refer to extern crate names passed with `--extern` on stable channel (see issue #53130)
error: imports can only refer to extern crate names passed with `--extern` in macros originating from 2015 edition
--> <::edition_imports_2015::gen_gated macros>:1:50
|
LL | ( ) => { fn check_gated ( ) { enum E { A } use E :: * ; } }
Expand All @@ -9,7 +9,6 @@ LL | ( ) => { fn check_gated ( ) { enum E { A } use E :: * ; } }
LL | gen_gated!();
| ------------- not an extern crate passed with `--extern`
|
= help: add #![feature(uniform_paths)] to the crate attributes to enable
note: this import refers to the enum defined here
--> $DIR/edition-imports-virtual-2015-gated.rs:9:5
|
Expand All @@ -19,4 +18,3 @@ LL | gen_gated!();

error: aborting due to previous error

For more information about this error, try `rustc --explain E0658`.
19 changes: 0 additions & 19 deletions src/test/ui/feature-gates/feature-gate-uniform-paths.rs

This file was deleted.

50 changes: 0 additions & 50 deletions src/test/ui/feature-gates/feature-gate-uniform-paths.stderr

This file was deleted.

2 changes: 0 additions & 2 deletions src/test/ui/imports/issue-56125.rs
Expand Up @@ -2,8 +2,6 @@
// compile-flags:--extern issue_56125
// aux-build:issue-56125.rs

#![feature(uniform_paths)]

mod m1 {
use issue_56125::last_segment::*;
//~^ ERROR `issue_56125` is ambiguous
Expand Down

0 comments on commit 75a369c

Please sign in to comment.