From 567da71b9bac96d6e87854f330139710fa2653a7 Mon Sep 17 00:00:00 2001
From: George Burgess IV <george.burgess.iv@gmail.com>
Date: Fri, 2 Sep 2016 22:59:57 +0000
Subject: [PATCH] [Sema] Relax overloading restrictions in C.

This patch allows us to perform incompatible pointer conversions when
resolving overloads in C. So, the following code will no longer fail to
compile (though it will still emit warnings, assuming the user hasn't
opted out of them):

```
void foo(char *) __attribute__((overloadable));
void foo(int) __attribute__((overloadable));

void callFoo() {
  unsigned char bar[128];
  foo(bar); // selects the char* overload.
}
```

These conversions are ranked below all others, so:

  A. Any other viable conversion will win out
  B. If we had another incompatible pointer conversion in the example
     above (e.g. `void foo(int *)`), we would complain about
     an ambiguity.

Differential Revision: https://reviews.llvm.org/D24113


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@280553 91177308-0d34-0410-b5e6-96231b3b80d8
---
 include/clang/Basic/AttrDocs.td               |  5 ++
 include/clang/Sema/Overload.h                 |  6 ++-
 lib/Sema/SemaExprCXX.cpp                      |  1 +
 lib/Sema/SemaOverload.cpp                     | 53 +++++++++++++------
 test/CodeGen/builtins-systemz-zvector-error.c | 33 ++++--------
 test/CodeGen/overloadable.c                   | 15 ++++++
 test/Sema/overloadable.c                      | 31 ++++++++++-
 test/Sema/pass-object-size.c                  |  2 +-
 test/SemaOpenCL/event_t_overload.cl           |  6 +--
 9 files changed, 109 insertions(+), 43 deletions(-)

diff --git a/include/clang/Basic/AttrDocs.td b/include/clang/Basic/AttrDocs.td
index b0375e8779e..975fa950c56 100644
--- a/include/clang/Basic/AttrDocs.td
+++ b/include/clang/Basic/AttrDocs.td
@@ -470,6 +470,11 @@ semantics:
 * A conversion from type ``T`` to a value of type ``U`` is permitted if ``T``
   and ``U`` are compatible types.  This conversion is given "conversion" rank.
 
+* A conversion from a pointer of type ``T*`` to a pointer of type ``U*``, where
+  ``T`` and ``U`` are incompatible, is allowed, but is ranked below all other
+  types of conversions. Please note: ``U`` lacking qualifiers that are present
+  on ``T`` is sufficient for ``T`` and ``U`` to be incompatible.
+
 The declaration of ``overloadable`` functions is restricted to function
 declarations and definitions.  Most importantly, if any function with a given
 name is given the ``overloadable`` attribute, then all function declarations
diff --git a/include/clang/Sema/Overload.h b/include/clang/Sema/Overload.h
index ce01ff99af6..3bcd327df94 100644
--- a/include/clang/Sema/Overload.h
+++ b/include/clang/Sema/Overload.h
@@ -84,6 +84,8 @@ namespace clang {
     ICK_Writeback_Conversion,  ///< Objective-C ARC writeback conversion
     ICK_Zero_Event_Conversion, ///< Zero constant to event (OpenCL1.2 6.12.10)
     ICK_C_Only_Conversion,     ///< Conversions allowed in C, but not C++
+    ICK_Incompatible_Pointer_Conversion, ///< C-only conversion between pointers
+                                         ///  with incompatible types
     ICK_Num_Conversion_Kinds,  ///< The number of conversion kinds
   };
 
@@ -97,8 +99,10 @@ namespace clang {
     ICR_Conversion,              ///< Conversion
     ICR_Complex_Real_Conversion, ///< Complex <-> Real conversion
     ICR_Writeback_Conversion,    ///< ObjC ARC writeback conversion
-    ICR_C_Conversion             ///< Conversion only allowed in the C standard.
+    ICR_C_Conversion,            ///< Conversion only allowed in the C standard.
                                  ///  (e.g. void* to char*)
+    ICR_C_Conversion_Extension   ///< Conversion not allowed by the C standard,
+                                 ///  but that we accept as an extension anyway.
   };
 
   ImplicitConversionRank GetConversionRank(ImplicitConversionKind Kind);
