rsmith added a comment.
This looks good, with some minor changes. Please add more test coverage,
though, specifically:
- test all four forms of lambda that we recognize after `delete`
- add a test that the FixItHint is correct (maybe in
test/FixIt/fixit-cxx0x.cpp, which checks that applying the fixes results in
working code)
================
Comment at: lib/Parse/ParseExprCXX.cpp:2956
+ GetLookAheadToken(3).isOneOf(tok::r_paren, tok::identifier) &&
+ GetLookAheadToken(4).is(tok::identifier))) {
+ SourceLocation RightBracketLock = NextToken().getLocation();
----------------
This doesn't seem quite right now. `[]()` is not recognized as a lambda unless
it's followed by an identifier. I think we want (and sorry if this is reverting
to what you had before switching to `isOneOf`):
```
if (Next.isOneOf(tok::l_brace, tok::less) ||
(Next.is(tok::l_paren) &&
(GetLookAheadToken(3).is(tok::r_paren) ||
(GetLookAheadToken(3).is(tok::identifier) &&
GetLookAheadToken(4).is(tok::r_paren))))) {
```
That is, we're matching:
* `[]{`
* `[]<`
* `[]()`
* `[](identifier identifier`
================
Comment at: lib/Parse/ParseExprCXX.cpp:2969
+ << FixItHint::CreateInsertion(
+ Lexer::getLocForEndOfToken(Lambda.get()->getLocEnd(), 1,
+ Actions.getSourceManager(),
----------------
The `1` here should be a `0`. Given
```
delete []{return new int;}();
```
this will insert the `)` after the final `;` instead of before it.
================
Comment at: lib/Parse/ParseExprCXX.cpp:2978
+ Lambda.get());
+ } else {
+ ArrayDelete = true;
----------------
No need for an `else` here, since the `if` block terminates by `return`.
Repository:
rC Clang
https://reviews.llvm.org/D36357
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
