Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix a few regressions from enabling macro modularization #53270

Merged
merged 3 commits into from
Aug 13, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/librustc_resolve/build_reduced_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -789,7 +789,6 @@ impl<'a, 'b, 'cl> BuildReducedGraphVisitor<'a, 'b, 'cl> {
fn visit_invoc(&mut self, id: ast::NodeId) -> &'b InvocationData<'b> {
let mark = id.placeholder_to_mark();
self.resolver.current_module.unresolved_invocations.borrow_mut().insert(mark);
self.resolver.unresolved_invocations_macro_export.insert(mark);
let invocation = self.resolver.invocations[&mark];
invocation.module.set(self.resolver.current_module);
invocation.legacy_scope.set(self.legacy_scope);
Expand Down
16 changes: 12 additions & 4 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1385,6 +1385,8 @@ pub struct Resolver<'a, 'b: 'a> {
use_injections: Vec<UseError<'a>>,
/// `use` injections for proc macros wrongly imported with #[macro_use]
proc_mac_errors: Vec<macros::ProcMacError>,
/// crate-local macro expanded `macro_export` referred to by a module-relative path
macro_expanded_macro_export_errors: BTreeSet<(Span, Span)>,

gated_errors: FxHashSet<Span>,
disallowed_shadowing: Vec<&'a LegacyBinding<'a>>,
Expand Down Expand Up @@ -1432,9 +1434,6 @@ pub struct Resolver<'a, 'b: 'a> {

/// Only supposed to be used by rustdoc, otherwise should be false.
pub ignore_extern_prelude_feature: bool,

/// Macro invocations in the whole crate that can expand into a `#[macro_export] macro_rules`.
unresolved_invocations_macro_export: FxHashSet<Mark>,
}

/// Nothing really interesting here, it just provides memory for the rest of the crate.
Expand Down Expand Up @@ -1706,6 +1705,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
proc_mac_errors: Vec::new(),
gated_errors: FxHashSet(),
disallowed_shadowing: Vec::new(),
macro_expanded_macro_export_errors: BTreeSet::new(),

arenas,
dummy_binding: arenas.alloc_name_binding(NameBinding {
Expand Down Expand Up @@ -1737,7 +1737,6 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
current_type_ascription: Vec::new(),
injected_crate: None,
ignore_extern_prelude_feature: false,
unresolved_invocations_macro_export: FxHashSet(),
}
}

Expand Down Expand Up @@ -4126,6 +4125,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
ns: Namespace,
module: Module<'a>,
found_traits: &mut Vec<TraitCandidate>) {
assert!(ns == TypeNS || ns == ValueNS);
let mut traits = module.traits.borrow_mut();
if traits.is_none() {
let mut collected_traits = Vec::new();
Expand Down Expand Up @@ -4371,6 +4371,14 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
self.report_proc_macro_import(krate);
let mut reported_spans = FxHashSet();

for &(span_use, span_def) in &self.macro_expanded_macro_export_errors {
let msg = "macro-expanded `macro_export` macros from the current crate \
cannot be referred to by absolute paths";
self.session.struct_span_err(span_use, msg)
.span_note(span_def, "the macro is defined here")
.emit();
}

for &AmbiguityError { span, name, b1, b2, lexical } in &self.ambiguity_errors {
if !reported_spans.insert(span) { continue }
let participle = |binding: &NameBinding| {
Expand Down
40 changes: 30 additions & 10 deletions src/librustc_resolve/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ use syntax::ext::expand::{AstFragment, Invocation, InvocationKind};
use syntax::ext::hygiene::{self, Mark};
use syntax::ext::tt::macro_rules;
use syntax::feature_gate::{self, feature_err, emit_feature_err, is_builtin_attr_name, GateIssue};
use syntax::feature_gate::EXPLAIN_DERIVE_UNDERSCORE;
use syntax::fold::{self, Folder};
use syntax::parse::parser::PathStyle;
use syntax::parse::token::{self, Token};
Expand Down Expand Up @@ -195,9 +196,7 @@ impl<'a, 'crateloader: 'a> base::Resolver for Resolver<'a, 'crateloader> {

self.current_module = invocation.module.get();
self.current_module.unresolved_invocations.borrow_mut().remove(&mark);
self.unresolved_invocations_macro_export.remove(&mark);
self.current_module.unresolved_invocations.borrow_mut().extend(derives);
self.unresolved_invocations_macro_export.extend(derives);
for &derive in derives {
self.invocations.insert(derive, invocation);
}
Expand Down Expand Up @@ -338,19 +337,37 @@ impl<'a, 'crateloader: 'a> base::Resolver for Resolver<'a, 'crateloader> {
match attr_kind {
NonMacroAttrKind::Tool | NonMacroAttrKind::DeriveHelper |
NonMacroAttrKind::Custom if is_attr_invoc => {
let features = self.session.features_untracked();
if attr_kind == NonMacroAttrKind::Tool &&
!self.session.features_untracked().tool_attributes {
!features.tool_attributes {
feature_err(&self.session.parse_sess, "tool_attributes",
invoc.span(), GateIssue::Language,
"tool attributes are unstable").emit();
}
if attr_kind == NonMacroAttrKind::Custom &&
!self.session.features_untracked().custom_attribute {
let msg = format!("The attribute `{}` is currently unknown to the compiler \
and may have meaning added to it in the future", path);
feature_err(&self.session.parse_sess, "custom_attribute", invoc.span(),
GateIssue::Language, &msg).emit();
if attr_kind == NonMacroAttrKind::Custom {
assert!(path.segments.len() == 1);
let name = path.segments[0].ident.name.as_str();
if name.starts_with("rustc_") {
if !features.rustc_attrs {
let msg = "unless otherwise specified, attributes with the prefix \
`rustc_` are reserved for internal compiler diagnostics";
feature_err(&self.session.parse_sess, "rustc_attrs", invoc.span(),
GateIssue::Language, &msg).emit();
}
} else if name.starts_with("derive_") {
if !features.custom_derive {
feature_err(&self.session.parse_sess, "custom_derive", invoc.span(),
GateIssue::Language, EXPLAIN_DERIVE_UNDERSCORE).emit();
}
} else if !features.custom_attribute {
let msg = format!("The attribute `{}` is currently unknown to the \
compiler and may have meaning added to it in the \
future", path);
feature_err(&self.session.parse_sess, "custom_attribute", invoc.span(),
GateIssue::Language, &msg).emit();
}
}

return Ok(Some(Lrc::new(SyntaxExtension::NonMacroAttr {
mark_used: attr_kind == NonMacroAttrKind::Tool,
})));
Expand Down Expand Up @@ -650,7 +667,10 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
}
}
WhereToResolve::BuiltinAttrs => {
if is_builtin_attr_name(ident.name) {
// FIXME: Only built-in attributes are not considered as candidates for
// non-attributes to fight off regressions on stable channel (#53205).
// We need to come up with some more principled approach instead.
if is_attr && is_builtin_attr_name(ident.name) {
let binding = (Def::NonMacroAttr(NonMacroAttrKind::Builtin),
ty::Visibility::Public, ident.span, Mark::root())
.to_name_binding(self.arenas);
Expand Down
20 changes: 17 additions & 3 deletions src/librustc_resolve/resolve_imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,14 @@ impl<'a, 'crateloader> Resolver<'a, 'crateloader> {
.try_borrow_mut()
.map_err(|_| Determined)?; // This happens when there is a cycle of imports

if let Some(binding) = resolution.binding {
if !restricted_shadowing && binding.expansion != Mark::root() {
if let NameBindingKind::Def(_, true) = binding.kind {
self.macro_expanded_macro_export_errors.insert((path_span, binding.span));
}
}
}

if record_used {
if let Some(binding) = resolution.binding {
if let Some(shadowed_glob) = resolution.shadowed_glob {
Expand Down Expand Up @@ -211,9 +219,15 @@ impl<'a, 'crateloader> Resolver<'a, 'crateloader> {
// if it cannot be shadowed by some new item/import expanded from a macro.
// This happens either if there are no unexpanded macros, or expanded names cannot
// shadow globs (that happens in macro namespace or with restricted shadowing).
let unexpanded_macros = !module.unresolved_invocations.borrow().is_empty() ||
(ns == MacroNS && ptr::eq(module, self.graph_root) &&
!self.unresolved_invocations_macro_export.is_empty());
//
// Additionally, any macro in any module can plant names in the root module if it creates
// `macro_export` macros, so the root module effectively has unresolved invocations if any
// module has unresolved invocations.
// However, it causes resolution/expansion to stuck too often (#53144), so, to make
// progress, we have to ignore those potential unresolved invocations from other modules
// and prohibit access to macro-expanded `macro_export` macros instead (unless restricted
// shadowing is enabled, see `macro_expanded_macro_export_errors`).
let unexpanded_macros = !module.unresolved_invocations.borrow().is_empty();
if let Some(binding) = resolution.binding {
if !unexpanded_macros || ns == MacroNS || restricted_shadowing {
return check_usable(self, binding);
Expand Down
46 changes: 25 additions & 21 deletions src/libsyntax/feature_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ macro_rules! declare_features {
self.macros_in_extern || self.proc_macro_path_invoc ||
self.proc_macro_mod || self.proc_macro_expr ||
self.proc_macro_non_items || self.proc_macro_gen ||
self.stmt_expr_attributes
self.stmt_expr_attributes || self.unrestricted_attribute_tokens
}
}
};
Expand Down Expand Up @@ -504,6 +504,9 @@ declare_features! (
// impl<I:Iterator> Iterator for &mut Iterator
// impl Debug for Foo<'_>
(active, impl_header_lifetime_elision, "1.30.0", Some(15872), Some(Edition::Edition2018)),

// Support for arbitrary delimited token streams in non-macro attributes.
(active, unrestricted_attribute_tokens, "1.30.0", Some(44690), None),
);

declare_features! (
Expand Down Expand Up @@ -721,8 +724,7 @@ pub fn is_builtin_attr_name(name: ast::Name) -> bool {
}

pub fn is_builtin_attr(attr: &ast::Attribute) -> bool {
BUILTIN_ATTRIBUTES.iter().any(|&(builtin_name, _, _)| attr.path == builtin_name) ||
attr.name().as_str().starts_with("rustc_")
BUILTIN_ATTRIBUTES.iter().any(|&(builtin_name, _, _)| attr.path == builtin_name)
}

// Attributes that have a special meaning to rustc or rustdoc
Expand Down Expand Up @@ -1521,25 +1523,27 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> {
}
}

// allow attr_literals in #[repr(align(x))] and #[repr(packed(n))]
let mut allow_attr_literal = false;
if attr.path == "repr" {
if let Some(content) = attr.meta_item_list() {
allow_attr_literal = content.iter().any(
|c| c.check_name("align") || c.check_name("packed"));
}
}

if self.context.features.use_extern_macros() && attr::is_known(attr) {
return
}
match attr.parse_meta(self.context.parse_sess) {
Ok(meta) => {
// allow attr_literals in #[repr(align(x))] and #[repr(packed(n))]
let mut allow_attr_literal = false;
if attr.path == "repr" {
if let Some(content) = meta.meta_item_list() {
allow_attr_literal = content.iter().any(
|c| c.check_name("align") || c.check_name("packed"));
}
}

if !allow_attr_literal {
let meta = panictry!(attr.parse_meta(self.context.parse_sess));
if contains_novel_literal(&meta) {
gate_feature_post!(&self, attr_literals, attr.span,
"non-string literals in attributes, or string \
literals in top-level positions, are experimental");
if !allow_attr_literal && contains_novel_literal(&meta) {
gate_feature_post!(&self, attr_literals, attr.span,
"non-string literals in attributes, or string \
literals in top-level positions, are experimental");
}
}
Err(mut err) => {
err.cancel();
gate_feature_post!(&self, unrestricted_attribute_tokens, attr.span,
"arbitrary tokens in non-macro attributes are unstable");
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ extern crate derive_b;
#[C] //~ ERROR: The attribute `C` is currently unknown to the compiler
#[B(D)]
#[B(E = "foo")]
#[B arbitrary tokens] //~ expected one of `(` or `=`, found `arbitrary`
#[B arbitrary tokens] //~ ERROR arbitrary tokens in non-macro attributes are unstable
struct B;

fn main() {}
32 changes: 14 additions & 18 deletions src/test/compile-fail/gated-attr-literals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,37 +11,33 @@
// Check that literals in attributes don't parse without the feature gate.

// gate-test-attr_literals
// gate-test-custom_attribute

#![feature(rustc_attrs)]
#![allow(dead_code)]
#![allow(unused_variables)]
#![feature(custom_attribute)]

#[fake_attr] //~ ERROR attribute `fake_attr` is currently unknown
#[fake_attr(100)] //~ ERROR attribute `fake_attr` is currently unknown
#[fake_attr] // OK
#[fake_attr(100)]
//~^ ERROR non-string literals in attributes
#[fake_attr(1, 2, 3)] //~ ERROR attribute `fake_attr` is currently unknown
#[fake_attr(1, 2, 3)]
//~^ ERROR non-string literals in attributes
#[fake_attr("hello")] //~ ERROR attribute `fake_attr` is currently unknown
#[fake_attr("hello")]
//~^ ERROR string literals in top-level positions, are experimental
#[fake_attr(name = "hello")] //~ ERROR attribute `fake_attr` is currently unknown
#[fake_attr(1, "hi", key = 12, true, false)] //~ ERROR attribute `fake_attr` is currently unknown
#[fake_attr(name = "hello")] // OK
#[fake_attr(1, "hi", key = 12, true, false)]
//~^ ERROR non-string literals in attributes, or string literals in top-level positions
#[fake_attr(key = "hello", val = 10)] //~ ERROR attribute `fake_attr` is currently unknown
#[fake_attr(key = "hello", val = 10)]
//~^ ERROR non-string literals in attributes
#[fake_attr(key("hello"), val(10))] //~ ERROR attribute `fake_attr` is currently unknown
#[fake_attr(key("hello"), val(10))]
//~^ ERROR non-string literals in attributes, or string literals in top-level positions
#[fake_attr(enabled = true, disabled = false)] //~ ERROR attribute `fake_attr` is currently unknown
#[fake_attr(enabled = true, disabled = false)]
//~^ ERROR non-string literals in attributes
#[fake_attr(true)] //~ ERROR attribute `fake_attr` is currently unknown
#[fake_attr(true)]
//~^ ERROR non-string literals in attributes
#[fake_attr(pi = 3.14159)] //~ ERROR attribute `fake_attr` is currently unknown
#[fake_attr(pi = 3.14159)]
//~^ ERROR non-string literals in attributes
#[fake_attr(b"hi")] //~ ERROR attribute `fake_attr` is currently unknown
#[fake_attr(b"hi")]
//~^ ERROR string literals in top-level positions, are experimental
#[fake_doc(r"doc")] //~ ERROR attribute `fake_doc` is currently unknown
#[fake_doc(r"doc")]
//~^ ERROR string literals in top-level positions, are experimental
struct Q { }

#[rustc_error]
fn main() { }
2 changes: 1 addition & 1 deletion src/test/compile-fail/macro-attribute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,5 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#[doc = $not_there] //~ error: unexpected token: `$`
#[doc = $not_there] //~ ERROR arbitrary tokens in non-macro attributes are unstable
fn main() { }
2 changes: 1 addition & 1 deletion src/test/parse-fail/attr-bad-meta.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,5 @@
// except according to those terms.

// asterisk is bogus
#[path*] //~ ERROR expected one of `(` or `=`
#[path*] //~ ERROR arbitrary tokens in non-macro attributes are unstable
mod m {}
2 changes: 1 addition & 1 deletion src/test/run-pass-fulldeps/proc-macro/derive-b.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
// aux-build:derive-b.rs
// ignore-stage1

#![feature(proc_macro_path_invoc)]
#![feature(proc_macro_path_invoc, unrestricted_attribute_tokens)]

extern crate derive_b;

Expand Down
18 changes: 18 additions & 0 deletions src/test/ui/feature-gate-rustc-attrs-1.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// ignore-tidy-linelength

// Test that `#[rustc_*]` attributes are gated by `rustc_attrs` feature gate.

#[rustc_variance] //~ ERROR the `#[rustc_variance]` attribute is just used for rustc unit tests and will never be stable
#[rustc_error] //~ ERROR the `#[rustc_error]` attribute is just used for rustc unit tests and will never be stable

fn main() {}
19 changes: 19 additions & 0 deletions src/test/ui/feature-gate-rustc-attrs-1.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
error[E0658]: the `#[rustc_variance]` attribute is just used for rustc unit tests and will never be stable (see issue #29642)
--> $DIR/feature-gate-rustc-attrs-1.rs:15:1
|
LL | #[rustc_variance] //~ ERROR the `#[rustc_variance]` attribute is just used for rustc unit tests and will never be stable
| ^^^^^^^^^^^^^^^^^
|
= help: add #![feature(rustc_attrs)] to the crate attributes to enable

error[E0658]: the `#[rustc_error]` attribute is just used for rustc unit tests and will never be stable (see issue #29642)
--> $DIR/feature-gate-rustc-attrs-1.rs:16:1
|
LL | #[rustc_error] //~ ERROR the `#[rustc_error]` attribute is just used for rustc unit tests and will never be stable
| ^^^^^^^^^^^^^^
|
= help: add #![feature(rustc_attrs)] to the crate attributes to enable

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0658`.
2 changes: 0 additions & 2 deletions src/test/ui/feature-gate-rustc-attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@

// Test that `#[rustc_*]` attributes are gated by `rustc_attrs` feature gate.

#[rustc_variance] //~ ERROR the `#[rustc_variance]` attribute is just used for rustc unit tests and will never be stable
#[rustc_error] //~ ERROR the `#[rustc_error]` attribute is just used for rustc unit tests and will never be stable
#[rustc_foo]
//~^ ERROR unless otherwise specified, attributes with the prefix `rustc_` are reserved for internal compiler diagnostics

Expand Down
Loading