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

[alexnask]

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]

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

[alexnask]

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]

++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]

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]

@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]

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

https://github.com/zhaihj/nagaqueen/commit/d14f181111f41481bc24c18470a99b78cf75db44

I'm trying to make the error message more clear

[alexnask]

Could be done with something like:

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

[horasal]

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

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

[alexnask]

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]

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]

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]

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]

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