diff --git a/lib/Sema/SemaExprCXX.cpp b/lib/Sema/SemaExprCXX.cpp
index f49d2663953..a0bf324a714 100644
--- a/lib/Sema/SemaExprCXX.cpp
+++ b/lib/Sema/SemaExprCXX.cpp
@@ -3705,6 +3705,7 @@ Sema::PerformImplicitConversion(Expr *From, QualType ToType,
   case ICK_Qualification:
   case ICK_Num_Conversion_Kinds:
   case ICK_C_Only_Conversion:
+  case ICK_Incompatible_Pointer_Conversion:
     llvm_unreachable("Improper second standard conversion");
   }
 
diff --git a/lib/Sema/SemaOverload.cpp b/lib/Sema/SemaOverload.cpp
index 7b03a62be12..38144a94a14 100644
--- a/lib/Sema/SemaOverload.cpp
+++ b/lib/Sema/SemaOverload.cpp
@@ -136,7 +136,8 @@ ImplicitConversionRank clang::GetConversionRank(ImplicitConversionKind Kind) {
     ICR_Exact_Match, // NOTE(gbiv): This may not be completely right --
                      // it was omitted by the patch that added
                      // ICK_Zero_Event_Conversion
-    ICR_C_Conversion
+    ICR_C_Conversion,
+    ICR_C_Conversion_Extension
   };
   return Rank[(int)Kind];
 }
@@ -170,7 +171,8 @@ static const char* GetImplicitConversionName(ImplicitConversionKind Kind) {
     "Transparent Union Conversion",
     "Writeback conversion",
     "OpenCL Zero Event Conversion",
-    "C specific type conversion"
+    "C specific type conversion",
+    "Incompatible pointer conversion"
   };
   return Name[Kind];
 }
@@ -1783,22 +1785,37 @@ static bool IsStandardConversion(Sema &S, Expr* From, QualType ToType,
     return false;
 
   ExprResult ER = ExprResult{From};
-  auto Conv = S.CheckSingleAssignmentConstraints(ToType, ER,
-                                                 /*Diagnose=*/false,
-                                                 /*DiagnoseCFAudited=*/false,
-                                                 /*ConvertRHS=*/false);
-  if (Conv != Sema::Compatible)
+  Sema::AssignConvertType Conv =
+      S.CheckSingleAssignmentConstraints(ToType, ER,
+                                         /*Diagnose=*/false,
+                                         /*DiagnoseCFAudited=*/false,
+                                         /*ConvertRHS=*/false);
+  ImplicitConversionKind ImplicitConv;
+  switch (Conv) {
+  case Sema::Compatible:
+    ImplicitConv = ICK_C_Only_Conversion;
+    break;
+  // For our purposes, discarding qualifiers is just as bad as using an
+  // incompatible pointer. Note that an IncompatiblePointer conversion can drop
+  // qualifiers, as well.
+  case Sema::CompatiblePointerDiscardsQualifiers:
+  case Sema::IncompatiblePointer:
+  case Sema::IncompatiblePointerSign:
+    ImplicitConv = ICK_Incompatible_Pointer_Conversion;
+    break;
+  default:
     return false;
+  }
 
   SCS.setAllToTypes(ToType);
   // We need to set all three because we want this conversion to rank terribly,
   // and we don't know what conversions it may overlap with.
-  SCS.First = ICK_C_Only_Conversion;
-  SCS.Second = ICK_C_Only_Conversion;
-  SCS.Third = ICK_C_Only_Conversion;
+  SCS.First = ImplicitConv;
+  SCS.Second = ImplicitConv;
+  SCS.Third = ImplicitConv;
   return true;
 }
-  
+
 static bool
 IsTransparentUnionStandardConversion(Sema &S, Expr* From, 
                                      QualType &ToType,
@@ -5105,6 +5122,7 @@ static bool CheckConvertedConstantConversions(Sema &S,
   case ICK_Writeback_Conversion:
   case ICK_Zero_Event_Conversion:
   case ICK_C_Only_Conversion:
+  case ICK_Incompatible_Pointer_Conversion:
     return false;
 
   case ICK_Lvalue_To_Rvalue:
@@ -5906,10 +5924,15 @@ Sema::SelectBestMethod(Selector Sel, MultiExprArg Args, bool IsInstance,
                                 /*AllowObjCWritebackConversion=*/
                                 getLangOpts().ObjCAutoRefCount,
                                 /*AllowExplicit*/false);
-        if (ConversionState.isBad()) {
-          Match = false;
-          break;
-        }
+      // This function looks for a reasonably-exact match, so we consider
+      // incompatible pointer conversions to be a failure here.
+      if (ConversionState.isBad() ||
+          (ConversionState.isStandard() &&
+           ConversionState.Standard.Second ==
+               ICK_Incompatible_Pointer_Conversion)) {
+        Match = false;
+        break;
+      }
     }
     // Promote additional arguments to variadic methods.
     if (Match && Method->isVariadic()) {
diff --git a/test/CodeGen/builtins-systemz-zvector-error.c b/test/CodeGen/builtins-systemz-zvector-error.c
index 8d5380dac16..b28e8d9bc1f 100644
--- a/test/CodeGen/builtins-systemz-zvector-error.c
+++ b/test/CodeGen/builtins-systemz-zvector-error.c
@@ -233,38 +233,27 @@ void test_core(void) {
                                              // expected-note@vecintrin.h:* 1 {{must be a constant integer from 0 to 1}}
 
   vsc = vec_load_bndry(cptrsc, idx);   // expected-error {{no matching function}}
-                                       // expected-note@vecintrin.h:* 8 {{candidate function not viable}}
-                                       // expected-note@vecintrin.h:* 1 {{must be a constant power of 2 from 64 to 4096}}
+                                       // expected-note@vecintrin.h:* 9 {{must be a constant power of 2 from 64 to 4096}}
   vsc = vec_load_bndry(cptrsc, 200);   // expected-error {{no matching function}}
-                                       // expected-note@vecintrin.h:* 8 {{candidate function not viable}}
-                                       // expected-note@vecintrin.h:* 1 {{must be a constant power of 2 from 64 to 4096}}
+                                       // expected-note@vecintrin.h:* 9 {{must be a constant power of 2 from 64 to 4096}}
   vsc = vec_load_bndry(cptrsc, 32);    // expected-error {{no matching function}}
-                                       // expected-note@vecintrin.h:* 8 {{candidate function not viable}}
-                                       // expected-note@vecintrin.h:* 1 {{must be a constant power of 2 from 64 to 4096}}
+                                       // expected-note@vecintrin.h:* 9 {{must be a constant power of 2 from 64 to 4096}}
   vsc = vec_load_bndry(cptrsc, 8192);  // expected-error {{no matching function}}
-                                       // expected-note@vecintrin.h:* 8 {{candidate function not viable}}
-                                       // expected-note@vecintrin.h:* 1 {{must be a constant power of 2 from 64 to 4096}}
+                                       // expected-note@vecintrin.h:* 9 {{must be a constant power of 2 from 64 to 4096}}
   vuc = vec_load_bndry(cptruc, idx);   // expected-error {{no matching function}}
-                                       // expected-note@vecintrin.h:* 8 {{candidate function not viable}}
-                                       // expected-note@vecintrin.h:* 1 {{must be a constant power of 2 from 64 to 4096}}
+                                       // expected-note@vecintrin.h:* 9 {{must be a constant power of 2 from 64 to 4096}}
   vss = vec_load_bndry(cptrss, idx);   // expected-error {{no matching function}}
-                                       // expected-note@vecintrin.h:* 8 {{candidate function not viable}}
-                                       // expected-note@vecintrin.h:* 1 {{must be a constant power of 2 from 64 to 4096}}
+                                       // expected-note@vecintrin.h:* 9 {{must be a constant power of 2 from 64 to 4096}}
   vus = vec_load_bndry(cptrus, idx);   // expected-error {{no matching function}}
-                                       // expected-note@vecintrin.h:* 8 {{candidate function not viable}}
-                                       // expected-note@vecintrin.h:* 1 {{must be a constant power of 2 from 64 to 4096}}
+                                       // expected-note@vecintrin.h:* 9 {{must be a constant power of 2 from 64 to 4096}}
   vsi = vec_load_bndry(cptrsi, idx);   // expected-error {{no matching function}}
-                                       // expected-note@vecintrin.h:* 8 {{candidate function not viable}}
-                                       // expected-note@vecintrin.h:* 1 {{must be a constant power of 2 from 64 to 4096}}
+                                       // expected-note@vecintrin.h:* 9 {{must be a constant power of 2 from 64 to 4096}}
   vui = vec_load_bndry(cptrui, idx);   // expected-error {{no matching function}}
-                                       // expected-note@vecintrin.h:* 8 {{candidate function not viable}}
-                                       // expected-note@vecintrin.h:* 1 {{must be a constant power of 2 from 64 to 4096}}
+                                       // expected-note@vecintrin.h:* 9 {{must be a constant power of 2 from 64 to 4096}}
   vsl = vec_load_bndry(cptrsl, idx);   // expected-error {{no matching function}}
-                                       // expected-note@vecintrin.h:* 8 {{candidate function not viable}}
-                                       // expected-note@vecintrin.h:* 1 {{must be a constant power of 2 from 64 to 4096}}
+                                       // expected-note@vecintrin.h:* 9 {{must be a constant power of 2 from 64 to 4096}}
   vul = vec_load_bndry(cptrul, idx);   // expected-error {{no matching function}}
-                                       // expected-note@vecintrin.h:* 8 {{candidate function not viable}}
-                                       // expected-note@vecintrin.h:* 1 {{must be a constant power of 2 from 64 to 4096}}
+                                       // expected-note@vecintrin.h:* 9 {{must be a constant power of 2 from 64 to 4096}}
 
   vuc = vec_genmask(idx);  // expected-error {{no matching function}}
                            // expected-note@vecintrin.h:* {{must be a constant integer}}
diff --git a/test/CodeGen/overloadable.c b/test/CodeGen/overloadable.c
index 634820cfe74..d9769c21f11 100644
--- a/test/CodeGen/overloadable.c
+++ b/test/CodeGen/overloadable.c
@@ -59,3 +59,18 @@ void foo() {
   // CHECK: @_Z13addrof_singlePc
   void *vp3 = &addrof_single;
 }
+
+
+void ovl_bar(char *) __attribute__((overloadable));
+void ovl_bar(int) __attribute__((overloadable));
+
+// CHECK-LABEL: define void @bar
+void bar() {
+  char charbuf[1];
+  unsigned char ucharbuf[1];
+
+  // CHECK: call void @_Z7ovl_barPc
+  ovl_bar(charbuf);
+  // CHECK: call void @_Z7ovl_barPc
+  ovl_bar(ucharbuf);
+}
diff --git a/test/Sema/overloadable.c b/test/Sema/overloadable.c
index b518aa90a01..f5e17d21191 100644
--- a/test/Sema/overloadable.c
+++ b/test/Sema/overloadable.c
@@ -23,7 +23,7 @@ float *accept_funcptr(int (*)(int, double)) __attribute__((overloadable)); //  \
 void test_funcptr(int (*f1)(int, double),
                   int (*f2)(int, float)) {
   float *fp = accept_funcptr(f1);
-  accept_funcptr(f2); // expected-error{{no matching function for call to 'accept_funcptr'}}
+  accept_funcptr(f2); // expected-error{{call to 'accept_funcptr' is ambiguous}}
 }
 
 struct X { int x; float y; };
@@ -122,3 +122,32 @@ void fn_type_conversions() {
 
   void *specific_disabled = &disabled;
 }
+
+void incompatible_pointer_type_conversions() {
+  char charbuf[1];
+  unsigned char ucharbuf[1];
+  int intbuf[1];
+
+  void foo(char *c) __attribute__((overloadable));
+  void foo(short *c) __attribute__((overloadable));
+  foo(charbuf);
+  foo(ucharbuf); // expected-error{{call to 'foo' is ambiguous}} expected-note@131{{candidate function}} expected-note@132{{candidate function}}
+  foo(intbuf); // expected-error{{call to 'foo' is ambiguous}} expected-note@131{{candidate function}} expected-note@132{{candidate function}}
+
+  void bar(unsigned char *c) __attribute__((overloadable));
+  void bar(signed char *c) __attribute__((overloadable));
+  bar(charbuf); // expected-error{{call to 'bar' is ambiguous}} expected-note@137{{candidate function}} expected-note@138{{candidate function}}
+  bar(ucharbuf);
+  bar(intbuf); // expected-error{{call to 'bar' is ambiguous}} expected-note@137{{candidate function}} expected-note@138{{candidate function}}
+}
+
+void dropping_qualifiers_is_incompatible() {
+  const char ccharbuf[1];
+  volatile char vcharbuf[1];
+
+  void foo(char *c) __attribute__((overloadable));
+  void foo(const volatile unsigned char *c) __attribute__((overloadable));
+
+  foo(ccharbuf); // expected-error{{call to 'foo' is ambiguous}} expected-note@148{{candidate function}} expected-note@149{{candidate function}}
+  foo(vcharbuf); // expected-error{{call to 'foo' is ambiguous}} expected-note@148{{candidate function}} expected-note@149{{candidate function}}
+}
diff --git a/test/Sema/pass-object-size.c b/test/Sema/pass-object-size.c
index ddfbbd5fc4e..0745105df83 100644
--- a/test/Sema/pass-object-size.c
+++ b/test/Sema/pass-object-size.c
@@ -52,5 +52,5 @@ void FunctionPtrs() {
 
   int P;
   (&NotOverloaded)(&P); //expected-error{{cannot take address of function 'NotOverloaded' because parameter 1 has pass_object_size attribute}}
-  (&IsOverloaded)(&P); //expected-error{{no matching function}} expected-note@35{{candidate address cannot be taken because parameter 1 has pass_object_size attribute}} expected-note@36{{candidate function not viable: no known conversion from 'int *' to 'char *' for 1st argument}}
+  (&IsOverloaded)(&P); //expected-warning{{incompatible pointer types passing 'int *' to parameter of type 'char *'}} expected-note@36{{passing argument to parameter 'p' here}}
 }
diff --git a/test/SemaOpenCL/event_t_overload.cl b/test/SemaOpenCL/event_t_overload.cl
index bc3ec44bd29..0bc67c13c72 100644
--- a/test/SemaOpenCL/event_t_overload.cl
+++ b/test/SemaOpenCL/event_t_overload.cl
@@ -1,11 +1,11 @@
 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only
 
-void __attribute__((overloadable)) foo(event_t, __local char *); // expected-note {{candidate function not viable: no known conversion from '__global int *' to '__local char *' for 2nd argument}}
-void __attribute__((overloadable)) foo(event_t, __local float *); // expected-note {{candidate function not viable: no known conversion from '__global int *' to '__local float *' for 2nd argument}}
+void __attribute__((overloadable)) foo(event_t, __local char *); // expected-note {{candidate function}}
+void __attribute__((overloadable)) foo(event_t, __local float *); // expected-note {{candidate function}}
 
 void kernel ker(__local char *src1, __local float *src2, __global int *src3) {
   event_t evt;
   foo(evt, src1);
   foo(0, src2);
-  foo(evt, src3); // expected-error {{no matching function for call to 'foo'}}
+  foo(evt, src3); // expected-error {{call to 'foo' is ambiguous}}
 }
-- 
GitLab