On Fri, Sep 23, 2011 at 10:58 AM, <[email protected]> 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
diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index d90db0f..370bf29 100644
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -865,7 +865,8 @@ public:
bool isClassName = false,
bool HasTrailingDot = false,
ParsedType ObjectType = ParsedType(),
- bool WantNontrivialTypeSourceInfo = false);
+ bool WantNontrivialTypeSourceInfo = false,
+ IdentifierInfo **CorrectedII = 0);
TypeSpecifierType isTagName(IdentifierInfo &II, Scope *S);
bool isMicrosoftMissingTypename(const CXXScopeSpec *SS);
bool DiagnoseUnknownTypeName(const IdentifierInfo &II,
diff --git a/lib/Parse/Parser.cpp b/lib/Parse/Parser.cpp
index dd6d527..49928dc 100644
--- a/lib/Parse/Parser.cpp
+++ b/lib/Parse/Parser.cpp
@@ -1278,13 +1278,18 @@ bool Parser::TryAnnotateTypeOrScopeToken(bool EnteringContext) {
return true;
if (Tok.is(tok::identifier)) {
+ IdentifierInfo *CorrectedII = 0;
// Determine whether the identifier is a type name.
if (ParsedType Ty = Actions.getTypeName(*Tok.getIdentifierInfo(),
Tok.getLocation(), getCurScope(),
&SS, false,
NextToken().is(tok::period),
ParsedType(),
- /*NonTrivialTypeSourceInfo*/true)) {
+ /*NonTrivialTypeSourceInfo*/true,
+ &CorrectedII)) {
+ // A FixIt was applied as a result of typo correction
+ if (CorrectedII)
+ Tok.setIdentifierInfo(CorrectedII);
// This is a typename. Replace the current token in-place with an
// annotation type token.
Tok.setKind(tok::annot_typename);
diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index 0e81765..86784f4 100644
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -70,7 +70,8 @@ ParsedType Sema::getTypeName(IdentifierInfo &II, SourceLocation NameLoc,
Scope *S, CXXScopeSpec *SS,
bool isClassName, bool HasTrailingDot,
ParsedType ObjectTypePtr,
- bool WantNontrivialTypeSourceInfo) {
+ bool WantNontrivialTypeSourceInfo,
+ IdentifierInfo **CorrectedII) {
// Determine where we will perform name lookup.
DeclContext *LookupCtx = 0;
if (ObjectTypePtr) {
@@ -145,6 +146,52 @@ ParsedType Sema::getTypeName(IdentifierInfo &II, SourceLocation NameLoc,
switch (Result.getResultKind()) {
case LookupResult::NotFound:
case LookupResult::NotFoundInCurrentInstantiation:
+ if (CorrectedII) {
+ TypoCorrection Correction = CorrectTypo(Result.getLookupNameInfo(),
+ Sema::LookupOrdinaryName,
+ S, SS, 0, false,
+ Sema::CTC_Type);
+ IdentifierInfo *NewII = Correction.getCorrectionAsIdentifierInfo();
+ TemplateTy Template;
+ bool MemberOfUnknownSpecialization;
+ UnqualifiedId TemplateName;
+ TemplateName.setIdentifier(NewII, NameLoc);
+ NestedNameSpecifier *NNS = Correction.getCorrectionSpecifier();
+ CXXScopeSpec NewSS, *NewSSPtr = SS;
+ if (SS && NNS) {
+ NewSS.MakeTrivial(Context, NNS, SourceRange(NameLoc));
+ NewSSPtr = &NewSS;
+ }
+ if (Correction && (NNS || NewII != &II) &&
+ // Ignore a correction to a template type as the to-be-corrected
+ // identifier is not a template (typo correction for template names
+ // is handled elsewhere).
+ !(getLangOptions().CPlusPlus && NewSSPtr &&
+ isTemplateName(S, *NewSSPtr, false, TemplateName, ParsedType(),
+ false, Template, MemberOfUnknownSpecialization))) {
+ ParsedType Ty = getTypeName(*NewII, NameLoc, S, NewSSPtr,
+ isClassName, HasTrailingDot, ObjectTypePtr,
+ WantNontrivialTypeSourceInfo);
+ if (Ty) {
+ std::string CorrectedStr(Correction.getAsString(getLangOptions()));
+ std::string CorrectedQuotedStr(
+ Correction.getQuoted(getLangOptions()));
+ Diag(NameLoc, diag::err_unknown_typename_suggest)
+ << Result.getLookupName() << CorrectedQuotedStr
+ << FixItHint::CreateReplacement(SourceRange(NameLoc),
+ CorrectedStr);
+ if (NamedDecl *FirstDecl = Correction.getCorrectionDecl())
+ Diag(FirstDecl->getLocation(), diag::note_previous_decl)
+ << CorrectedQuotedStr;
+
+ if (SS && NNS)
+ SS->MakeTrivial(Context, NNS, SourceRange(NameLoc));
+ *CorrectedII = NewII;
+ return Ty;
+ }
+ }
+ }
+ // If typo correction failed or was not performed, fall through
case LookupResult::FoundOverloaded:
case LookupResult::FoundUnresolvedValue:
Result.suppressDiagnostics();
diff --git a/test/SemaCXX/missing-namespace-qualifier-typo-corrections.cpp b/test/SemaCXX/missing-namespace-qualifier-typo-corrections.cpp
index 85e3e7e..76ceea1 100644
--- a/test/SemaCXX/missing-namespace-qualifier-typo-corrections.cpp
+++ b/test/SemaCXX/missing-namespace-qualifier-typo-corrections.cpp
@@ -1,8 +1,10 @@
// RUN: %clang_cc1 -fsyntax-only -verify -Wno-c++0x-extensions %s
-namespace fizbin { class Foobar; } // expected-note{{'fizbin::Foobar' declared here}}
+namespace fizbin { class Foobar {}; } // expected-note 2 {{'fizbin::Foobar' declared here}} \
+ // expected-note {{'Foobar' declared here}}
Foobar *my_bar // expected-error{{unknown type name 'Foobar'; did you mean 'fizbin::Foobar'?}}
- = new Foobar; // expected-error{{expected a type}}
+ = new Foobar; // expected-error{{unknown type name 'Foobar'; did you mean 'fizbin::Foobar'?}}
+fizbin::Foobar *my_foo = new fizbin::FooBar; // expected-error{{unknown type name 'FooBar'; did you mean 'Foobar'?}}
namespace barstool { int toFoobar() { return 1; } } // expected-note 3 {{'barstool::toFoobar' declared here}}
int Double(int x) { return x + x; }
@@ -62,11 +64,13 @@ void f() {
// Test case from http://llvm.org/bugs/show_bug.cgi?id=10318
namespace llvm {
- template <typename T> class GraphWriter {}; // expected-note{{'llvm::GraphWriter' declared here}}
+ template <typename T> class GraphWriter {}; // expected-note {{'llvm::GraphWriter' declared here}} \
+ // expected-note {{'GraphWriter' declared here}}
}
struct S {};
void bar() {
GraphWriter<S> x; //expected-error{{no template named 'GraphWriter'; did you mean 'llvm::GraphWriter'?}}
-
+ (void)new llvm::GraphWriter; // expected-error {{expected a type}}
+ (void)new llvm::Graphwriter<S>; // expected-error {{no template named 'Graphwriter' in namespace 'llvm'; did you mean 'GraphWriter'?}}
}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
