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

Macro errors are reported at def, not use #1448

Closed
nikomatsakis opened this issue Jan 6, 2012 · 7 comments
Closed

Macro errors are reported at def, not use #1448

nikomatsakis opened this issue Jan 6, 2012 · 7 comments

Comments

@nikomatsakis
Copy link
Contributor

If you write something like #debug["%u", 10], the type error (unsigned vs signed int) gets reported in core.rs, at the definition of #debug, rather than at the point of use. This is very confusing and makes you think something is fundamentally wrong in the Rust libraries (as in, they don't compile) rather than your own code. I think this ought to be fixed before 0.1 so I associated it with that milestone, but others may disagree.

@brson
Copy link
Contributor

brson commented Jan 6, 2012

Difficult to solve generally, since each new AST node a syntax extension introduces has to synthesize a reasonable span. All syntax extensions currently suffer from this issue.

See also #1000, #1127, #1387

@brson
Copy link
Contributor

brson commented Jan 7, 2012

I looked at simplext.rs briefly and couldn't understand what it was doing.

@kevina
Copy link
Contributor

kevina commented Jan 8, 2012

I will try to look into it this week if I can find the time, I want to understand what that code is doing anyway.

@kevina
Copy link
Contributor

kevina commented Jan 21, 2012

Actually using the example given, and the latest Rust compiler (0.10), the problem is closer to #1387.

For an example of this bug try:

fn main() {
    #macro[[#apply[f, [x, ...]], f(x, ...)]];
    fn add(a: int, b: int) -> int { ret a + b; }
    assert (#apply[add, [y, 15]] == 16);
}

Here I know exactly what is going on, the span used is the one for the location of the pattern variable x and not the location of y. Change that too #apply[add, [(1 + y), 15]] and the correct location will be reported. The problem is the AST node for y has two locations associated with it and nether one is necessary the correct choice. What is really needed is a backtrace of sorts so that both locations will be reported, much like is done for C++ templates.

Failing a full backtrace I think the better location is the one for the location of y, I can try to fix that if its important enough, but like I said it will do nothing towards fixing the example given.

@kevina
Copy link
Contributor

kevina commented Jan 21, 2012

Just to clarify, when I try to reproduce the error on the example given using:

fn main() {
    #debug["%u", 10];
}

I get

mac.rs:3:200: 3:201 error: mismatched types: expected `uint` but found `int` (types differ)

which is basically the same as #1387 as there is no column 200 in mac.rs except that I don't get a crash.

@kevina
Copy link
Contributor

kevina commented Jan 22, 2012

This issue should now be fixed in a pull request I will submit shortly.

The actual cause of the bogus location was due to the bug I outlined in an earlier comment. The problem was that a hack to fix Issue #1362 was confusing the manner. I fixed the root cause of #1362 and also fixed this bug.

The actual fix was a fit involved and involved modifying how fold works, but I didn't see any other sensible way.

kevina added a commit to kevina/rust that referenced this issue Jan 23, 2012
Need a better fix, right now it is just causing even more confusion,
for example in issue rust-lang#1448 and rust-lang#1387.

This reverts commit 1e4de33.
kevina added a commit to kevina/rust that referenced this issue Jan 23, 2012
the replacement and not the span of the pattern variable.

Fixes issue rust-lang#1448, and rust-lang#1387.
kevina added a commit to kevina/rust that referenced this issue Jan 23, 2012
brson pushed a commit that referenced this issue Jan 24, 2012
Need a better fix, right now it is just causing even more confusion,
for example in issue #1448 and #1387.

This reverts commit 1e4de33.
brson pushed a commit that referenced this issue Jan 24, 2012
the replacement and not the span of the pattern variable.

Fixes issue #1448, and #1387.
brson pushed a commit that referenced this issue Jan 24, 2012
Need a better fix, right now it is just causing even more confusion,
for example in issue #1448 and #1387.

This reverts commit 1e4de33.
brson pushed a commit that referenced this issue Jan 24, 2012
the replacement and not the span of the pattern variable.

Fixes issue #1448, and #1387.
@brson
Copy link
Contributor

brson commented Jan 24, 2012

Fixed by kevina

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants