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

Multiple '+' or '-' prefix operators in a row should be disallowed #982

Open
alexnask opened this issue Mar 13, 2016 · 13 comments
Open

Multiple '+' or '-' prefix operators in a row should be disallowed #982

alexnask opened this issue Mar 13, 2016 · 13 comments

Comments

@alexnask
Copy link
Collaborator

Currently, using two '+'s or '-'s in a row causes rock to produce a postfix increment/decrement operator in the resulting C code.

Semantically, there is no reason in particular why it should be allowed anyways, it just causes confusion since ooc is a C family language but we have no increment/decrement operators.

@marcusnaslund
Copy link
Contributor

I agree. If I remember correctly, ++variable works, but variable++ does not, which always confused me.

@alexnask
Copy link
Collaborator Author

Yeah, it works accidentally, it shouldn't.

I would highly advise you to go through your code and replace all instances it appears in, I will make it an error soon.

@vendethiel
Copy link

++variable is just semantically +(+(variable)), but is generating C code "++variable" because of no special-casing. Whereas I gather "variable++" simply doesn't parse?

@marcusnaslund
Copy link
Contributor

I would highly advise you to go through your code and replace all instances it appears in, I will make it an error soon.

I was just about to. :)

@alexnask
Copy link
Collaborator Author

@vendethiel
Exactly, but I do think disallowing multiple prefix + and - operators is the way to go, as I said it would be really confusing otherwise, even if the generated code did not result in C's increment/decrement operators.

@horasal
Copy link
Contributor

horasal commented Mar 14, 2016

change the rule of unary operator to '+' !'+' works for me.

horasal/nagaqueen@d14f181

I'm trying to make the error message more clear

@alexnask
Copy link
Collaborator Author

Could be done with something like:

UNARY_PLUS  = '+' ('+' {  rewindWhiteSpace; throwError(...); })?

@horasal
Copy link
Contributor

horasal commented Mar 14, 2016

@Shamanas
I'm wondering why this doesn't work:

UNARY_PLUS = '+' (!'+') ~{ throwError( ); }

@alexnask
Copy link
Collaborator Author

I'm not too sure how ~ is implemented in greg, perhaps it can only be used after a match, so the ! here makes it useless?

I don't know, so I'm probably wrong though :)

@fasterthanlime
Copy link
Collaborator

I think @Shamanas is right: (!'+') doesn't mean "Match something that isn't a plus", it means "stop matching if the character ahead is a plus" - so the rule probably doesn't match at all.

I think this was the right idea:

UNARY_PLUS = '+' ('+' { rewindWhiteSpace; throwError(...); })?

..except why can't it live in a separate PRE_INCREMENT / POST_INCREMENT rule? It might feel silly to actively look out for an operator we don't support, but that's the price to pay for having clear error messages, I guess.

@alexnask
Copy link
Collaborator Author

If it lived in a PRE_INCREMENT rule, we should do something like:

UnaryOp = (UNARY_PLUS | PRE_INCREMENT | .... ) expr: Access

Anyways right?
So we still actively look for the operator we don't support, just in the child rule.

To me, UNARY_PLUS = '+' ('+' { rewindWhiteSpace; throwError(...); })? reads like: match unary + operator exactly one time and error out if we find it more than once, feels natural.

Anyway, it's just a little trick for better error reporting, this can be fixed in better ways (but with bad error reporting :P)

@fasterthanlime
Copy link
Collaborator

If it lived in a PRE_INCREMENT rule, we should do something like (...)

not necessarily, you can have several rules that start with '+', as long as unary plus has a !'+' after the first plus is matches, the double-plus rules should kick in next.

(then again I haven't touched nagaqueen in a while)

@alexnask
Copy link
Collaborator Author

Sure, I don't disagree.
The point is, at some point or another, you will need to 'call' the double-plus rule so that error checking is done.
That means that no matter what you do, you will have some weird looking rule.
I personally prefer having the error checking in the UNARY_PLUS rule because I feel it makes it explicit that a chain of +s is not legal.
Another way would be to have a separate DISALLOW_PREFIX_INCREMENT, that is fine too imo.

Anyway, I think it's just a question of style, I don't lean in any direction too much :P

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants