On Thu, Sep 6, 2012 at 1:40 AM, Richard Smith <[email protected]> wrote:
> Hi,
>
> This patch needs a few more testcases (both positive and negative). I'd
> particularly like tests which cover the places where C++11 allows a braced
> initializer list, since you're using '{' as an indicator of
> not-an-assignment-expression.

We added some testcases.

> Please make isNotExpressionStart be a static function local to
> ParseExpr.cpp, rather than a member of Parser.

This doesn't make much sense anymore, because we added declaration
checking, and isDeclarationStatement() is in Parser.

About declarations, isDeclarationStatement() returns true for "int(f())".
What do you think should be done in that case? For now, we only check
declarations when not parsing C++.

> Also, instead of replacing the ',' with a ';' and producing a diagnostic,
> please just leave the ',' in the token stream and bail out. This will allow
> the appropriate point higher up in the parser to produce the relevant
> diagnostic (which may not be to replace with a semicolon, depending on the
> context). This affects cases like "if (1, {".

Done!

> Thanks!
> Richard

Thank you for your help!

-- 
Amaury de la Vieuville
Ahmed Bougacha

Attachment: PR7606.patch
Description: Binary data

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to