This is an archive of the discontinued LLVM Phabricator instance.

[Parser] Support alternative operator token keyword args in Objective-C++
ClosedPublic

Authored by erik.pilkington on Aug 9 2018, 12:35 PM.

Diff Detail

Repository
rC Clang

Event Timeline

rsmith added a subscriber: rsmith.Aug 9 2018, 1:00 PM

One comment, but otherwise the code change looks mechanically correct. Not accepting only because I don't know whether this is the intended language rule for Objective-C++ or not (please get someone else to sign off on that).

clang/lib/Parse/ParseExpr.cpp
278 ↗(On Diff #159964)

Just check Tok.getIdentifierInfo(). That will be null for a punctuation token and non-null for a token spelled as an identifier. (I don't think you even need the switch, because we already only get here for operators.)

Remove isBinaryCXXAlternativeOperatorToken, this check can be done using OpToken.getAsIdentifierInfo(). Thanks!

FWIW, I can't imagine this being anything but an oversight, we go out of our way to support these tokens in ParseObjCSelectorPiece, and we already happen to support unary alternative operators, such as [foo not].

Assuming you've done enough source-compatibility testing to say with reasonable confidence that this won't break anything, I think this is fine. It's a core goal of Objective-C/C++ to allow the base language as a complete subset if at all possible.

Assuming you've done enough source-compatibility testing to say with reasonable confidence that this won't break anything, I think this is fine. It's a core goal of Objective-C/C++ to allow the base language as a complete subset if at all possible.

I don't really see how this could break source-compatibility, I don't believe there is a way to start a binary operator's RHS with a colon[1] or a r_square, so this change can only affect programs that were previously ill-formed. I'll run some internal builds and report back though, just to be paranoid.

[1] excluding the GNU conditional expr, which is handled because getIdentifierInfo will always return nullptr for '?'.

The build came back clean!

rjmccall accepted this revision.Aug 20 2018, 2:07 PM

Ping!

If the build came back clean, then I think our combination of previous sign-offs is good enough. :)

This revision is now accepted and ready to land.Aug 20 2018, 2:07 PM
This revision was automatically updated to reflect the committed changes.