Add typo correction to DiagnoseInvalidRedeclaration

Reworks DiagnoseInvalidRedeclaration to add the ability to correct typos when diagnosing invalid function redeclarations

[chandlerc, 2011-08-17 20:46:03]

Sorry this one took so long.

http://codereview.appspot.com/4908044/diff/1/lib/Sema/SemaDecl.cpp
File lib/Sema/SemaDecl.cpp (right):

http://codereview.appspot.com/4908044/diff/1/lib/Sema/SemaDecl.cpp#newcode4201
lib/Sema/SemaDecl.cpp:4201: // If the qualified name lookup yielded nothing, try
typo correction
This comment is misleading. The first thing we do is use the qualified name
lookup. I'd put this down below in the CorrecTypo if.

http://codereview.appspot.com/4908044/diff/1/lib/Sema/SemaDecl.cpp#newcode4203
lib/Sema/SemaDecl.cpp:4203: for (LookupResult::iterator Func = Prev.begin(),
FuncEnd = Prev.end();
I'm a bit sad there isn't an I or an It or Iter in the name of the iterator...
but maybe its just madness to have FI, FE, and FD in the same scope..

http://codereview.appspot.com/4908044/diff/1/lib/Sema/SemaDecl.cpp#newcode4208
lib/Sema/SemaDecl.cpp:4208: NearMatches.push_back(std::pair<FunctionDecl*,
unsigned>(FD,
I would put FD on the next line as well to line up all the arguments.

Perhaps even better, move the computation and comment below to a local variable.
Might then be able to use make_pair

http://codereview.appspot.com/4908044/diff/1/lib/Sema/SemaDecl.cpp#newcode4211
lib/Sema/SemaDecl.cpp:4211: MismatchedParams.size() > 0 ?
MismatchedParams.front() + 1 : 0));
Always test .empty() rather than .size() > 0

http://codereview.appspot.com/4908044/diff/1/lib/Sema/SemaDecl.cpp#newcode4215
lib/Sema/SemaDecl.cpp:4215: Prev.getLookupKind(), NULL, NULL, DC))) {
80 columns

http://codereview.appspot.com/4908044/diff/1/test/SemaCXX/function-redecl-typ...
File test/SemaCXX/function-redecl-typo-correction.cpp (right):

http://codereview.appspot.com/4908044/diff/1/test/SemaCXX/function-redecl-typ...
test/SemaCXX/function-redecl-typo-correction.cpp:1: // RUN: %clang_cc1
-fsyntax-only -verify %s
Could we just merge this with existing function redecl tests? Seems a bit small
to live on its own.

[Kaelyn, 2011-08-18 17:14:08]

PTAL

http://codereview.appspot.com/4908044/diff/1/lib/Sema/SemaDecl.cpp
File lib/Sema/SemaDecl.cpp (right):

http://codereview.appspot.com/4908044/diff/1/lib/Sema/SemaDecl.cpp#newcode4201
lib/Sema/SemaDecl.cpp:4201: // If the qualified name lookup yielded nothing, try
typo correction
On 2011/08/17 20:46:04, chandlerc wrote:
> This comment is misleading. The first thing we do is use the qualified name
> lookup. I'd put this down below in the CorrecTypo if.

Whoops! Forgot to move the comment when I reordered the two if blocks. Moved.

http://codereview.appspot.com/4908044/diff/1/lib/Sema/SemaDecl.cpp#newcode4203
lib/Sema/SemaDecl.cpp:4203: for (LookupResult::iterator Func = Prev.begin(),
FuncEnd = Prev.end();
On 2011/08/17 20:46:04, chandlerc wrote:
> I'm a bit sad there isn't an I or an It or Iter in the name of the iterator...
> but maybe its just madness to have FI, FE, and FD in the same scope..

FI, FE, and FD in the same scope would prove confusing. Plus I tend to not
rename variables when I'm just moving code around (of that for loop, the only
real change I made was to the body of the interior if block). ;)

http://codereview.appspot.com/4908044/diff/1/lib/Sema/SemaDecl.cpp#newcode4208
lib/Sema/SemaDecl.cpp:4208: NearMatches.push_back(std::pair<FunctionDecl*,
unsigned>(FD,
On 2011/08/17 20:46:04, chandlerc wrote:
> I would put FD on the next line as well to line up all the arguments.
> 
> Perhaps even better, move the computation and comment below to a local
variable.
> Might then be able to use make_pair

Done.

http://codereview.appspot.com/4908044/diff/1/lib/Sema/SemaDecl.cpp#newcode4211
lib/Sema/SemaDecl.cpp:4211: MismatchedParams.size() > 0 ?
MismatchedParams.front() + 1 : 0));
On 2011/08/17 20:46:04, chandlerc wrote:
> Always test .empty() rather than .size() > 0

Yeah, I'm not sure why I'd done that...

Fixed.

http://codereview.appspot.com/4908044/diff/1/lib/Sema/SemaDecl.cpp#newcode4215
lib/Sema/SemaDecl.cpp:4215: Prev.getLookupKind(), NULL, NULL, DC))) {
On 2011/08/17 20:46:04, chandlerc wrote:
> 80 columns

Done.

[chandlerc, 2011-08-18 17:27:46]

This looks good, but...

On 2011/08/17 20:46:03, chandlerc wrote:
>
http://codereview.appspot.com/4908044/diff/1/test/SemaCXX/function-redecl-typ...
> File test/SemaCXX/function-redecl-typo-correction.cpp (right):
> 
>
http://codereview.appspot.com/4908044/diff/1/test/SemaCXX/function-redecl-typ...
> test/SemaCXX/function-redecl-typo-correction.cpp:1: // RUN: %clang_cc1
> -fsyntax-only -verify %s
> Could we just merge this with existing function redecl tests? Seems a bit
small
> to live on its own.

?

[Kaelyn, 2011-08-18 18:20:21]

On 2011/08/18 17:27:46, chandlerc wrote:
> This looks good, but...
> 
> On 2011/08/17 20:46:03, chandlerc wrote:
> >
>
http://codereview.appspot.com/4908044/diff/1/test/SemaCXX/function-redecl-typ...
> > File test/SemaCXX/function-redecl-typo-correction.cpp (right):
> > 
> >
>
http://codereview.appspot.com/4908044/diff/1/test/SemaCXX/function-redecl-typ...
> > test/SemaCXX/function-redecl-typo-correction.cpp:1: // RUN: %clang_cc1
> > -fsyntax-only -verify %s
> > Could we just merge this with existing function redecl tests? Seems a bit
> small
> > to live on its own.
> 
> ?

The function redecl tests have been merged, and the patchset committed as
r137966.