Skip to content

Improve TokenTree variants - #18229

Merged
bors merged 7 commits into
rust-lang:masterfrom
brendanzab:ttdelim
Oct 28, 2014
Merged

Improve TokenTree variants#18229
bors merged 7 commits into
rust-lang:masterfrom
brendanzab:ttdelim

Conversation

@brendanzab

@brendanzabbrendanzab commented Oct 22, 2014

Copy link
Copy Markdown
Contributor

I have renamed the TokenTree variants to be longer and clearer, and to use standard capitalisation. They should also fit in better with the existing TTNonterminal variant.

The new definition of TtDelimited adds an associated Span that covers the whole token tree, and moves the opening and closing delimiters out of the vector of delimited token trees. This came up when working on the gl-rs generator plugin.

The bool in TtSequence has been replaced with a KleeneOp enum. This should make the intent of the code clearer (I had trouble understanding it).

A get_span method has also been added to TokenTree type to make it easier to implement better error messages for syntax extensions.

[breaking-change]

Renamed:

  • TTTok -> TtToken
  • TTDelim -> TtDelimited
  • TTSeq -> TtSequence
  • TTNonterminal -> TtNonterminal

New definition of TtDelimited:

TtDelimited(Span,Rc<(Delimiter,Vec<TokenTree>,Delimiter)>)

New definition of TtSequence:

TtSequence(Span,Rc<Vec<TokenTree>>,Option<::parse::token::Token>,KleeneOp)

@brendanzabbrendanzab changed the title Add Span and separate open/close delims to TTDelimRename TokenTree variants for Clarity, and solidify delimeter invariantsOct 22, 2014
@brendanzabbrendanzab changed the title Rename TokenTree variants for Clarity, and solidify delimeter invariantsRename TokenTree variants for clarity and solidify delimeter invariantsOct 22, 2014
@brendanzabbrendanzab changed the title Rename TokenTree variants for clarity and solidify delimeter invariantsRename TokenTree variants for clarity and solidify delimiter invariantsOct 22, 2014
@brendanzabbrendanzab added the A-parser Area: The lexing & parsing of Rust source code to an AST label Oct 22, 2014
@brendanzab
brendanzabforce-pushed the ttdelim branch 3 times, most recently from 448b25d to 7580e84CompareOctober 22, 2014 14:47
@pczarn

pczarn commented Oct 22, 2014

Copy link
Copy Markdown
Contributor

Could you rename TTTok to TtToken etc. as per convention?

Don't you think the new TTDelimited will increase memory usage? The size of each TokenTree will be 1+2+(2+1+1+2)*2+1 = 16 words, in contrast to 10 words thus far on x64.

cc #17830.

@brendanzab

brendanzab commented Oct 22, 2014

Copy link
Copy Markdown
ContributorAuthor

Ah, this is true... Would boxing the delimiters help? That would take us to 12 words if my calculations are correct...

@pczarn

pczarn commented Oct 22, 2014

Copy link
Copy Markdown
Contributor

Very nice. Isn't it 6 words? TtSequence looks larger. Ideally, TtDelimited would contain Rc<(Delimiter, Vec<TokenTree>, Delimiter)> (not considering DST). But this won't change the size in practice.

@brendanzab

brendanzab commented Oct 22, 2014

Copy link
Copy Markdown
ContributorAuthor

Unfortunately with this you lose the nice patten matching... but I guess that is not the biggest issue.

@brendanzab

brendanzab commented Oct 22, 2014

Copy link
Copy Markdown
ContributorAuthor

Ok, updated with your suggestion.

@brendanzabbrendanzab changed the title Rename TokenTree variants for clarity and solidify delimiter invariantsImprove TokenTree variantsOct 23, 2014
@brendanzab

brendanzab commented Oct 23, 2014

Copy link
Copy Markdown
ContributorAuthor

I've also just made a change to TtSequence to make it a bit clearer.

@brendanzab
Add Span and separate open/close delims to TTDelim
971d776
This came up when working [on the gl-rs generator extension](https://github.com/bjz/gl-rs/blob/990383de801bd2e233159d5be07c9b5622827620/src/gl_generator/lib.rs#L135-L146).
The new definition of `TTDelim` adds an associated `Span` that covers the whole token tree and enforces the invariant that a delimited sequence of token trees must have an opening and closing delimiter.
A `get_span` method has also been added to `TokenTree` type to make it easier to implement better error messages for syntax extensions.
@brendanzab
Rename TokenTree variants for clarity
ec3f020
This should be clearer, and fits in better with the `TTNonterminal` variant.
Renames:
- `TTTok` -> `TTToken`
- `TTDelim` -> `TTDelimited`
- `TTSeq` -> `TTSequence`
@brendanzab
Prevent some vector reallocations
6a50b4d
@brendanzab
Use standard capitalisation for TokenTree variants
dfb4163
@brendanzab
Reduce the size of the TokenTree
34dacb8
@brendanzab
Add a KleeneOp enum for clarity
94d6eee
@brendanzab
Update parse::test::string_to_tts_1 test
6598d33
@brendanzab

brendanzab commented Oct 26, 2014

Copy link
Copy Markdown
ContributorAuthor

Rebased

alexcrichton added a commit to alexcrichton/rust that referenced this pull request Oct 27, 2014
@alexcrichton
rollup merge ofrust-lang#18229: bjz/ttdelim
b3c676e
@bors
bors merged commit 6598d33 into rust-lang:masterOct 28, 2014
@brendanzab
brendanzab deleted the ttdelim branch October 28, 2014 01:17
@tomakatomaka mentioned this pull request Oct 28, 2014
Merged
lnicola pushed a commit to lnicola/rust that referenced this pull request Oct 17, 2024
@bors
Auto merge ofrust-lang#18229- mrkajetanp:rustfmt-path, r=Veykril
1ef2712
fix: Join rustfmt overrideCommand with project root
When providing a custom rustfmt command, join it with the project root instead of the workspace root. This fixes rust-analyzer getting the wrong invocation path in projects containing subprojects.
This makes the behaviour consistent with how a custom path provided in rust-analyzer.procMacro.server behaves already.
Resolves issue rust-lang#18222
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

A-parserArea: The lexing & parsing of Rust source code to an AST

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@brendanzab@pczarn@sfackler@bors