[cfe-commits] [PATCH] Parsing C++0x lambda expressions

[Douglas Gregor]

On Jul 6, 2011, at 2:00 PM, John Freeman wrote:

> Thanks for the review, Doug! Some more discussion below:
> 
>> +/// isCXX0XLambdaExpression - Make sure we are looking at a C++ lambda
>> +/// expression and not a (possibly nested) Objective-C++ message expression.
>> +bool Parser::isCXX0XLambdaExpression() {
>> +  assert(getLang().CPlusPlus0x
>> +&&  Tok.is(tok::l_square)
>> +&&  "why would you call me?");
>> +  // FIXME: Is there a smaller set of conditions we can check?
>> +  return !getLang().ObjC1
>> +         || NextToken().is(tok::r_square)
>> +         || NextToken().is(tok::equal)
>> +         || NextToken().is(tok::amp)
>> +         || (NextToken().is(tok::identifier)
>> +&&  (GetLookAheadToken(2).is(tok::comma)
>> +                 || GetLookAheadToken(2).is(tok::r_square)));
>> 
>> This isn't quite enough, because of the comma operator:
>> 
>> 	[x,y method:17]
>> 
>> To catch this evil case, you're going to need tentative parsing… but only after you've done a quick check for the common/easy cases.
> 
> Ok. Could you take a look at my tentative parsing questions on cfe-dev? Particularly, are diagnostics suppressed until parsing has been committed, or do I need to remove diagnostics from tentative parsing?

You need to avoid generating diagnostics during tentative parsing. They will not be suppressed for you.

> Some errors could be slight enough that they don't mean I'm not parsing a lambda-introducer, like if '&' is followed by 'this'. I'd like to diagnose them, but have just one convenient place to commit to parsing (at the end). Is that possible? What is the convention? Should I try to commit to parsing as early as possible?

If you commit to parsing earlier, the lexer doesn't need to do additional (useless) caching of tokens, so you should commit early if possible. 


>> +  bool first = true;
>> +  while (Tok.isNot(tok::r_square)) {
>> +    if (first) {
>> +      first = false;
>> +
>> +      // Parse capture-default.
>> +      if (Tok.is(tok::amp)&&  NextToken().isNot(tok::identifier)) {
>> +        List.Default = DEFAULT_CAPTURE_BY_REF;
>> +      } else if (Tok.is(tok::equal)) {
>> +        List.Default = DEFAULT_CAPTURE_BY_COPY;
>> +      }
>> +
>> +      if (List.Default != DEFAULT_CAPTURE_NONE) {
>> +        // Consume '&' or '='.
>> +        ConsumeToken();
>> +        continue;
>> +      }
>> +    } else {
>> +      if (ExpectAndConsume(tok::comma,
>> +                           diag::err_expected_comma,
>> +                           "",
>> +                           tok::r_square)) {
>> +        // FIXME: We could also expect end of capture list. Should the
>> +        // diagnostic indicate this?
>> +        return ExprError();
>> +      }
>> +    }
>> 
>> I find this logic to be a bit twisty. Why not do an initial check for&  (followed by non-identifier) or = to set the default capture kind, then go into the loop that handles individual captures?
> 
> I believe it will still require some twisty logic. What will the loop handle? If it succeeds on an empty list, then we will need special logic if the default is followed by a comma but no capture past that. If it succeeds only on a non-empty list, then we will need special logic for an empty list. I thought this was the best solution.

I think the weirdness is because of the way you handle the comma. Each time you go through the loop, you should consume one thing (default capture or a single capture), and then expect either a comma or a ']' at the end.

	- Doug