Skip to content

Commit

Permalink
Rollup merge of rust-lang#51952 - petrochenkov:transmark, r=alexcrichton
Browse files Browse the repository at this point in the history
 hygiene: Decouple transparencies from expansion IDs

And remove fallback to parent modules during resolution of names in scope.

This is a breaking change for users of unstable macros 2.0 (both procedural and declarative), code like this:
```rust
#![feature(decl_macro)]

macro m($S: ident) {
    struct $S;
    mod m {
        type A = $S;
    }
}

fn main() {
    m!(S);
}
```
or equivalent
```rust
#![feature(decl_macro)]

macro m($S: ident) {
    mod m {
        type A = $S;
    }
}

fn main() {
    struct S;
    m!(S);
}
```
stops working due to module boundaries being properly enforced.

For proc macro derives this is still reported as a compatibility warning to give `actix_derive`, `diesel_derives` and `palette_derive` time to fix their issues.

Fixes rust-lang#50504 in accordance with [this comment](rust-lang#50504 (comment)).
  • Loading branch information
Mark-Simulacrum authored Jul 11, 2018
2 parents d2a8a2b + fc74e35 commit 322632a
Show file tree
Hide file tree
Showing 21 changed files with 450 additions and 173 deletions.
19 changes: 7 additions & 12 deletions src/libproc_macro/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1351,7 +1351,7 @@ pub mod __internal {
use syntax::parse::token::{self, Token};
use syntax::tokenstream;
use syntax_pos::{BytePos, Loc, DUMMY_SP};
use syntax_pos::hygiene::{Mark, SyntaxContext, Transparency};
use syntax_pos::hygiene::{SyntaxContext, Transparency};

use super::{TokenStream, LexError, Span};

Expand Down Expand Up @@ -1436,20 +1436,15 @@ pub mod __internal {

// No way to determine def location for a proc macro right now, so use call location.
let location = cx.current_expansion.mark.expn_info().unwrap().call_site;
// Opaque mark was already created by expansion, now create its transparent twin.
// We can't use the call-site span literally here, even if it appears to provide
// correct name resolution, because it has all the `ExpnInfo` wrong, so the edition
// checks, lint macro checks, macro backtraces will all break.
let opaque_mark = cx.current_expansion.mark;
let transparent_mark = Mark::fresh_cloned(opaque_mark);
transparent_mark.set_transparency(Transparency::Transparent);

let to_span = |mark| Span(location.with_ctxt(SyntaxContext::empty().apply_mark(mark)));
let to_span = |transparency| Span(location.with_ctxt(
SyntaxContext::empty().apply_mark_with_transparency(cx.current_expansion.mark,
transparency))
);
p.set(ProcMacroSess {
parse_sess: cx.parse_sess,
data: ProcMacroData {
def_site: to_span(opaque_mark),
call_site: to_span(transparent_mark),
def_site: to_span(Transparency::Opaque),
call_site: to_span(Transparency::Transparent),
},
});
f()
Expand Down
10 changes: 10 additions & 0 deletions src/librustc/hir/def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,16 @@ pub enum Namespace {
MacroNS,
}

impl Namespace {
pub fn descr(self) -> &'static str {
match self {
TypeNS => "type",
ValueNS => "value",
MacroNS => "macro",
}
}
}

/// Just a helper ‒ separate structure for each namespace.
#[derive(Copy, Clone, Default, Debug)]
pub struct PerNS<T> {
Expand Down
36 changes: 7 additions & 29 deletions src/librustc/hir/map/definitions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ impl Decodable for DefPathTable {
/// The definition table containing node definitions.
/// It holds the DefPathTable for local DefIds/DefPaths and it also stores a
/// mapping from NodeIds to local DefIds.
#[derive(Clone)]
pub struct Definitions {
table: DefPathTable,
node_to_def_index: NodeMap<DefIndex>,
Expand All @@ -161,34 +162,12 @@ pub struct Definitions {
/// If `Mark` is an ID of some macro expansion,
/// then `DefId` is the normal module (`mod`) in which the expanded macro was defined.
parent_modules_of_macro_defs: FxHashMap<Mark, DefId>,
/// Item with a given `DefIndex` was defined during opaque macro expansion with ID `Mark`.
/// It can actually be defined during transparent macro expansions inside that opaque expansion,
/// but transparent expansions are ignored here.
opaque_expansions_that_defined: FxHashMap<DefIndex, Mark>,
/// Item with a given `DefIndex` was defined during macro expansion with ID `Mark`.
expansions_that_defined: FxHashMap<DefIndex, Mark>,
next_disambiguator: FxHashMap<(DefIndex, DefPathData), u32>,
def_index_to_span: FxHashMap<DefIndex, Span>,
}

// Unfortunately we have to provide a manual impl of Clone because of the
// fixed-sized array field.
impl Clone for Definitions {
fn clone(&self) -> Self {
Definitions {
table: self.table.clone(),
node_to_def_index: self.node_to_def_index.clone(),
def_index_to_node: [
self.def_index_to_node[0].clone(),
self.def_index_to_node[1].clone(),
],
node_to_hir_id: self.node_to_hir_id.clone(),
parent_modules_of_macro_defs: self.parent_modules_of_macro_defs.clone(),
opaque_expansions_that_defined: self.opaque_expansions_that_defined.clone(),
next_disambiguator: self.next_disambiguator.clone(),
def_index_to_span: self.def_index_to_span.clone(),
}
}
}

/// A unique identifier that we can use to lookup a definition
/// precisely. It combines the index of the definition's parent (if
/// any) with a `DisambiguatedDefPathData`.
Expand Down Expand Up @@ -409,7 +388,7 @@ impl Definitions {
def_index_to_node: [vec![], vec![]],
node_to_hir_id: IndexVec::new(),
parent_modules_of_macro_defs: FxHashMap(),
opaque_expansions_that_defined: FxHashMap(),
expansions_that_defined: FxHashMap(),
next_disambiguator: FxHashMap(),
def_index_to_span: FxHashMap(),
}
Expand Down Expand Up @@ -584,9 +563,8 @@ impl Definitions {
self.node_to_def_index.insert(node_id, index);
}

let expansion = expansion.modern();
if expansion != Mark::root() {
self.opaque_expansions_that_defined.insert(index, expansion);
self.expansions_that_defined.insert(index, expansion);
}

// The span is added if it isn't dummy
Expand All @@ -606,8 +584,8 @@ impl Definitions {
self.node_to_hir_id = mapping;
}

pub fn opaque_expansion_that_defined(&self, index: DefIndex) -> Mark {
self.opaque_expansions_that_defined.get(&index).cloned().unwrap_or(Mark::root())
pub fn expansion_that_defined(&self, index: DefIndex) -> Mark {
self.expansions_that_defined.get(&index).cloned().unwrap_or(Mark::root())
}

pub fn parent_module_of_macro_def(&self, mark: Mark) -> DefId {
Expand Down
12 changes: 12 additions & 0 deletions src/librustc/lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,12 @@ declare_lint! {
"checks the object safety of where clauses"
}

declare_lint! {
pub PROC_MACRO_DERIVE_RESOLUTION_FALLBACK,
Warn,
"detects proc macro derives using inaccessible names from parent modules"
}

/// Does nothing as a lint pass, but registers some `Lint`s
/// which are used by other parts of the compiler.
#[derive(Copy, Clone)]
Expand Down Expand Up @@ -372,6 +378,7 @@ impl LintPass for HardwiredLints {
DUPLICATE_MACRO_EXPORTS,
INTRA_DOC_LINK_RESOLUTION_FAILURE,
WHERE_CLAUSES_OBJECT_SAFETY,
PROC_MACRO_DERIVE_RESOLUTION_FALLBACK,
)
}
}
Expand All @@ -384,6 +391,7 @@ pub enum BuiltinLintDiagnostics {
BareTraitObject(Span, /* is_global */ bool),
AbsPathWithModule(Span),
DuplicatedMacroExports(ast::Ident, Span, Span),
ProcMacroDeriveResolutionFallback(Span),
}

impl BuiltinLintDiagnostics {
Expand Down Expand Up @@ -420,6 +428,10 @@ impl BuiltinLintDiagnostics {
db.span_label(later_span, format!("`{}` already exported", ident));
db.span_note(earlier_span, "previous macro export is now shadowed");
}
BuiltinLintDiagnostics::ProcMacroDeriveResolutionFallback(span) => {
db.span_label(span, "names from parent modules are not \
accessible without an explicit import");
}
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2724,7 +2724,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
pub fn adjust_ident(self, mut ident: Ident, scope: DefId, block: NodeId) -> (Ident, DefId) {
ident = ident.modern();
let target_expansion = match scope.krate {
LOCAL_CRATE => self.hir.definitions().opaque_expansion_that_defined(scope.index),
LOCAL_CRATE => self.hir.definitions().expansion_that_defined(scope.index),
_ => Mark::root(),
};
let scope = match ident.span.adjust(target_expansion) {
Expand Down
5 changes: 5 additions & 0 deletions src/librustc_lint/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,11 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
reference: "issue #50589 <https://github.com/rust-lang/rust/issues/50589>",
edition: None,
},
FutureIncompatibleInfo {
id: LintId::of(PROC_MACRO_DERIVE_RESOLUTION_FALLBACK),
reference: "issue #50504 <https://github.com/rust-lang/rust/issues/50504>",
edition: None,
},
]);

// Register renamed and removed lints
Expand Down
91 changes: 61 additions & 30 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ use syntax::util::lev_distance::find_best_match_for_name;

use syntax::visit::{self, FnKind, Visitor};
use syntax::attr;
use syntax::ast::{Arm, IsAsync, BindingMode, Block, Crate, Expr, ExprKind};
use syntax::ast::{CRATE_NODE_ID, Arm, IsAsync, BindingMode, Block, Crate, Expr, ExprKind};
use syntax::ast::{FnDecl, ForeignItem, ForeignItemKind, GenericParamKind, Generics};
use syntax::ast::{Item, ItemKind, ImplItem, ImplItemKind};
use syntax::ast::{Label, Local, Mutability, Pat, PatKind, Path};
Expand Down Expand Up @@ -1891,7 +1891,12 @@ impl<'a> Resolver<'a> {

ident.span = ident.span.modern();
loop {
module = unwrap_or!(self.hygienic_lexical_parent(module, &mut ident.span), break);
let (opt_module, poisoned) = if record_used {
self.hygienic_lexical_parent_with_compatibility_fallback(module, &mut ident.span)
} else {
(self.hygienic_lexical_parent(module, &mut ident.span), false)
};
module = unwrap_or!(opt_module, break);
let orig_current_module = self.current_module;
self.current_module = module; // Lexical resolutions can never be a privacy error.
let result = self.resolve_ident_in_module_unadjusted(
Expand All @@ -1900,7 +1905,19 @@ impl<'a> Resolver<'a> {
self.current_module = orig_current_module;

match result {
Ok(binding) => return Some(LexicalScopeBinding::Item(binding)),
Ok(binding) => {
if poisoned {
self.session.buffer_lint_with_diagnostic(
lint::builtin::PROC_MACRO_DERIVE_RESOLUTION_FALLBACK,
CRATE_NODE_ID, ident.span,
&format!("cannot find {} `{}` in this scope", ns.descr(), ident),
lint::builtin::BuiltinLintDiagnostics::
ProcMacroDeriveResolutionFallback(ident.span),
);
}
return Some(LexicalScopeBinding::Item(binding))
}
_ if poisoned => break,
Err(Undetermined) => return None,
Err(Determined) => {}
}
Expand Down Expand Up @@ -1935,7 +1952,7 @@ impl<'a> Resolver<'a> {
None
}

fn hygienic_lexical_parent(&mut self, mut module: Module<'a>, span: &mut Span)
fn hygienic_lexical_parent(&mut self, module: Module<'a>, span: &mut Span)
-> Option<Module<'a>> {
if !module.expansion.is_descendant_of(span.ctxt().outer()) {
return Some(self.macro_def_scope(span.remove_mark()));
Expand All @@ -1945,22 +1962,41 @@ impl<'a> Resolver<'a> {
return Some(module.parent.unwrap());
}

let mut module_expansion = module.expansion.modern(); // for backward compatibility
while let Some(parent) = module.parent {
let parent_expansion = parent.expansion.modern();
if module_expansion.is_descendant_of(parent_expansion) &&
parent_expansion != module_expansion {
return if parent_expansion.is_descendant_of(span.ctxt().outer()) {
Some(parent)
} else {
None
};
None
}

fn hygienic_lexical_parent_with_compatibility_fallback(
&mut self, module: Module<'a>, span: &mut Span) -> (Option<Module<'a>>, /* poisoned */ bool
) {
if let module @ Some(..) = self.hygienic_lexical_parent(module, span) {
return (module, false);
}

// We need to support the next case under a deprecation warning
// ```
// struct MyStruct;
// ---- begin: this comes from a proc macro derive
// mod implementation_details {
// // Note that `MyStruct` is not in scope here.
// impl SomeTrait for MyStruct { ... }
// }
// ---- end
// ```
// So we have to fall back to the module's parent during lexical resolution in this case.
if let Some(parent) = module.parent {
// Inner module is inside the macro, parent module is outside of the macro.
if module.expansion != parent.expansion &&
module.expansion.is_descendant_of(parent.expansion) {
// The macro is a proc macro derive
if module.expansion.looks_like_proc_macro_derive() {
if parent.expansion.is_descendant_of(span.ctxt().outer()) {
return (module.parent, true);
}
}
}
module = parent;
module_expansion = parent_expansion;
}

None
(None, false)
}

fn resolve_ident_in_module(&mut self,
Expand Down Expand Up @@ -1996,17 +2032,17 @@ impl<'a> Resolver<'a> {
let mut iter = ctxt.marks().into_iter().rev().peekable();
let mut result = None;
// Find the last modern mark from the end if it exists.
while let Some(&mark) = iter.peek() {
if mark.transparency() == Transparency::Opaque {
while let Some(&(mark, transparency)) = iter.peek() {
if transparency == Transparency::Opaque {
result = Some(mark);
iter.next();
} else {
break;
}
}
// Then find the last legacy mark from the end if it exists.
for mark in iter {
if mark.transparency() == Transparency::SemiTransparent {
for (mark, transparency) in iter {
if transparency == Transparency::SemiTransparent {
result = Some(mark);
} else {
break;
Expand Down Expand Up @@ -4037,8 +4073,9 @@ impl<'a> Resolver<'a> {
let mut search_module = self.current_module;
loop {
self.get_traits_in_module_containing_item(ident, ns, search_module, &mut found_traits);
search_module =
unwrap_or!(self.hygienic_lexical_parent(search_module, &mut ident.span), break);
search_module = unwrap_or!(
self.hygienic_lexical_parent(search_module, &mut ident.span), break
);
}

if let Some(prelude) = self.prelude {
Expand Down Expand Up @@ -4406,12 +4443,6 @@ impl<'a> Resolver<'a> {
(TypeNS, _) => "type",
};

let namespace = match ns {
ValueNS => "value",
MacroNS => "macro",
TypeNS => "type",
};

let msg = format!("the name `{}` is defined multiple times", name);

let mut err = match (old_binding.is_extern_crate(), new_binding.is_extern_crate()) {
Expand All @@ -4429,7 +4460,7 @@ impl<'a> Resolver<'a> {

err.note(&format!("`{}` must be defined only once in the {} namespace of this {}",
name,
namespace,
ns.descr(),
container));

err.span_label(span, format!("`{}` re{} here", name, new_participle));
Expand Down
11 changes: 3 additions & 8 deletions src/librustc_resolve/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use syntax::errors::DiagnosticBuilder;
use syntax::ext::base::{self, Annotatable, Determinacy, MultiModifier, MultiDecorator};
use syntax::ext::base::{MacroKind, SyntaxExtension, Resolver as SyntaxResolver};
use syntax::ext::expand::{self, AstFragment, AstFragmentKind, Invocation, InvocationKind};
use syntax::ext::hygiene::{self, Mark, Transparency};
use syntax::ext::hygiene::{self, Mark};
use syntax::ext::placeholders::placeholder;
use syntax::ext::tt::macro_rules;
use syntax::feature_gate::{self, emit_feature_err, GateIssue};
Expand Down Expand Up @@ -331,13 +331,8 @@ impl<'a> base::Resolver for Resolver<'a> {

self.unused_macros.remove(&def_id);
let ext = self.get_macro(def);
if ext.is_modern() {
let transparency =
if ext.is_transparent() { Transparency::Transparent } else { Transparency::Opaque };
invoc.expansion_data.mark.set_transparency(transparency);
} else if def_id.krate == BUILTIN_MACROS_CRATE {
invoc.expansion_data.mark.set_is_builtin(true);
}
invoc.expansion_data.mark.set_default_transparency(ext.default_transparency());
invoc.expansion_data.mark.set_is_builtin(def_id.krate == BUILTIN_MACROS_CRATE);
Ok(Some(ext))
}

Expand Down
Loading

0 comments on commit 322632a

Please sign in to comment.