<div dir="ltr">Could you post/send (on/off list, etc) an aggregate patch so I can play around with the behavior of this patch?<br><br>Some superficial patch review<br><br>lambda passed to CorrectDelayedTyposInExpr has 'else-after-return' - you could move the short block up to the top:<br><br>if (!...CPlusPlus11)<br> return this->VerifyIntegerConstantExpression(E);<br>/* unindented the longer part of the code */<br><br>I'm slightly surprised at the indentation there too - I think clang-format has a special case for lambdas as the last function argument, where it won't indent them extra (so the "});" will be at the same level as the expression - maybe it only does that if you're not initializing a variable? Dunno.<br><br>Do you need the 'this->' qualification in that lambda? I assume not. (& I usually favor just capture everything by reference [&] - but up to you on that, we're all still certainly feeling out how best to write C++11)<br><br>EmptyLookupTypoDiags::operator() - some {} on single-statement blocks (if (Ctx)/else)<br>Might be worth inverting the if/following block - the if block is longer/nested than the tail block, so switching them around should reduce indentation, etc.<br><br>std::string CorrectedStr(TC.getAsString(SemaRef.getLangOpts()));<br><br>Prefer copy/move init rather than direct init:<br><br>std::string CorrectedStr = TC.getAsString(SemaRef.getLangOpts());<br><br>Hmm - it might be nice to write EmptyLookupTypoDiags just as a single free function (taking all those members as parameters instead) then call that in a lambda - that way you don't have to write all the member state/ctor/etc logic?<br><br>lambda in ParseCaseStatement: I'd probably write "return ExprResult(...)" rather than explicitly specifying the return type. (do you even need to do either? If CorrectDelayedTyposInExpr is a template, then its return result should just implicitly convert to ExprResult in the caller, if it takes a std::function, that should still work too... - I can explain this more in person, perhaps.<br><br>Rather than goto in ParseCXXIdExpression, could that be a loop?<br><br><br>Hmm - why is the correction applied to the LHS/RHS of the binary expression, rather than waiting until the full expression is parsed?<br><br><br><br><br><br></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Oct 29, 2014 at 12:49 PM, Kaelyn Takata <span dir="ltr"><<a href="mailto:rikka@google.com" target="_blank">rikka@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
Among other things, this eliminates at least one redundant error case<br>
and allows recovery in several cases where it wasn't possible before<br>
(e.g. correcting a mistyped static_cast<>).<br>
---<br>
include/clang/Parse/Parser.h | 9 +++<br>
include/clang/Sema/Sema.h | 4 +-<br>
lib/Parse/ParseCXXInlineMethods.cpp | 1 +<br>
lib/Parse/ParseDecl.cpp | 4 +-<br>
lib/Parse/ParseDeclCXX.cpp | 2 +-<br>
lib/Parse/ParseExpr.cpp | 30 +++++++---<br>
lib/Parse/ParseExprCXX.cpp | 13 ++++-<br>
lib/Parse/ParseObjc.cpp | 5 +-<br>
lib/Parse/ParseOpenMP.cpp | 3 +-<br>
lib/Parse/ParseStmt.cpp | 8 ++-<br>
lib/Sema/SemaExpr.cpp | 110 ++++++++++++++++++++++++++++++++---<br>
lib/Sema/SemaStmt.cpp | 19 ++++++<br>
test/FixIt/fixit-unrecoverable.cpp | 4 +-<br>
test/SemaCXX/typo-correction.cpp | 7 ++-<br>
test/SemaTemplate/crash-10438657.cpp | 2 +-<br>
15 files changed, 189 insertions(+), 32 deletions(-)<br>
<br>
<br>_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
<br></blockquote></div><br></div>