Skip to content

In libsyntax, parser.bump_and_get actually does "get and then bump" (function name confusing) #18394

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

Closed
allan-simon opened this issue Oct 28, 2014 · 4 comments
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup.

Comments

@allan-simon
Copy link

The name really get me confused because i was doing something like that

(just a little context here, in this piece of code , I use a custom lexer which pop token::EOF also for a custom token ( %> for a <% xxxx %> templating system ), EOF was chosen because

  1. i needed to have a value in the token::Token enum to be able to leverage the other parser functions
  2. it needed to be something that would not be parsed as part of an expression
  3. something for which i will still have a way to make the difference between the actual token and my custom one (for EOF i still have parser.reader.is_eof() for me )
    match (
        parser.parse_ident(),
        parser.parse_fn_decl(true),
        parser.bump_and_get()
    ) {
        (
            functioname,
            ref function_decl,
            token::EOF,

        ) if parser.token == token::EOF && !parser.reader.is_eof()  => {

and it was not working because parser.token == token::EOF was actually testing the NEXT token , because bump_and_get was actually first returning the last token (the one used in the match) and AFTER moving the parser by one more token, (the one stored in parser.token)

anyway I've already been to fix my problem, so the problem is more about naming

my proposition

  • replace bump_and_get by get_and_bump
  • maybe permit one more value in the Token enum a token::USER_DEFINED(uint), so in order to help people writing syntax extension that superset rust, to still be able to use the normal parser, and just have to implement their own custom lexer::Reader trait.
@bstrie bstrie added the C-cleanup Category: PRs that clean code up or issues documenting cleanup. label Oct 28, 2014
@allan-simon
Copy link
Author

or maybe we want both bump_and_get and get_and_bump ?

@steveklabnik
Copy link
Member

/cc @rust-lang/compiler, is this a cleanup we want to do?

@nrc
Copy link
Member

nrc commented Nov 7, 2015

replace bump_and_get by get_and_bump

I'd take a PR for this, I don't think it's high priority or anything though.

AN extra token should probably have some discussion on internals or as an RFC.

bors added a commit that referenced this issue Mar 5, 2017
Inline function to avoid naming confusion.

Fixes the non-RFC requiring portion of #18394. The suggestion for a new token there probably needs to either be refiled onto rust-lang/rfcs if it's still relevant (may not be given Macros 2.0 work, not sure).
@steveklabnik
Copy link
Member

#40266 fixed the first part of this, I would suggest opening an internals discussion if you're still interested in the other part!

lnicola pushed a commit to lnicola/rust that referenced this issue Oct 29, 2024
internal: Pretty-print Config in status command
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup.
Projects
None yet
Development

No branches or pull requests

4 participants