Skip to content

Commit

Permalink
Auto merge of #31089 - fhahn:macro-ice, r=pnkfelix
Browse files Browse the repository at this point in the history
This is a  work in progress PR that potentially should fix #29084, #28308, #25385, #28288, #31011. I think this may also adresse parts of  #2887.

The problem in this issues seems to be that when transcribing macro arguments, we just clone the argument Nonterminal, which still has to original spans. This leads to the unprintable spans. One solution would be to update the spans of the inserted argument to match the argument in the macro definition. So for [this testcase](https://github.com/rust-lang/rust/compare/master...fhahn:macro-ice?expand=1#diff-f7def7420c51621640707b6337726876R2) the error message would be displayed in the macro definition:

    src/test/compile-fail/issue-31011.rs:4:12: 4:22 error: attempted access of field `trace` on type `&T`, but no field with that name was found
    src/test/compile-fail/issue-31011.rs:4         if $ctx.trace {

Currently I've added a very simple `update_span` function, which updates the span of the outer-most expression of a `NtExpr`, but this `update_span` function should be updated to handle all Nonterminals. But I'm pretty new to the macro system and would like to check if this approach makes sense, before doing that.
  • Loading branch information
bors committed Jan 27, 2016
2 parents b8b18aa + ecb7b01 commit 8256c47
Show file tree
Hide file tree
Showing 12 changed files with 354 additions and 30 deletions.
90 changes: 60 additions & 30 deletions src/libsyntax/parse/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,6 @@ macro_rules! maybe_whole {
)
}


fn maybe_append(mut lhs: Vec<Attribute>, rhs: Option<Vec<Attribute>>)
-> Vec<Attribute> {
if let Some(ref attrs) = rhs {
Expand All @@ -255,6 +254,7 @@ pub struct Parser<'a> {
pub cfg: CrateConfig,
/// the previous token or None (only stashed sometimes).
pub last_token: Option<Box<token::Token>>,
last_token_interpolated: bool,
pub buffer: [TokenAndSpan; 4],
pub buffer_start: isize,
pub buffer_end: isize,
Expand Down Expand Up @@ -362,6 +362,7 @@ impl<'a> Parser<'a> {
span: span,
last_span: span,
last_token: None,
last_token_interpolated: false,
buffer: [
placeholder.clone(),
placeholder.clone(),
Expand Down Expand Up @@ -542,6 +543,19 @@ impl<'a> Parser<'a> {
self.commit_stmt(&[edible], &[])
}

/// returns the span of expr, if it was not interpolated or the span of the interpolated token
fn interpolated_or_expr_span(&self,
expr: PResult<'a, P<Expr>>)
-> PResult<'a, (Span, P<Expr>)> {
expr.map(|e| {
if self.last_token_interpolated {
(self.last_span, e)
} else {
(e.span, e)
}
})
}

pub fn parse_ident(&mut self) -> PResult<'a, ast::Ident> {
self.check_strict_keywords();
self.check_reserved_keywords();
Expand Down Expand Up @@ -933,6 +947,7 @@ impl<'a> Parser<'a> {
} else {
None
};
self.last_token_interpolated = self.token.is_interpolated();
let next = if self.buffer_start == self.buffer_end {
self.reader.real_token()
} else {
Expand Down Expand Up @@ -2328,18 +2343,20 @@ impl<'a> Parser<'a> {
-> PResult<'a, P<Expr>> {
let attrs = try!(self.parse_or_use_outer_attributes(already_parsed_attrs));

let b = try!(self.parse_bottom_expr());
self.parse_dot_or_call_expr_with(b, attrs)
let b = self.parse_bottom_expr();
let (span, b) = try!(self.interpolated_or_expr_span(b));
self.parse_dot_or_call_expr_with(b, span.lo, attrs)
}

pub fn parse_dot_or_call_expr_with(&mut self,
e0: P<Expr>,
lo: BytePos,
attrs: ThinAttributes)
-> PResult<'a, P<Expr>> {
// Stitch the list of outer attributes onto the return value.
// A little bit ugly, but the best way given the current code
// structure
self.parse_dot_or_call_expr_with_(e0)
self.parse_dot_or_call_expr_with_(e0, lo)
.map(|expr|
expr.map(|mut expr| {
expr.attrs.update(|a| a.prepend(attrs));
Expand All @@ -2366,7 +2383,8 @@ impl<'a> Parser<'a> {
fn parse_dot_suffix(&mut self,
ident: Ident,
ident_span: Span,
self_value: P<Expr>)
self_value: P<Expr>,
lo: BytePos)
-> PResult<'a, P<Expr>> {
let (_, tys, bindings) = if self.eat(&token::ModSep) {
try!(self.expect_lt());
Expand All @@ -2380,8 +2398,6 @@ impl<'a> Parser<'a> {
self.span_err(last_span, "type bindings are only permitted on trait paths");
}

let lo = self_value.span.lo;

Ok(match self.token {
// expr.f() method call.
token::OpenDelim(token::Paren) => {
Expand Down Expand Up @@ -2414,9 +2430,8 @@ impl<'a> Parser<'a> {
})
}

fn parse_dot_or_call_expr_with_(&mut self, e0: P<Expr>) -> PResult<'a, P<Expr>> {
fn parse_dot_or_call_expr_with_(&mut self, e0: P<Expr>, lo: BytePos) -> PResult<'a, P<Expr>> {
let mut e = e0;
let lo = e.span.lo;
let mut hi;
loop {
// expr.f
Expand All @@ -2427,7 +2442,7 @@ impl<'a> Parser<'a> {
hi = self.span.hi;
self.bump();

e = try!(self.parse_dot_suffix(i, mk_sp(dot_pos, hi), e));
e = try!(self.parse_dot_suffix(i, mk_sp(dot_pos, hi), e, lo));
}
token::Literal(token::Integer(n), suf) => {
let sp = self.span;
Expand Down Expand Up @@ -2480,7 +2495,7 @@ impl<'a> Parser<'a> {
let dot_pos = self.last_span.hi;
e = try!(self.parse_dot_suffix(special_idents::invalid,
mk_sp(dot_pos, dot_pos),
e));
e, lo));
}
}
continue;
Expand Down Expand Up @@ -2715,27 +2730,31 @@ impl<'a> Parser<'a> {
let ex = match self.token {
token::Not => {
self.bump();
let e = try!(self.parse_prefix_expr(None));
hi = e.span.hi;
let e = self.parse_prefix_expr(None);
let (span, e) = try!(self.interpolated_or_expr_span(e));
hi = span.hi;
self.mk_unary(UnNot, e)
}
token::BinOp(token::Minus) => {
self.bump();
let e = try!(self.parse_prefix_expr(None));
hi = e.span.hi;
let e = self.parse_prefix_expr(None);
let (span, e) = try!(self.interpolated_or_expr_span(e));
hi = span.hi;
self.mk_unary(UnNeg, e)
}
token::BinOp(token::Star) => {
self.bump();
let e = try!(self.parse_prefix_expr(None));
hi = e.span.hi;
let e = self.parse_prefix_expr(None);
let (span, e) = try!(self.interpolated_or_expr_span(e));
hi = span.hi;
self.mk_unary(UnDeref, e)
}
token::BinOp(token::And) | token::AndAnd => {
try!(self.expect_and());
let m = try!(self.parse_mutability());
let e = try!(self.parse_prefix_expr(None));
hi = e.span.hi;
let e = self.parse_prefix_expr(None);
let (span, e) = try!(self.interpolated_or_expr_span(e));
hi = span.hi;
ExprAddrOf(m, e)
}
token::Ident(..) if self.token.is_keyword(keywords::In) => {
Expand All @@ -2753,9 +2772,10 @@ impl<'a> Parser<'a> {
}
token::Ident(..) if self.token.is_keyword(keywords::Box) => {
self.bump();
let subexpression = try!(self.parse_prefix_expr(None));
hi = subexpression.span.hi;
ExprBox(subexpression)
let e = self.parse_prefix_expr(None);
let (span, e) = try!(self.interpolated_or_expr_span(e));
hi = span.hi;
ExprBox(e)
}
_ => return self.parse_dot_or_call_expr(Some(attrs))
};
Expand Down Expand Up @@ -2790,12 +2810,21 @@ impl<'a> Parser<'a> {
try!(self.parse_prefix_expr(attrs))
}
};


if self.expr_is_complete(&*lhs) {
// Semi-statement forms are odd. See https://github.com/rust-lang/rust/issues/29071
return Ok(lhs);
}
self.expected_tokens.push(TokenType::Operator);
while let Some(op) = AssocOp::from_token(&self.token) {

let lhs_span = if self.last_token_interpolated {
self.last_span
} else {
lhs.span
};

let cur_op_span = self.span;
let restrictions = if op.is_assign_like() {
self.restrictions & Restrictions::RESTRICTION_NO_STRUCT_LITERAL
Expand All @@ -2812,12 +2841,12 @@ impl<'a> Parser<'a> {
// Special cases:
if op == AssocOp::As {
let rhs = try!(self.parse_ty());
lhs = self.mk_expr(lhs.span.lo, rhs.span.hi,
lhs = self.mk_expr(lhs_span.lo, rhs.span.hi,
ExprCast(lhs, rhs), None);
continue
} else if op == AssocOp::Colon {
let rhs = try!(self.parse_ty());
lhs = self.mk_expr(lhs.span.lo, rhs.span.hi,
lhs = self.mk_expr(lhs_span.lo, rhs.span.hi,
ExprType(lhs, rhs), None);
continue
} else if op == AssocOp::DotDot {
Expand All @@ -2839,7 +2868,7 @@ impl<'a> Parser<'a> {
} else {
None
};
let (lhs_span, rhs_span) = (lhs.span, if let Some(ref x) = rhs {
let (lhs_span, rhs_span) = (lhs_span, if let Some(ref x) = rhs {
x.span
} else {
cur_op_span
Expand Down Expand Up @@ -2879,14 +2908,14 @@ impl<'a> Parser<'a> {
AssocOp::Equal | AssocOp::Less | AssocOp::LessEqual | AssocOp::NotEqual |
AssocOp::Greater | AssocOp::GreaterEqual => {
let ast_op = op.to_ast_binop().unwrap();
let (lhs_span, rhs_span) = (lhs.span, rhs.span);
let (lhs_span, rhs_span) = (lhs_span, rhs.span);
let binary = self.mk_binary(codemap::respan(cur_op_span, ast_op), lhs, rhs);
self.mk_expr(lhs_span.lo, rhs_span.hi, binary, None)
}
AssocOp::Assign =>
self.mk_expr(lhs.span.lo, rhs.span.hi, ExprAssign(lhs, rhs), None),
self.mk_expr(lhs_span.lo, rhs.span.hi, ExprAssign(lhs, rhs), None),
AssocOp::Inplace =>
self.mk_expr(lhs.span.lo, rhs.span.hi, ExprInPlace(lhs, rhs), None),
self.mk_expr(lhs_span.lo, rhs.span.hi, ExprInPlace(lhs, rhs), None),
AssocOp::AssignOp(k) => {
let aop = match k {
token::Plus => BiAdd,
Expand All @@ -2900,7 +2929,7 @@ impl<'a> Parser<'a> {
token::Shl => BiShl,
token::Shr => BiShr
};
let (lhs_span, rhs_span) = (lhs.span, rhs.span);
let (lhs_span, rhs_span) = (lhs_span, rhs.span);
let aopexpr = self.mk_assign_op(codemap::respan(cur_op_span, aop), lhs, rhs);
self.mk_expr(lhs_span.lo, rhs_span.hi, aopexpr, None)
}
Expand Down Expand Up @@ -3834,7 +3863,8 @@ impl<'a> Parser<'a> {
let e = self.mk_mac_expr(span.lo, span.hi,
mac.and_then(|m| m.node),
None);
let e = try!(self.parse_dot_or_call_expr_with(e, attrs));
let lo = e.span.lo;
let e = try!(self.parse_dot_or_call_expr_with(e, lo, attrs));
let e = try!(self.parse_assoc_expr_with(0, LhsExpr::AlreadyParsed(e)));
try!(self.handle_expression_like_statement(
e,
Expand Down
8 changes: 8 additions & 0 deletions src/libsyntax/parse/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,14 @@ impl Token {
}
}

/// Returns `true` if the token is interpolated.
pub fn is_interpolated(&self) -> bool {
match *self {
Interpolated(..) => true,
_ => false,
}
}

/// Returns `true` if the token is an interpolated path.
pub fn is_path(&self) -> bool {
match *self {
Expand Down
24 changes: 24 additions & 0 deletions src/test/compile-fail/issue-25385.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// 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.


macro_rules! foo {
($e:expr) => { $e.foo() }
//~^ ERROR no method named `foo` found for type `i32` in the current scope
}

fn main() {
let a = 1i32;
foo!(a);
//~^ NOTE in this expansion of foo!

foo!(1i32.foo());
//~^ ERROR no method named `foo` found for type `i32` in the current scope
}
40 changes: 40 additions & 0 deletions src/test/compile-fail/issue-25386.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// 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.

mod stuff {
pub struct Item {
c_object: Box<CObj>,
}
pub struct CObj {
name: Option<String>,
}
impl Item {
pub fn new() -> Item {
Item {
c_object: Box::new(CObj { name: None }),
}
}
}
}

macro_rules! check_ptr_exist {
($var:expr, $member:ident) => (
(*$var.c_object).$member.is_some()
//~^ ERROR field `name` of struct `stuff::CObj` is private
//~^^ ERROR field `c_object` of struct `stuff::Item` is private
);
}

fn main() {
let item = stuff::Item::new();
println!("{}", check_ptr_exist!(item, name));
//~^ NOTE in this expansion of check_ptr_exist!
//~^^ NOTE in this expansion of check_ptr_exist!
}
33 changes: 33 additions & 0 deletions src/test/compile-fail/issue-25793.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// 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.

macro_rules! width(
($this:expr) => {
$this.width.unwrap()
//~^ ERROR cannot use `self.width` because it was mutably borrowed
}
);

struct HasInfo {
width: Option<usize>
}

impl HasInfo {
fn get_size(&mut self, n: usize) -> usize {
n
}

fn get_other(&mut self) -> usize {
self.get_size(width!(self))
//~^ NOTE in this expansion of width!
}
}

fn main() {}
21 changes: 21 additions & 0 deletions src/test/compile-fail/issue-26093.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// 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.

macro_rules! not_an_lvalue {
($thing:expr) => {
$thing = 42;
//~^ ERROR invalid left-hand side expression
}
}

fn main() {
not_an_lvalue!(99);
//~^ NOTE in this expansion of not_an_lvalue!
}
Loading

0 comments on commit 8256c47

Please sign in to comment.