[Clang][Sema] better -Wcast-function-type diagnose for pointer parameters and parameters with cv-qualifiers

[ychen]

Summary

1. Skip diagnosing all pointer-like parameters.
2. Replace Context.hasSameUnqualifiedType with Context.hasSimilarType.
3. Add corresponding tests.

After some thoughts, I think we could add the following extension in the
future (probably under an additional flag such as `-Wcast-function-type=2)
according to user feedbacks for the reasons: 1) users could use
void(*)(void) to explicitly override checks (for example:
https://trac.webkit.org/changeset/231565/webkit); 2) these checks could
hide real bugs that users may want to catch; 3) using void(*)(void)
makes following extension worthwhile only when a codebase uses these
patterns on a large scale which seems unlikely.

(GCC does not do the following)

• allow integer promotion (void(int) -> void(char))

• allow arbitrary pointer-to-member conversion

• allow parameter counts mismatch The use pattern is "passing an extra (unwanted) parameter to callbacks" https://bugs.freedesktop.org/show_bug.cgi?id=107349#c0

Thoughts?

[ychen]

ping..

[ychen]

ping

[ychen]

ping..

[ychen]

ping

[ychen]

ping

clang/lib/Sema/SemaCast.cpp

Self.Diag(BeginLoc, diag::warn_reinterpret_different_from_static)
     << OpRange;
   Self.Diag(BeginLoc, diag::note_reinterpret_updowncast_use_static)
     << int(ReinterpretKind)
     << FixItHint::CreateReplacement(BeginLoc, "static_cast");
 }
 
 static bool argTypeIsABIEquivalent(QualType SrcType, QualType DestType,
                                    ASTContext &Context) {
-  if (SrcType->isPointerType() && DestType->isPointerType())
+  if (SrcType->hasPointerRepresentation() &&
+      DestType->hasPointerRepresentation() &&
+      SrcType->isReferenceType() == DestType->isReferenceType())
+    return true;
+
+  // nullptr_t is compatible with member pointers.
+  if ((SrcType->isNullPtrType() && DestType->isMemberPointerType()) ||
+      (DestType->isNullPtrType() && SrcType->isMemberPointerType()))
     return true;
 
   // Allow integral type mismatch if their size are equal.
   if (SrcType->isIntegralType(Context) && DestType->isIntegralType(Context))
     if (Context.getTypeInfoInChars(SrcType).Width ==
         Context.getTypeInfoInChars(DestType).Width)
       return true;
 
-  return Context.hasSameUnqualifiedType(SrcType, DestType);
+  return Context.hasSimilarType(SrcType, DestType);
 }
 
 static bool checkCastFunctionType(Sema &Self, const ExprResult &SrcExpr,
                                   QualType DestType) {
   if (Self.Diags.isIgnored(diag::warn_cast_function_type,
                            SrcExpr.get()->getExprLoc()))
     return true;
 

clang/test/Sema/warn-cast-function-type.c

 // RUN: %clang_cc1 -x c %s -fsyntax-only -Wcast-function-type -triple x86_64-- -verify
 
 int x(long);
 
 typedef int (f1)(long);
 typedef int (f2)(void*);
 typedef int (f3)();
 typedef void (f4)();
 typedef void (f5)(void);
 typedef int (f6)(long, int);
 typedef int (f7)(long,...);
+typedef int (f8)(int);
+typedef int (f9)(unsigned long);
 
 f1 *a;
 f2 *b;
 f3 *c;
 f4 *d;
 f5 *e;
 f6 *f;
 f7 *g;
+f8 *h;
+f9 *i;
 
 void foo(void) {
   a = (f1 *)x;
   b = (f2 *)x; /* expected-warning {{cast from 'int (*)(long)' to 'f2 *' (aka 'int (*)(void *)') converts to incompatible function type}} */
   c = (f3 *)x;
   d = (f4 *)x; /* expected-warning {{cast from 'int (*)(long)' to 'f4 *' (aka 'void (*)()') converts to incompatible function type}} */
   e = (f5 *)x;
   f = (f6 *)x; /* expected-warning {{cast from 'int (*)(long)' to 'f6 *' (aka 'int (*)(long, int)') converts to incompatible function type}} */
   g = (f7 *)x;
+  h = (f8 *)x; /* expected-warning {{cast from 'int (*)(long)' to 'f8 *' (aka 'int (*)(int)') converts to incompatible function type}} */
+  i = (f9 *)x;
 }

clang/test/Sema/warn-cast-function-type.m

+// RUN: %clang_cc1 %s -fsyntax-only -Wcast-function-type -verify
+
+// expected-no-diagnostics
+
+int x(long*);
+
+typedef int (f1)(id);
+
+f1 *a;
+
+void foo(void) {
+  a = (f1 *)x;
+}

clang/test/SemaCXX/warn-cast-function-type.cpp

-// RUN: %clang_cc1 -x c++ %s -fblocks -fsyntax-only -Wcast-function-type -triple x86_64-- -verify
+// RUN: %clang_cc1 -x c++ -std=c++11 %s -fblocks -fsyntax-only -Wcast-function-type -triple x86_64-- -verify
 
-int x(long);
+using nullptr_t = decltype(nullptr);
 
+int x(long);
 typedef int (f1)(long);
 typedef int (f2)(void*);
 typedef int (f3)(...);
 typedef void (f4)(...);
 typedef void (f5)(void);
 typedef int (f6)(long, int);
 typedef int (f7)(long,...);
 typedef int (&f8)(long, int);
 
 f1 *a;
 f2 *b;
 f3 *c;
 f4 *d;
 f5 *e;
 f6 *f;
 f7 *g;
 
-struct S
-{
-  void foo (int*);
-  void bar (int);
-};
+struct S1 { void foo(int *); };
+struct S2 {};
 
-typedef void (S::*mf)(int);
+typedef void (S1::*pmf1)(int);
+typedef float S1::*pm1;
+typedef const float S1::*pm2;
+
+void y(long *);
+typedef void(pf1)(long **);
+typedef void(pf2)(long &);
+typedef void(pf3)(void (^)(long));
+typedef void(pf4)(nullptr_t);
+
+void z(long &);
+typedef void(pf5)(long **);
+typedef void(pf6)(long &);
+typedef void(pf7)(void (^)(long));
+typedef void(pf8)(nullptr_t);
+
+void u(pm1);
+typedef void(pf9)(pm2);
+typedef void(pf10)(nullptr_t);
+
+pf1 *p1;
+pf2 *p2;
+pf3 *p3;
+pf4 *p4;
+pf5 *p5;
+pf6 *p6;
+pf7 *p7;
+pf8 *p8;
+pf9 *p9;
+pf10 *p10;
 
 void foo() {
   a = (f1 *)x;
   b = (f2 *)x; /* expected-warning {{cast from 'int (*)(long)' to 'f2 *' (aka 'int (*)(void *)') converts to incompatible function type}} */
   b = reinterpret_cast<f2 *>(x); /* expected-warning {{cast from 'int (*)(long)' to 'f2 *' (aka 'int (*)(void *)') converts to incompatible function type}} */
   c = (f3 *)x;
   d = (f4 *)x; /* expected-warning {{cast from 'int (*)(long)' to 'f4 *' (aka 'void (*)(...)') converts to incompatible function type}} */
   e = (f5 *)x;
   f = (f6 *)x; /* expected-warning {{cast from 'int (*)(long)' to 'f6 *' (aka 'int (*)(long, int)') converts to incompatible function type}} */
   g = (f7 *)x;
 
-  mf p1 = (mf)&S::foo; /* expected-warning {{cast from 'void (S::*)(int *)' to 'mf' (aka 'void (S::*)(int)') converts to incompatible function type}} */
+  pmf1 pm1 = (pmf1)&S1::foo; /* expected-warning {{cast from 'void (S1::*)(int *)' to 'pmf1' (aka 'void (S1::*)(int)') converts to incompatible function type}} */
 
   f8 f2 = (f8)x; /* expected-warning {{cast from 'int (long)' to 'f8' (aka 'int (&)(long, int)') converts to incompatible function type}} */
   (void)f2;
 
-  int (^y)(long);
-  f = (f6 *)y; /* expected-warning {{cast from 'int (^)(long)' to 'f6 *' (aka 'int (*)(long, int)') converts to incompatible function type}} */
+  int (^bb)(long);
+  f = (f6 *)bb; /* expected-warning {{cast from 'int (^)(long)' to 'f6 *' (aka 'int (*)(long, int)') converts to incompatible function type}} */
+
+  p1 = (pf1 *)y;
+  p2 = (pf2 *)y; /* expected-warning {{cast from 'void (*)(long *)' to 'pf2 *' (aka 'void (*)(long &)') converts to incompatible function type}} */
+  p3 = (pf3 *)y;
+  p4 = (pf4 *)y;
+  p5 = (pf5 *)z; /* expected-warning {{cast from 'void (*)(long &)' to 'pf5 *' (aka 'void (*)(long **)') converts to incompatible function type}} */
+  p6 = (pf6 *)z;
+  p7 = (pf7 *)z; /* expected-warning {{cast from 'void (*)(long &)' to 'pf7 *' (aka 'void (*)(void (^)(long))') converts to incompatible function type}} */
+  p8 = (pf8 *)z; /* expected-warning {{cast from 'void (*)(long &)' to 'pf8 *' (aka 'void (*)(nullptr_t)') converts to incompatible function type}} */
+  p9 = (pf9 *)u;
+  p10 = (pf10 *)u;
 }