Navigation Menu

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

Cleanup handling of hygiene for built-in macros #64469

Merged
merged 4 commits into from Sep 15, 2019
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/librustc/hir/lowering.rs
Expand Up @@ -1316,7 +1316,7 @@ impl<'a> LoweringContext<'a> {
ImplTraitContext::Universal(in_band_ty_params),
);
// Set the name to `impl Bound1 + Bound2`.
let ident = Ident::from_str(&pprust::ty_to_string(t)).with_span_pos(span);
let ident = Ident::from_str_and_span(&pprust::ty_to_string(t), span);
in_band_ty_params.push(hir::GenericParam {
hir_id: self.lower_node_id(def_node_id),
name: ParamName::Plain(ident),
Expand Down
17 changes: 6 additions & 11 deletions src/librustc_lint/unused.rs
Expand Up @@ -618,24 +618,19 @@ impl UnusedImportBraces {
}

// Trigger the lint if the nested item is a non-self single item
let node_ident;
match items[0].0.kind {
let node_name = match items[0].0.kind {
ast::UseTreeKind::Simple(rename, ..) => {
let orig_ident = items[0].0.prefix.segments.last().unwrap().ident;
if orig_ident.name == kw::SelfLower {
return;
}
node_ident = rename.unwrap_or(orig_ident);
rename.unwrap_or(orig_ident).name
}
ast::UseTreeKind::Glob => {
node_ident = ast::Ident::from_str("*");
}
ast::UseTreeKind::Nested(_) => {
return;
}
}
ast::UseTreeKind::Glob => Symbol::intern("*"),
ast::UseTreeKind::Nested(_) => return,
};

let msg = format!("braces around {} is unnecessary", node_ident.name);
let msg = format!("braces around {} is unnecessary", node_name);
cx.span_lint(UNUSED_IMPORT_BRACES, item.span, &msg);
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/librustc_metadata/cstore_impl.rs
Expand Up @@ -444,7 +444,8 @@ impl cstore::CStore {
.insert(local_span, (name.to_string(), data.get_span(id.index, sess)));

LoadedMacro::MacroDef(ast::Item {
ident: ast::Ident::from_str(&name.as_str()),
// FIXME: cross-crate hygiene
ident: ast::Ident::with_dummy_span(name.as_symbol()),
id: ast::DUMMY_NODE_ID,
span: local_span,
attrs: attrs.iter().cloned().collect(),
Expand Down
34 changes: 16 additions & 18 deletions src/librustc_resolve/lib.rs
Expand Up @@ -40,7 +40,7 @@ use rustc_metadata::cstore::CStore;
use syntax::ext::hygiene::{ExpnId, Transparency, SyntaxContext};
use syntax::ast::{self, Name, NodeId, Ident, FloatTy, IntTy, UintTy};
use syntax::ext::base::{SyntaxExtension, MacroKind, SpecialDerives};
use syntax::symbol::{Symbol, kw, sym};
use syntax::symbol::{kw, sym};

use syntax::visit::{self, Visitor};
use syntax::attr;
Expand Down Expand Up @@ -241,7 +241,7 @@ impl Segment {

fn names_to_string(segments: &[Segment]) -> String {
names_to_string(&segments.iter()
.map(|seg| seg.ident)
.map(|seg| seg.ident.name)
.collect::<Vec<_>>())
}
}
Expand Down Expand Up @@ -951,7 +951,7 @@ pub struct Resolver<'a> {
struct_constructors: DefIdMap<(Res, ty::Visibility)>,

/// Features enabled for this crate.
active_features: FxHashSet<Symbol>,
active_features: FxHashSet<Name>,

/// Stores enum visibilities to properly build a reduced graph
/// when visiting the correspondent variants.
Expand Down Expand Up @@ -1018,8 +1018,8 @@ impl<'a> hir::lowering::Resolver for Resolver<'a> {
fn resolve_str_path(
&mut self,
span: Span,
crate_root: Option<Symbol>,
components: &[Symbol],
crate_root: Option<Name>,
components: &[Name],
ns: Namespace,
) -> (ast::Path, Res) {
let root = if crate_root.is_some() {
Expand Down Expand Up @@ -2555,7 +2555,7 @@ impl<'a> Resolver<'a> {
fn add_suggestion_for_rename_of_use(
&self,
err: &mut DiagnosticBuilder<'_>,
name: Symbol,
name: Name,
directive: &ImportDirective<'_>,
binding_span: Span,
) {
Expand Down Expand Up @@ -2770,38 +2770,37 @@ impl<'a> Resolver<'a> {
}
}

fn names_to_string(idents: &[Ident]) -> String {
fn names_to_string(names: &[Name]) -> String {
let mut result = String::new();
for (i, ident) in idents.iter()
.filter(|ident| ident.name != kw::PathRoot)
for (i, name) in names.iter()
.filter(|name| **name != kw::PathRoot)
.enumerate() {
if i > 0 {
result.push_str("::");
}
result.push_str(&ident.as_str());
result.push_str(&name.as_str());
}
result
}

fn path_names_to_string(path: &Path) -> String {
names_to_string(&path.segments.iter()
.map(|seg| seg.ident)
.map(|seg| seg.ident.name)
.collect::<Vec<_>>())
}

/// A somewhat inefficient routine to obtain the name of a module.
fn module_to_string(module: Module<'_>) -> Option<String> {
let mut names = Vec::new();

fn collect_mod(names: &mut Vec<Ident>, module: Module<'_>) {
fn collect_mod(names: &mut Vec<Name>, module: Module<'_>) {
if let ModuleKind::Def(.., name) = module.kind {
if let Some(parent) = module.parent {
names.push(Ident::with_dummy_span(name));
names.push(name);
collect_mod(names, parent);
}
} else {
// danger, shouldn't be ident?
names.push(Ident::from_str("<opaque>"));
names.push(Name::intern("<opaque>"));
collect_mod(names, module.parent.unwrap());
}
}
Expand All @@ -2810,9 +2809,8 @@ fn module_to_string(module: Module<'_>) -> Option<String> {
if names.is_empty() {
return None;
}
Some(names_to_string(&names.into_iter()
.rev()
.collect::<Vec<_>>()))
names.reverse();
Some(names_to_string(&names))
}

#[derive(Copy, Clone, Debug)]
Expand Down
10 changes: 6 additions & 4 deletions src/librustc_resolve/resolve_imports.rs
Expand Up @@ -1433,15 +1433,17 @@ fn import_path_to_string(names: &[Ident],
let global = !names.is_empty() && names[0].name == kw::PathRoot;
if let Some(pos) = pos {
let names = if global { &names[1..pos + 1] } else { &names[..pos + 1] };
names_to_string(names)
names_to_string(&names.iter().map(|ident| ident.name).collect::<Vec<_>>())
} else {
let names = if global { &names[1..] } else { names };
if names.is_empty() {
import_directive_subclass_to_string(subclass)
} else {
format!("{}::{}",
names_to_string(names),
import_directive_subclass_to_string(subclass))
format!(
"{}::{}",
names_to_string(&names.iter().map(|ident| ident.name).collect::<Vec<_>>()),
import_directive_subclass_to_string(subclass),
)
}
}
}
Expand Down
11 changes: 2 additions & 9 deletions src/libsyntax/ext/base.rs
Expand Up @@ -977,13 +977,6 @@ impl<'a> ExtCtxt<'a> {
span.with_call_site_ctxt(self.current_expansion.id)
}

/// Span with a context reproducing `macro_rules` hygiene (hygienic locals, unhygienic items).
/// FIXME: This should be eventually replaced either with `with_def_site_ctxt` (preferably),
/// or with `with_call_site_ctxt` (where necessary).
pub fn with_legacy_ctxt(&self, span: Span) -> Span {
span.with_legacy_ctxt(self.current_expansion.id)
}

/// Returns span for the macro which originally caused the current expansion to happen.
///
/// Stops backtracing at include! boundary.
Expand Down Expand Up @@ -1081,8 +1074,8 @@ impl<'a> ExtCtxt<'a> {
pub fn set_trace_macros(&mut self, x: bool) {
self.ecfg.trace_mac = x
}
pub fn ident_of(&self, st: &str) -> ast::Ident {
ast::Ident::from_str(st)
pub fn ident_of(&self, st: &str, sp: Span) -> ast::Ident {
ast::Ident::from_str_and_span(st, sp)
}
pub fn std_path(&self, components: &[Symbol]) -> Vec<ast::Ident> {
let def_site = self.with_def_site_ctxt(DUMMY_SP);
Expand Down
4 changes: 2 additions & 2 deletions src/libsyntax/ext/build.rs
Expand Up @@ -363,7 +363,7 @@ impl<'a> ExtCtxt<'a> {
self.expr(sp, ast::ExprKind::Field(expr, ident.with_span_pos(sp)))
}
pub fn expr_tup_field_access(&self, sp: Span, expr: P<ast::Expr>, idx: usize) -> P<ast::Expr> {
let ident = Ident::from_str(&idx.to_string()).with_span_pos(sp);
let ident = Ident::new(sym::integer(idx), sp);
self.expr(sp, ast::ExprKind::Field(expr, ident))
}
pub fn expr_addr_of(&self, sp: Span, e: P<ast::Expr>) -> P<ast::Expr> {
Expand Down Expand Up @@ -525,7 +525,7 @@ impl<'a> ExtCtxt<'a> {
let err = self.std_path(&[sym::result, sym::Result, sym::Err]);
let err_path = self.path_global(sp, err);

let binding_variable = self.ident_of("__try_var");
let binding_variable = self.ident_of("__try_var", sp);
let binding_pat = self.pat_ident(sp, binding_variable);
let binding_expr = self.expr_ident(sp, binding_variable);

Expand Down
2 changes: 1 addition & 1 deletion src/libsyntax/parse/parser/item.rs
Expand Up @@ -1256,7 +1256,7 @@ impl<'a> Parser<'a> {
for part in idents {
fixed_name.push_str(&format!("_{}", part.name));
}
ident = Ident::from_str(&fixed_name).with_span_pos(fixed_name_sp);
ident = Ident::from_str_and_span(&fixed_name, fixed_name_sp);

self.struct_span_err(fixed_name_sp, error_msg)
.span_label(fixed_name_sp, "dash-separated idents are not valid")
Expand Down
2 changes: 1 addition & 1 deletion src/libsyntax_ext/asm.rs
Expand Up @@ -62,7 +62,7 @@ pub fn expand_asm<'cx>(cx: &'cx mut ExtCtxt<'_>,
MacEager::expr(P(ast::Expr {
id: ast::DUMMY_NODE_ID,
node: ast::ExprKind::InlineAsm(P(inline_asm)),
span: cx.with_legacy_ctxt(sp),
span: cx.with_def_site_ctxt(sp),
attrs: ThinVec::new(),
}))
}
Expand Down
4 changes: 3 additions & 1 deletion src/libsyntax_ext/assert.rs
Expand Up @@ -23,7 +23,9 @@ pub fn expand_assert<'cx>(
}
};

let sp = cx.with_legacy_ctxt(sp);
// `core::panic` and `std::panic` are different macros, so we use call-site
// context to pick up whichever is currently in scope.
let sp = cx.with_call_site_ctxt(sp);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather use the call-site span in a more targeted way though, for the panic identifier specifically, and def-site for everything else.
(Not sure if this makes observable difference in this case, but it's a safer default.)

let panic_call = Mac {
path: Path::from_ident(Ident::new(sym::panic, sp)),
tts: custom_message.unwrap_or_else(|| {
Expand Down
2 changes: 1 addition & 1 deletion src/libsyntax_ext/cfg.rs
Expand Up @@ -16,7 +16,7 @@ pub fn expand_cfg(
sp: Span,
tts: TokenStream,
) -> Box<dyn base::MacResult + 'static> {
let sp = cx.with_legacy_ctxt(sp);
let sp = cx.with_def_site_ctxt(sp);

match parse_cfg(cx, sp, tts) {
Ok(cfg) => {
Expand Down
2 changes: 1 addition & 1 deletion src/libsyntax_ext/concat.rs
Expand Up @@ -59,6 +59,6 @@ pub fn expand_concat(
} else if has_errors {
return DummyResult::any(sp);
}
let sp = cx.with_legacy_ctxt(sp);
let sp = cx.with_def_site_ctxt(sp);
base::MacEager::expr(cx.expr_str(sp, Symbol::intern(&accumulator)))
}
2 changes: 1 addition & 1 deletion src/libsyntax_ext/concat_idents.rs
Expand Up @@ -39,7 +39,7 @@ pub fn expand_concat_idents<'cx>(cx: &'cx mut ExtCtxt<'_>,
}
}

let ident = ast::Ident::new(Symbol::intern(&res_str), cx.with_legacy_ctxt(sp));
let ident = ast::Ident::new(Symbol::intern(&res_str), cx.with_call_site_ctxt(sp));

struct ConcatIdentsResult { ident: ast::Ident }

Expand Down
2 changes: 1 addition & 1 deletion src/libsyntax_ext/deriving/cmp/partial_ord.rs
Expand Up @@ -109,7 +109,7 @@ pub fn some_ordering_collapsed(
GtOp => "gt",
GeOp => "ge",
};
cx.expr_method_call(span, lft, ast::Ident::from_str_and_span(op_str, span), vec![rgt])
cx.expr_method_call(span, lft, cx.ident_of(op_str, span), vec![rgt])
}

pub fn cs_partial_cmp(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> P<Expr> {
Expand Down
8 changes: 4 additions & 4 deletions src/libsyntax_ext/deriving/debug.rs
Expand Up @@ -62,7 +62,7 @@ fn show_substructure(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>
// We want to make sure we have the ctxt set so that we can use unstable methods
let span = cx.with_def_site_ctxt(span);
let name = cx.expr_lit(span, ast::LitKind::Str(ident.name, ast::StrStyle::Cooked));
let builder = Ident::from_str_and_span("debug_trait_builder", span);
let builder = cx.ident_of("debug_trait_builder", span);
let builder_expr = cx.expr_ident(span, builder.clone());

let fmt = substr.nonself_args[0].clone();
Expand All @@ -72,7 +72,7 @@ fn show_substructure(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>
ast::VariantData::Tuple(..) | ast::VariantData::Unit(..) => {
// tuple struct/"normal" variant
let expr =
cx.expr_method_call(span, fmt, Ident::from_str("debug_tuple"), vec![name]);
cx.expr_method_call(span, fmt, cx.ident_of("debug_tuple", span), vec![name]);
stmts.push(cx.stmt_let(span, true, builder, expr));

for field in fields {
Expand All @@ -93,7 +93,7 @@ fn show_substructure(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>
ast::VariantData::Struct(..) => {
// normal struct/struct variant
let expr =
cx.expr_method_call(span, fmt, Ident::from_str("debug_struct"), vec![name]);
cx.expr_method_call(span, fmt, cx.ident_of("debug_struct", span), vec![name]);
stmts.push(cx.stmt_let(DUMMY_SP, true, builder, expr));

for field in fields {
Expand All @@ -113,7 +113,7 @@ fn show_substructure(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>
}
}

let expr = cx.expr_method_call(span, builder_expr, Ident::from_str("finish"), vec![]);
let expr = cx.expr_method_call(span, builder_expr, cx.ident_of("finish", span), vec![]);

stmts.push(cx.stmt_expr(expr));
let block = cx.block(span, stmts);
Expand Down
20 changes: 12 additions & 8 deletions src/libsyntax_ext/deriving/decodable.rs
Expand Up @@ -66,10 +66,14 @@ fn decodable_substructure(cx: &mut ExtCtxt<'_>,
krate: &str)
-> P<Expr> {
let decoder = substr.nonself_args[0].clone();
let recurse = vec![cx.ident_of(krate), cx.ident_of("Decodable"), cx.ident_of("decode")];
let recurse = vec![
cx.ident_of(krate, trait_span),
cx.ident_of("Decodable", trait_span),
cx.ident_of("decode", trait_span),
];
let exprdecode = cx.expr_path(cx.path_global(trait_span, recurse));
// throw an underscore in front to suppress unused variable warnings
let blkarg = cx.ident_of("_d");
let blkarg = cx.ident_of("_d", trait_span);
let blkdecoder = cx.expr_ident(trait_span, blkarg);

return match *substr.fields {
Expand All @@ -78,7 +82,7 @@ fn decodable_substructure(cx: &mut ExtCtxt<'_>,
Unnamed(ref fields, _) => fields.len(),
Named(ref fields) => fields.len(),
};
let read_struct_field = cx.ident_of("read_struct_field");
let read_struct_field = cx.ident_of("read_struct_field", trait_span);

let path = cx.path_ident(trait_span, substr.type_ident);
let result =
Expand All @@ -94,17 +98,17 @@ fn decodable_substructure(cx: &mut ExtCtxt<'_>,
let result = cx.expr_ok(trait_span, result);
cx.expr_method_call(trait_span,
decoder,
cx.ident_of("read_struct"),
cx.ident_of("read_struct", trait_span),
vec![cx.expr_str(trait_span, substr.type_ident.name),
cx.expr_usize(trait_span, nfields),
cx.lambda1(trait_span, result, blkarg)])
}
StaticEnum(_, ref fields) => {
let variant = cx.ident_of("i");
let variant = cx.ident_of("i", trait_span);

let mut arms = Vec::with_capacity(fields.len() + 1);
let mut variants = Vec::with_capacity(fields.len());
let rvariant_arg = cx.ident_of("read_enum_variant_arg");
let rvariant_arg = cx.ident_of("read_enum_variant_arg", trait_span);

for (i, &(ident, v_span, ref parts)) in fields.iter().enumerate() {
variants.push(cx.expr_str(v_span, ident.name));
Expand Down Expand Up @@ -132,11 +136,11 @@ fn decodable_substructure(cx: &mut ExtCtxt<'_>,
let variant_vec = cx.expr_addr_of(trait_span, variant_vec);
let result = cx.expr_method_call(trait_span,
blkdecoder,
cx.ident_of("read_enum_variant"),
cx.ident_of("read_enum_variant", trait_span),
vec![variant_vec, lambda]);
cx.expr_method_call(trait_span,
decoder,
cx.ident_of("read_enum"),
cx.ident_of("read_enum", trait_span),
vec![cx.expr_str(trait_span, substr.type_ident.name),
cx.lambda1(trait_span, result, blkarg)])
}
Expand Down