[PATCH v2 6/6] Wire up delayed typo correction to DiagnoseEmptyLookup and set up Sema::ActOnIdExpression to use the new functionality.

[David Blaikie]

Could you post/send (on/off list, etc) an aggregate patch so I can play
around with the behavior of this patch?

Some superficial patch review

lambda passed to CorrectDelayedTyposInExpr has 'else-after-return' - you
could move the short block up to the top:

if (!...CPlusPlus11)
  return this->VerifyIntegerConstantExpression(E);
/* unindented the longer part of the code */

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.

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)

EmptyLookupTypoDiags::operator() - some {} on single-statement blocks (if
(Ctx)/else)
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.

std::string CorrectedStr(TC.getAsString(SemaRef.getLangOpts()));

Prefer copy/move init rather than direct init:

std::string CorrectedStr = TC.getAsString(SemaRef.getLangOpts());

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?

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.

Rather than goto in ParseCXXIdExpression, could that be a loop?


Hmm - why is the correction applied to the LHS/RHS of the binary
expression, rather than waiting until the full expression is parsed?






On Wed, Oct 29, 2014 at 12:49 PM, Kaelyn Takata <rikka at google.com> wrote:

>
> Among other things, this eliminates at least one redundant error case
> and allows recovery in several cases where it wasn't possible before
> (e.g. correcting a mistyped static_cast<>).
> ---
>  include/clang/Parse/Parser.h         |   9 +++
>  include/clang/Sema/Sema.h            |   4 +-
>  lib/Parse/ParseCXXInlineMethods.cpp  |   1 +
>  lib/Parse/ParseDecl.cpp              |   4 +-
>  lib/Parse/ParseDeclCXX.cpp           |   2 +-
>  lib/Parse/ParseExpr.cpp              |  30 +++++++---
>  lib/Parse/ParseExprCXX.cpp           |  13 ++++-
>  lib/Parse/ParseObjc.cpp              |   5 +-
>  lib/Parse/ParseOpenMP.cpp            |   3 +-
>  lib/Parse/ParseStmt.cpp              |   8 ++-
>  lib/Sema/SemaExpr.cpp                | 110
> ++++++++++++++++++++++++++++++++---
>  lib/Sema/SemaStmt.cpp                |  19 ++++++
>  test/FixIt/fixit-unrecoverable.cpp   |   4 +-
>  test/SemaCXX/typo-correction.cpp     |   7 ++-
>  test/SemaTemplate/crash-10438657.cpp |   2 +-
>  15 files changed, 189 insertions(+), 32 deletions(-)
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141104/084dbbac/attachment.html>