rsmith added inline comments.

================
Comment at: lib/Parse/ParseExprCXX.cpp:2906-2912
+    // Basic lookahead to check if we have a lambda expression. If we
+    // encounter two braces with a semicolon, we can be pretty sure
+    // that this is a lambda, not say a compound literal. 
+    if (!SkipUntil(tok::l_brace, SkipUntilFlags::StopAtSemi) ||
+        (NextToken().isNot(tok::r_brace) && !SkipUntil(tok::semi)) ||
+        !SkipUntil(tok::r_brace, SkipUntilFlags::StopAtSemi)) {
+      TPA.Revert();
----------------
This seems error-prone. Given:

```
for (item *p = first, *oldp = p; p; p = p->next, delete [] oldp, oldp = p) {
  /*...*/;
}
```

... we'll decide the //delete-expression// is followed by a lambda. Likewise 
for cases like:

```
delete [] f->getPtr([] {
  return blah;
});
```

Our goal here should be a fast heuristic (don't use lookahead except when 
you're already confident you have a lambda) and zero false positives. How about 
these heuristics instead:

Assume that the `delete []` is actually `delete` followed by a lambda if either:

 * The next token is `{` or `<`, or
 * The next token is `(` and either
    * the following token is `)` or
    * the following tokens are a type specifier followed by an identifier

This should have no false positives, and only has false negatives if a lambda 
has an unnamed parameter or a parameter with a non-trivial parameter type. (For 
the last condition, we could try to tentatively parse an entire parameter and 
see if it has a name, which would handle all cases except an 
expression/declaration ambiguity in the parameter declaration, but that seems 
like overkill to me. This is already performing more lookahead than I'd like, 
but `delete []` expressions are rare enough that using two lookahead tokens for 
an improved error message seems OK.)


https://reviews.llvm.org/D36357



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D36357: A... Nicolas Lesser via Phabricator via cfe-commits
    • [PATCH] D363... Richard Smith - zygoloid via Phabricator via cfe-commits
    • [PATCH] D363... Nicolas Lesser via Phabricator via cfe-commits
    • [PATCH] D363... Nicolas Lesser via Phabricator via cfe-commits
    • [PATCH] D363... Richard Smith - zygoloid via Phabricator via cfe-commits

Reply via email to