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

syntax: Document the AST #23428

Closed
wants to merge 0 commits into from
Closed

Conversation

Manishearth
Copy link
Member

I often have to run ast-json or look into the pretty-printer source to figure out what the fields of an AST enum mean. I've tried to document most of what I know (and some semi-obvious stuff).

r? @steveklabnik

f? @eddyb

@Munksgaard
Copy link
Contributor

👍

@Manishearth Manishearth force-pushed the ast-doc branch 4 times, most recently from 169034f to 9d2ffab Compare March 17, 2015 18:52
pub expr: Option<P<Expr>>,
pub id: NodeId,
/// Unsafety of the block
Copy link
Member

Choose a reason for hiding this comment

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

Maybe "distinguishes between unsafe { ... } and { ... }.

@huonw
Copy link
Member

huonw commented Mar 18, 2015

Looks great; just a few little nits. (r=me once fixed)

@ghost
Copy link

ghost commented Mar 18, 2015

In the future it may be worth considering converting some of the variants into struct variants so that the types are more self-documenting.

@Manishearth
Copy link
Member Author

@bors: r=huon

@bors
Copy link
Contributor

bors commented Mar 18, 2015

📌 Commit a5828ff has been approved by huon

Manishearth added a commit to Manishearth/rust that referenced this pull request Mar 18, 2015
 I often have to run `ast-json` or look into the pretty-printer source to figure out what the fields of an AST enum mean. I've tried to document most of what I know (and some semi-obvious stuff).

r? @steveklabnik

f? @eddyb
ExprVec(Vec<P<Expr>>),
/// A function call
/// The first field resolves to the function itself,
Copy link
Member

Choose a reason for hiding this comment

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

you need a line break between these two unless you want them to get rendered all together

@steveklabnik
Copy link
Member

This is missing a ton of periods at the end of buches of summaries. other than that and the few other things I mentioned, looks pretty good.

@bors
Copy link
Contributor

bors commented Mar 18, 2015

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

@Manishearth
Copy link
Member Author

@bors: r=steveklabnik rollup

(Sadly, only part of this could merge in the rollup)

@bors
Copy link
Contributor

bors commented Mar 18, 2015

📌 Commit 6de89f5 has been approved by steveklabnik

Manishearth added a commit to Manishearth/rust that referenced this pull request Mar 19, 2015
 I often have to run `ast-json` or look into the pretty-printer source to figure out what the fields of an AST enum mean. I've tried to document most of what I know (and some semi-obvious stuff).

r? @steveklabnik

f? @eddyb
@Manishearth Manishearth deleted the ast-doc branch March 19, 2015 06:28
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.

5 participants