Skip to content

Commit

Permalink
Auto merge of #51519 - ExpHP:issue-51331-b, r=petrochenkov
Browse files Browse the repository at this point in the history
Fix for $crate var normalization in proc macro for externally defined macros

Fixes #51331, a bug that has existed in at least *some* form for a year and a half.

The PR includes the addition of a `fold_qpath` method to `syntax::fold::Folder`.  Overriding this method is useful for folds that modify paths in a way that invalidates indices (insertion or removal of a component), as it provides the opportunity to update `qself.position` in `<A as B>::C` paths.  I added it because the bugfix is messy without it.

(unfortunately, grepping around the codebase, I did not see anything else that could use it.)
  • Loading branch information
bors committed Jun 12, 2018
2 parents 3f3ba6c + d13bfd2 commit ef8cb40
Show file tree
Hide file tree
Showing 5 changed files with 147 additions and 32 deletions.
25 changes: 22 additions & 3 deletions src/librustc_resolve/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,23 @@ impl<'a> base::Resolver for Resolver<'a> {
struct EliminateCrateVar<'b, 'a: 'b>(&'b mut Resolver<'a>, Span);

impl<'a, 'b> Folder for EliminateCrateVar<'a, 'b> {
fn fold_path(&mut self, mut path: ast::Path) -> ast::Path {
fn fold_path(&mut self, path: ast::Path) -> ast::Path {
match self.fold_qpath(None, path) {
(None, path) => path,
_ => unreachable!(),
}
}

fn fold_qpath(&mut self, mut qself: Option<ast::QSelf>, mut path: ast::Path)
-> (Option<ast::QSelf>, ast::Path) {
qself = qself.map(|ast::QSelf { ty, path_span, position }| {
ast::QSelf {
ty: self.fold_ty(ty),
path_span: self.new_span(path_span),
position,
}
});

let ident = path.segments[0].ident;
if ident.name == keywords::DollarCrate.name() {
path.segments[0].ident.name = keywords::CrateRoot.name();
Expand All @@ -150,10 +166,13 @@ impl<'a> base::Resolver for Resolver<'a> {
ast::Ident::with_empty_ctxt(name).with_span_pos(span)
),
_ => unreachable!(),
})
});
if let Some(qself) = &mut qself {
qself.position += 1;
}
}
}
path
(qself, path)
}

