[cfe-commits] Add typo correction for the (non-template) type name in C++ "new" statements. (issue 5014044)

[Kaelyn Uhrain]

On Fri, Sep 23, 2011 at 10:58 AM, <chandlerc at gmail.com> wrote:

> I don't really like all the plumbing of bool parameters through so much
> of the parser.
>
> Why don't we *always* want typo correction when we call
> TryAnnotateTypeOrScopeToken? If there are good reasons, why can we not
> detect it from the immediate context?
>
> I looked at a few other calls, and didn't see any reason why we'd want
> to skip typo correction, but I didn't look at all of them. I think this
> really does need someone more deeply familiar with name lookup in the
> parser to look at it...
>

Thanks for prompting me to reexamine the patch with fresh eyes! I was able
to fix the problems in the typo correction code  in Sema::getTypeName and to
remove the bool parameters I'd added to the parser. :)


>
> http://codereview.appspot.com/**5014044/diff/1/include/clang/**
> Parse/Parser.h<http://codereview.appspot.com/5014044/diff/1/include/clang/Parse/Parser.h>
> File include/clang/Parse/Parser.h (right):
>
> http://codereview.appspot.com/**5014044/diff/1/include/clang/**
> Parse/Parser.h#newcode1316<http://codereview.appspot.com/5014044/diff/1/include/clang/Parse/Parser.h#newcode1316>
> include/clang/Parse/Parser.h:**1316: bool
> ParseCXXTypeSpecifierSeq(**DeclSpec &DS, bool TryTypoCorrection=false);
> spaces around =
>
> http://codereview.appspot.com/**5014044/diff/1/lib/Parse/**ParseDecl.cpp<http://codereview.appspot.com/5014044/diff/1/lib/Parse/ParseDecl.cpp>
> File lib/Parse/ParseDecl.cpp (right):
>
> http://codereview.appspot.com/**5014044/diff/1/lib/Parse/**
> ParseDecl.cpp#newcode2241<http://codereview.appspot.com/5014044/diff/1/lib/Parse/ParseDecl.cpp#newcode2241>
> lib/Parse/ParseDecl.cpp:2241: TemplateInfo, SuppressDeclarations,
> TryTypoCorrection);
> 80-columns
>
> http://codereview.appspot.com/**5014044/diff/1/lib/Parse/**
> ParseDecl.cpp#newcode2252<http://codereview.appspot.com/5014044/diff/1/lib/Parse/ParseDecl.cpp#newcode2252>
> lib/Parse/ParseDecl.cpp:2252: TemplateInfo, SuppressDeclarations,
> TryTypoCorrection);
> 80-columns
>

The three above comments are now moot. ^.^


>
> http://codereview.appspot.com/**5014044/diff/1/lib/Parse/**Parser.cpp<http://codereview.appspot.com/5014044/diff/1/lib/Parse/Parser.cpp>
> File lib/Parse/Parser.cpp (right):
>
> http://codereview.appspot.com/**5014044/diff/1/lib/Parse/**
> Parser.cpp#newcode1282<http://codereview.appspot.com/5014044/diff/1/lib/Parse/Parser.cpp#newcode1282>
> lib/Parse/Parser.cpp:1282: IdentifierInfo *CorrectedII = NULL;
> s/NULL/0/
>
> http://codereview.appspot.com/**5014044/diff/1/lib/Parse/**
> Parser.cpp#newcode1291<http://codereview.appspot.com/5014044/diff/1/lib/Parse/Parser.cpp#newcode1291>
> lib/Parse/Parser.cpp:1291: : NULL)) {
> s/NULL/0/
>
> http://codereview.appspot.com/**5014044/diff/1/lib/Sema/**SemaDecl.cpp<http://codereview.appspot.com/5014044/diff/1/lib/Sema/SemaDecl.cpp>
> File lib/Sema/SemaDecl.cpp (right):
>
> http://codereview.appspot.com/**5014044/diff/1/lib/Sema/**
> SemaDecl.cpp#newcode150<http://codereview.appspot.com/5014044/diff/1/lib/Sema/SemaDecl.cpp#newcode150>
> lib/Sema/SemaDecl.cpp:150: TypoCorrection Corr =
> CorrectTypo(Result.**getLookupNameInfo(),
> Can we use a name better than 'Corr'?
>

Missed this before uploading to rietveld. Changed to "Correction" and fixed
the one line that went over 80 characters.


> http://codereview.appspot.com/**5014044/diff/1/lib/Sema/**
> SemaDecl.cpp#newcode165<http://codereview.appspot.com/5014044/diff/1/lib/Sema/SemaDecl.cpp#newcode165>
> lib/Sema/SemaDecl.cpp:165: std::string
> CorrectedStr(Corr.getAsString(**getLangOptions()));
> Doesn't CorrecReplacement accept a StringRef? If so, can't we pass it a
> StringRef without creating an additional string?
>
> http://codereview.appspot.com/**5014044/diff/1/lib/Sema/**
> SemaDecl.cpp#newcode166<http://codereview.appspot.com/5014044/diff/1/lib/Sema/SemaDecl.cpp#newcode166>
> lib/Sema/SemaDecl.cpp:166: std::string
> CorrectedQuotedStr(Corr.**getQuoted(getLangOptions()));
> Please stream the identifier info into the diagnostic. Formatting
> decisions such as this should be made entirely based on the type of the
> object being printed in the diagnostic and the diagnostic text in the
> table.
>

The reason for calling getAsString() and getQuoted() is because the typo
correction may include a namespace qualifier, and those functions build a
string containing the minimally qualified string needed to reference the new
identifier in the appropriate namespace. See
test/SemaCXX/msising-namespace-qualifier-typo-corrections.cpp for lots of
examples where the displayed string has to be build up from a new
NestedNamespaceQualifier and DeclarationName/IdentifierInfo but for which
e.g. calling the associated NamedDecl's getQualifiedNameAsString() method
returns the wrong thing.

Cheers,
Kaelyn
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110923/6684d6da/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: type-name-typo-correction.diff
Type: text/x-diff
Size: 7271 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110923/6684d6da/attachment.diff>