[clang] Fix message text for `-Wpointer-sign` to account for plain char

The `-Wpointer-sign` warning text is inappropriate for describing the
incompatible pointer conversion between plain `char` and explicitly
`signed`/`unsigned` `char` (whichever plain `char` has the same range
as) and vice versa.

Specifically, in part, it reads "converts between pointers to integer
types with different sign". This patch changes that portion to read
instead as "converts between pointers to integer types where one is of
the unique plain 'char' type and the other is not" when one of the types
is plain `char`.

C17 subclause 6.5.16.1 indicates that the conversions resulting in
`-Wpointer-sign` warnings in assignment-like contexts are constraint
violations. This means that strict conformance requires a diagnostic for
the case where the message text is wrong before this patch. The lack of
an even more specialized warning group is consistent with GCC.

Reviewed By: aaron.ballman

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

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index c048fc8..19b0033 100644 (file)
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7814,21 +7814,11 @@ def ext_typecheck_convert_incompatible_pointer_sign : ExtWarn<
   "|%diff{sending $ to parameter of type $|"
   "sending to parameter of different type}0,1"
   "|%diff{casting $ to type $|casting between types}0,1}2"
-  " converts between pointers to integer types with different sign">,
+  " converts between pointers to integer types %select{with different sign|"
+  "where one is of the unique plain 'char' type and the other is not}3">,
   InGroup<DiagGroup<"pointer-sign">>;
-def err_typecheck_convert_incompatible_pointer_sign : Error<
-  "%select{%diff{assigning to $ from $|assigning to different types}0,1"
-  "|%diff{passing $ to parameter of type $|"
-  "passing to parameter of different type}0,1"
-  "|%diff{returning $ from a function with result type $|"
-  "returning from function with different return type}0,1"
-  "|%diff{converting $ to type $|converting between types}0,1"
-  "|%diff{initializing $ with an expression of type $|"
-  "initializing with expression of different type}0,1"
-  "|%diff{sending $ to parameter of type $|"
-  "sending to parameter of different type}0,1"
-  "|%diff{casting $ to type $|casting between types}0,1}2"
-  " converts between pointers to integer types with different sign">;
+def err_typecheck_convert_incompatible_pointer_sign :
+  Error<ext_typecheck_convert_incompatible_pointer_sign.Text>;
 def ext_typecheck_convert_incompatible_pointer : ExtWarn<
   "incompatible pointer types "
   "%select{%diff{assigning to $ from $|assigning to different types}0,1"

diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index da555c9..571c4fb 100644 (file)
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -15962,6 +15962,16 @@ bool Sema::DiagnoseAssignmentResult(AssignConvertType ConvTy,
   else
     FDiag << FirstType << SecondType << Action << SrcExpr->getSourceRange();
 
+  if (DiagKind == diag::ext_typecheck_convert_incompatible_pointer_sign ||
+      DiagKind == diag::err_typecheck_convert_incompatible_pointer_sign) {
+    auto isPlainChar = [](const clang::Type *Type) {
+      return Type->isSpecificBuiltinType(BuiltinType::Char_S) ||
+             Type->isSpecificBuiltinType(BuiltinType::Char_U);
+    };
+    FDiag << (isPlainChar(FirstType->getPointeeOrArrayElementType()) ||
+              isPlainChar(SecondType->getPointeeOrArrayElementType()));
+  }
+
   // If we can fix the conversion, suggest the FixIts.
   if (!ConvHints.isNull()) {
     for (FixItHint &H : ConvHints.Hints)

diff --git a/clang/test/Sema/incompatible-sign.c b/clang/test/Sema/incompatible-sign.c
index 74e8b5d..64c93d1 100644 (file)
--- a/clang/test/Sema/incompatible-sign.c
+++ b/clang/test/Sema/incompatible-sign.c
@@ -6,18 +6,18 @@ int b(unsigned* y) { return a(y); } // expected-warning {{passing 'unsigned int
 
 signed char *plainCharToSignedChar(signed char *arg) { // expected-note{{passing argument to parameter}}
   extern char c;
-  signed char *p = &c; // expected-warning {{converts between pointers to integer types with different sign}}
-  struct { signed char *p; } s = { &c }; // expected-warning {{converts between pointers to integer types with different sign}}
-  p = &c; // expected-warning {{converts between pointers to integer types with different sign}}
-  plainCharToSignedChar(&c); // expected-warning {{converts between pointers to integer types with different sign}}
-  return &c; // expected-warning {{converts between pointers to integer types with different sign}}
+  signed char *p = &c; // expected-warning {{converts between pointers to integer types where one is of the unique plain 'char' type and the other is not}}
+  struct { signed char *p; } s = { &c }; // expected-warning {{converts between pointers to integer types where one is of the unique plain 'char' type and the other is not}}
+  p = &c; // expected-warning {{converts between pointers to integer types where one is of the unique plain 'char' type and the other is not}}
+  plainCharToSignedChar(&c); // expected-warning {{converts between pointers to integer types where one is of the unique plain 'char' type and the other is not}}
+  return &c; // expected-warning {{converts between pointers to integer types where one is of the unique plain 'char' type and the other is not}}
 }
 
 char *unsignedCharToPlainChar(char *arg) { // expected-note{{passing argument to parameter}}
   extern unsigned char uc[];
-  char *p = uc; // expected-warning {{converts between pointers to integer types with different sign}}
-  (void) (char *[]){ [42] = uc }; // expected-warning {{converts between pointers to integer types with different sign}}
-  p = uc; // expected-warning {{converts between pointers to integer types with different sign}}
-  unsignedCharToPlainChar(uc); // expected-warning {{converts between pointers to integer types with different sign}}
-  return uc; // expected-warning {{converts between pointers to integer types with different sign}}
+  char *p = uc; // expected-warning {{converts between pointers to integer types where one is of the unique plain 'char' type and the other is not}}
+  (void) (char *[]){ [42] = uc }; // expected-warning {{converts between pointers to integer types where one is of the unique plain 'char' type and the other is not}}
+  p = uc; // expected-warning {{converts between pointers to integer types where one is of the unique plain 'char' type and the other is not}}
+  unsignedCharToPlainChar(uc); // expected-warning {{converts between pointers to integer types where one is of the unique plain 'char' type and the other is not}}
+  return uc; // expected-warning {{converts between pointers to integer types where one is of the unique plain 'char' type and the other is not}}
 }

diff --git a/clang/test/Sema/incompatible-sign.cpp b/clang/test/Sema/incompatible-sign.cpp
index ebe7585..6ad1466 100644 (file)
--- a/clang/test/Sema/incompatible-sign.cpp
+++ b/clang/test/Sema/incompatible-sign.cpp
@@ -4,11 +4,11 @@
 void plainToSigned() {
   extern char c;
   signed char *p;
-  p = &c; // expected-error {{converts between pointers to integer types with different sign}}
+  p = &c; // expected-error {{converts between pointers to integer types where one is of the unique plain 'char' type and the other is not}}
 }
 
 void unsignedToPlain() {
   extern unsigned char uc;
   char *p;
-  p = &uc; // expected-error {{converts between pointers to integer types with different sign}}
+  p = &uc; // expected-error {{converts between pointers to integer types where one is of the unique plain 'char' type and the other is not}}
 }

diff --git a/clang/test/SemaObjC/objc-cf-audited-warning.m b/clang/test/SemaObjC/objc-cf-audited-warning.m
index db78229..7a5fd8e 100644 (file)
--- a/clang/test/SemaObjC/objc-cf-audited-warning.m
+++ b/clang/test/SemaObjC/objc-cf-audited-warning.m
@@ -20,5 +20,5 @@ CF_IMPLICIT_BRIDGING_DISABLED
 
 void saveImageToJPG(const char *filename)
 {
-    CFURLRef url = CFURLCreateFromFileSystemRepresentation(kCFAllocatorDefault, filename, 10, 0); // expected-warning {{passing 'const char *' to parameter of type 'const UInt8 *' (aka 'const unsigned char *') converts between pointers to integer types with different sign}}
+    CFURLRef url = CFURLCreateFromFileSystemRepresentation(kCFAllocatorDefault, filename, 10, 0); // expected-warning {{passing 'const char *' to parameter of type 'const UInt8 *' (aka 'const unsigned char *') converts between pointers to integer types where one is of the unique plain 'char' type and the other is not}}
 }