Hi,<br><br><div class="gmail_quote">On Mon, Sep 10, 2012 at 11:15 AM, Ahmed Bougacha <span dir="ltr"><<a href="mailto:ahmed.bougacha@gmail.com" target="_blank">ahmed.bougacha@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im">On Thu, Sep 6, 2012 at 1:40 AM, Richard Smith <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>> wrote:<br>
> Hi,<br>
><br>
> This patch needs a few more testcases (both positive and negative). I'd<br>
> particularly like tests which cover the places where C++11 allows a braced<br>
> initializer list, since you're using '{' as an indicator of<br>
> not-an-assignment-expression.<br>
<br>
</div>We added some testcases.</blockquote><div><br></div><div>Thanks.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">
> Please make isNotExpressionStart be a static function local to<br>
> ParseExpr.cpp, rather than a member of Parser.<br>
<br>
</div>This doesn't make much sense anymore, because we added declaration<br>
checking, and isDeclarationStatement() is in Parser.<br></blockquote><div><br></div><div>OK.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
About declarations, isDeclarationStatement() returns true for "int(f())".<br>
What do you think should be done in that case? For now, we only check<br>
declarations when not parsing C++.</blockquote><div><br></div><div>In C++, you can use isCXXDeclarationSpecifier(). It will only return TPResult::True() if you're looking at something which could be the start of a declaration and can't be the start of an expression.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">
> Also, instead of replacing the ',' with a ';' and producing a diagnostic,<br>
> please just leave the ',' in the token stream and bail out. This will allow<br>
> the appropriate point higher up in the parser to produce the relevant<br>
> diagnostic (which may not be to replace with a semicolon, depending on the<br>
> context). This affects cases like "if (1, {".<br>
<br>
</div>Done!<br></blockquote><div><br></div><div>It looks like you're now consuming the ',' before bailing out. I'm not sure whether that's detectable, but you should leave it in the token stream anyway so other parts of the compiler can recover better (for instance, we could offer a ',' -> ';' typo correction in this situation by extending IsCommonTypo). Given that you're now looking a couple of tokens past the comma, it'll be best to go back to your former approach of putting it back into the token stream if you find that it's a typo.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
> Thanks!<br>
> Richard<br>
<br>
Thank you for your help!</blockquote><div><br></div><div>Thanks for working on this!</div><div>Richard</div></div>