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

Proper expansion info for for loops and other compiler expanded language items #25318

Merged
merged 2 commits into from
May 13, 2015

Conversation

nrc
Copy link
Member

@nrc nrc commented May 12, 2015

@nrc nrc changed the title For expn Proper expansion info for for loops and other compiler expanded language items May 12, 2015
@sfackler
Copy link
Member

Why do we want expansion info here?

@nrc
Copy link
Member Author

nrc commented May 12, 2015

Because otherwise the spans are lies - we get a span for parts of the expansions of the for loop which are not what the span is for - i.e., a span for a type which is actually a value in the source. Furthermore, without expansion info you can't tell that the code has been expanded - a tool should assume that the expanded source is the same as the pre-expansion source, which it isn't.

@sfackler
Copy link
Member

@bors r+ edb2ee5

@bors
Copy link
Contributor

bors commented May 13, 2015

⌛ Testing commit edb2ee5 with merge eb4cb6d...

bors added a commit that referenced this pull request May 13, 2015
@bors bors merged commit edb2ee5 into rust-lang:master May 13, 2015
bors added a commit that referenced this pull request Jun 18, 2015
r? @nrc, because breakage was caused by #25318
}

ast::ExprClosure(capture_clause, fn_decl, block) => {
push_compiler_expansion(fld, span, "closure expansion");
Copy link
Member

Choose a reason for hiding this comment

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

Came here from git blame, was this change accidental?
Closures are not expanded differently from free functions, so they shouldn't require custom expansion stacks.
All that does is pollute diagnostic output IMO.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, probably wrong

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

Successfully merging this pull request may close these issues.

4 participants