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

hygiene: Decouple transparencies from expansion IDs #51952

Merged
merged 3 commits into from
Jul 11, 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
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 @@ -4395,12 +4432,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 @@ -4418,7 +4449,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