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

[breaking-batch] Simplify ast::StructField #32682

Merged
merged 2 commits into from
Apr 6, 2016
Merged

Conversation

petrochenkov
Copy link
Contributor

The AST part of #31937

Unlike HIR, AST still uses Option for field names because parser can't know field indexes reliably due to constructions like

struct S(#[cfg(false)] u8, u8); // The index of the second field changes from 1 during parsing to 0 after expansion.

and I wouldn't like to put the burden of renaming fields on expansion passes and syntax extensions.

plugin-[breaking-change] cc #31645
r? @Manishearth

@Manishearth
Copy link
Member

r=me, on hold for more changes to batch up

@Manishearth
Copy link
Member

We'll batch with the fix for #32655 when it lands.

@bors
Copy link
Contributor

bors commented Apr 6, 2016

☔ The latest upstream changes (presumably #32688) made this pull request unmergeable. Please resolve the merge conflicts.

@petrochenkov
Copy link
Contributor Author

Rebased.

ping @Manishearth, another breaking change (#32688) has just landed.

Manishearth added a commit to Manishearth/rust that referenced this pull request Apr 6, 2016
 The AST part of rust-lang#31937

Unlike HIR, AST still uses `Option` for field names because parser can't know field indexes reliably due to constructions like
```
struct S(#[cfg(false)] u8, u8); // The index of the second field changes from 1 during parsing to 0 after expansion.
```
and I wouldn't like to put the burden of renaming fields on expansion passes and syntax extensions.

plugin-[breaking-change] cc rust-lang#31645
r? @Manishearth
@bors bors merged commit 8fe4290 into rust-lang:master Apr 6, 2016
@petrochenkov petrochenkov deleted the field3 branch September 21, 2016 19:54
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.

3 participants