[SemaCXX] Param diagnostic matches overload logic

[modocache]

Summary

Given the following test program:

class C {
public:
  int A(int a, int& b);
};

int C::A(const int a, int b) {
  return a * b;
}

Clang would produce an error message that correctly diagnosed the
redeclaration of C::A to not match the original declaration (the
parameters to the two declarations do not match -- the original takes an
int & as its 2nd parameter, but the redeclaration takes an int). However,
it also produced a note diagnostic that inaccurately pointed to the
first parameter, claiming that const int in the redeclaration did not
match the unqualified int in the original. The diagnostic is
misleading because it has nothing to do with why the program does not
compile.

The logic for checking for a function overload, in
Sema::FunctionParamTypesAreEqual, discards cv-qualifiers before
checking whether the types are equal. Do the same when producing the
overload diagnostic.

[cpplearner]

lib/Sema/SemaDecl.cpp

5090

I see this is what Sema::FunctionParamTypesAreEqual does, but maybe both places should use hasSameUnqualifiedType?

[modocache]

Thanks, @cpplearner! You're absolutely right. I got confused because I didn't realize that the method FunctionProtoType::getUnqualifiedType was being used from within Sema::FunctionParamTypesAreEqual, not QualType::getUnqualifiedType. I modified my patch to use ASTContext::hasSameUnqualifiedType instead.

[modocache]

Friendly ping! Is there anything I can do to improve this patch? I think it's a clear improvement of the diagnostic, with a test case to boot!

[rsmith]

Thanks, LGTM.

[modocache]

Thanks for the reviews!

lib/Sema/SemaDecl.cpp

static bool hasSimilarParameters(ASTContext &Context,
   Params.clear();
   if (Declaration->param_size() != Definition->param_size())
     return false;
   for (unsigned Idx = 0; Idx < Declaration->param_size(); ++Idx) {
     QualType DeclParamTy = Declaration->getParamDecl(Idx)->getType();
     QualType DefParamTy = Definition->getParamDecl(Idx)->getType();
 
     // The parameter types are identical
-    if (Context.hasSameType(DefParamTy, DeclParamTy))
+    if (Context.hasSameUnqualifiedType(DefParamTy, DeclParamTy))
       continue;
 
     QualType DeclParamBaseTy = getCoreType(DeclParamTy);
     QualType DefParamBaseTy = getCoreType(DefParamTy);
     const IdentifierInfo *DeclTyName = DeclParamBaseTy.getBaseTypeIdentifier();
     const IdentifierInfo *DefTyName = DefParamBaseTy.getBaseTypeIdentifier();
 
     if (Context.hasSameUnqualifiedType(DeclParamBaseTy, DefParamBaseTy) ||

test/SemaCXX/function-redecl.cpp

namespace Foo {
     void beEvil();
   }
 }
 bool Foo::isGood() { // expected-error {{out-of-line definition of 'isGood' does not match any declaration in namespace 'redecl_typo::Foo'; did you mean 'Bar::Foo::isGood'?}}
   return true;
 }
 void Foo::beEvil() {} // expected-error {{out-of-line definition of 'beEvil' does not match any declaration in namespace 'redecl_typo::Foo'; did you mean 'BeEvil'?}}
 }
+
+struct CVQualFun {
+  void func(int a, int &b); // expected-note {{type of 2nd parameter of member declaration does not match definition ('int &' vs 'int')}}
+};
+
+void CVQualFun::func(const int a, int b) {} // expected-error {{out-of-line definition of 'func' does not match any declaration in 'CVQualFun'}}