[cfe-commits] Restart handling of a function declarator if the function name was typo-corrected (issue 4929043)

[chandlerc at gmail.com]

Thanks for working on this. I wonder if we need to generalize this whole
pattern somewhere, as its a lot of work. But it sure seems to pay off.
=D


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

http://codereview.appspot.com/4929043/diff/5001/lib/Sema/SemaDecl.cpp#newcode2909
lib/Sema/SemaDecl.cpp:2909: /// hasSimilarParameters - Determine whether
the C++ functions Declaration
s/functions/function's/

http://codereview.appspot.com/4929043/diff/5001/lib/Sema/SemaDecl.cpp#newcode4205
lib/Sema/SemaDecl.cpp:4205: // The rest of the parameters are for
calling ActOnFunctionDeclarator
Ugh. Could we stash these in a parameter object, or somehow factor it so
that they're more clearly managed? That would also help with the
confusing naming of 'S' and 'Sc' for example.

Also, this function definitely needs a doxyment.

http://codereview.appspot.com/4929043/diff/5001/lib/Sema/SemaDecl.cpp#newcode4244
lib/Sema/SemaDecl.cpp:4244: // Back up the value of the by-reference
booleans
I have a slight preference to call the function with our own local
variables, and then to set the output parameters if it succeeds... Makes
for nice names like "TrialRedeclaration" and "TrialAddToScope"

http://codereview.appspot.com/4929043/diff/5001/lib/Sema/SemaDecl.cpp#newcode4284
lib/Sema/SemaDecl.cpp:4284: if (TypoWasCorrected)
Why not just hoist this into the code that sets TypoWasCorrected to
true?

Hmm, at this point in this function's complexity, I think we need to
split it into several functions. Specifically I'm thinking the loop
below to emit notes should be a separate function that we can call from
the various places where it makes sense, and then we can hoist these
diagnostics up into the branches above, and switch them to use early
exit, reducing indentation.

Does that make sense? Separate patches would be fine with me here, and
those should be fine for post-commit review.

http://codereview.appspot.com/4929043/diff/5001/lib/Sema/SemaDecl.cpp#newcode4290
lib/Sema/SemaDecl.cpp:4290: S.Diag(NewFD->getLocation(), DiagMsg) <<
Name << NewDC << NewFD->getLocation();
80 columns

http://codereview.appspot.com/4929043/