fn fold_mac(&mut self, mac: ast::Mac) -> ast::Mac {
Expand Down
49 changes: 24 additions & 25 deletions src/libsyntax/fold.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,10 @@ pub trait Folder : Sized {
noop_fold_path(p, self)
}

fn fold_qpath(&mut self, qs: Option<QSelf>, p: Path) -> (Option<QSelf>, Path) {
noop_fold_qpath(qs, p, self)
}

fn fold_path_parameters(&mut self, p: PathParameters) -> PathParameters {
noop_fold_path_parameters(p, self)
}
Expand Down Expand Up @@ -370,14 +374,8 @@ pub fn noop_fold_ty<T: Folder>(t: P<Ty>, fld: &mut T) -> P<Ty> {
TyKind::Tup(tys) => TyKind::Tup(tys.move_map(|ty| fld.fold_ty(ty))),
TyKind::Paren(ty) => TyKind::Paren(fld.fold_ty(ty)),
TyKind::Path(qself, path) => {
let qself = qself.map(|QSelf { ty, path_span, position }| {
QSelf {
ty: fld.fold_ty(ty),
path_span: fld.new_span(path_span),
position,
}
});
TyKind::Path(qself, fld.fold_path(path))
let (qself, path) = fld.fold_qpath(qself, path);
TyKind::Path(qself, path)
}
TyKind::Array(ty, length) => {
TyKind::Array(fld.fold_ty(ty), fld.fold_anon_const(length))
Expand Down Expand Up @@ -442,6 +440,19 @@ pub fn noop_fold_path<T: Folder>(Path { segments, span }: Path, fld: &mut T) ->
}
}

pub fn noop_fold_qpath<T: Folder>(qself: Option<QSelf>,
path: Path,
fld: &mut T) -> (Option<QSelf>, Path) {
let qself = qself.map(|QSelf { ty, path_span, position }| {
QSelf {
ty: fld.fold_ty(ty),
path_span: fld.new_span(path_span),
position,
}
});
(qself, fld.fold_path(path))
}

pub fn noop_fold_path_parameters<T: Folder>(path_parameters: PathParameters, fld: &mut T)
-> PathParameters
{
Expand Down Expand Up @@ -1097,15 +1108,9 @@ pub fn noop_fold_pat<T: Folder>(p: P<Pat>, folder: &mut T) -> P<Pat> {
PatKind::TupleStruct(folder.fold_path(pth),
pats.move_map(|x| folder.fold_pat(x)), ddpos)
}
PatKind::Path(opt_qself, pth) => {
let opt_qself = opt_qself.map(|qself| {
QSelf {
ty: folder.fold_ty(qself.ty),
path_span: folder.new_span(qself.path_span),
position: qself.position,
}
});
PatKind::Path(opt_qself, folder.fold_path(pth))
PatKind::Path(qself, pth) => {
let (qself, pth) = folder.fold_qpath(qself, pth);
PatKind::Path(qself, pth)
}
PatKind::Struct(pth, fields, etc) => {
let pth = folder.fold_path(pth);
Expand Down Expand Up @@ -1267,14 +1272,8 @@ pub fn noop_fold_expr<T: Folder>(Expr {id, node, span, attrs}: Expr, folder: &mu
lim)
}
ExprKind::Path(qself, path) => {
let qself = qself.map(|QSelf { ty, path_span, position }| {
QSelf {
ty: folder.fold_ty(ty),
path_span: folder.new_span(path_span),
position,
}
});
ExprKind::Path(qself, folder.fold_path(path))
let (qself, path) = folder.fold_qpath(qself, path);
ExprKind::Path(qself, path)
}
ExprKind::Break(opt_label, opt_expr) => {
ExprKind::Break(opt_label.map(|label| folder.fold_label(label)),
Expand Down
2 changes: 2 additions & 0 deletions src/test/run-pass-fulldeps/proc-macro/auxiliary/double.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ extern crate proc_macro;

use proc_macro::TokenStream;

// Outputs another copy of the struct. Useful for testing the tokens
// seen by the proc_macro.
#[proc_macro_derive(Double)]
pub fn derive(input: TokenStream) -> TokenStream {
format!("mod foo {{ {} }}", input.to_string()).parse().unwrap()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// Copyright 2016 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.

pub struct ExternFoo;

pub trait ExternTrait {
const CONST: u32;
type Assoc;
}

impl ExternTrait for ExternFoo {
const CONST: u32 = 0;
type Assoc = ExternFoo;
}

#[macro_export]
macro_rules! external { () => {
mod bar {
#[derive(Double)]
struct Bar($crate::ExternFoo);
}

mod qself {
#[derive(Double)]
struct QSelf(<$crate::ExternFoo as $crate::ExternTrait>::Assoc);
}

mod qself_recurse {
#[derive(Double)]
struct QSelfRecurse(<
<$crate::ExternFoo as $crate::ExternTrait>::Assoc
as $crate::ExternTrait>::Assoc
);
}

mod qself_in_const {
#[derive(Double)]
#[repr(u32)]
enum QSelfInConst {
Variant = <$crate::ExternFoo as $crate::ExternTrait>::CONST,
}
}
} }

52 changes: 48 additions & 4 deletions src/test/run-pass-fulldeps/proc-macro/crate-var.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,63 @@
// except according to those terms.

// aux-build:double.rs
// aux-build:external-crate-var.rs
// ignore-stage1

#![allow(unused)]

#[macro_use]
extern crate double;
#[macro_use]
extern crate external_crate_var;

struct Foo;

macro_rules! m { () => {
#[derive(Double)]
struct Bar($crate::Foo);
trait Trait {
const CONST: u32;
type Assoc;
}

impl Trait for Foo {
const CONST: u32 = 0;
type Assoc = Foo;
}

macro_rules! local { () => {
// derive_Double outputs secondary copies of each definition
// to test what the proc_macro sees.
mod bar {
#[derive(Double)]
struct Bar($crate::Foo);
}

mod qself {
#[derive(Double)]
struct QSelf(<::Foo as $crate::Trait>::Assoc);
}

mod qself_recurse {
#[derive(Double)]
struct QSelfRecurse(<<$crate::Foo as $crate::Trait>::Assoc as $crate::Trait>::Assoc);
}

mod qself_in_const {
#[derive(Double)]
#[repr(u32)]
enum QSelfInConst {
Variant = <::Foo as $crate::Trait>::CONST,
}
}
} }
m!();

mod local {
local!();
}

// and now repeat the above tests, using a macro defined in another crate

mod external {
external!{}
}

fn main() {}

0 comments on commit ef8cb40

Please sign in to comment.