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

[Hubert Tong via cfe-commits]

Author: Hubert Tong
Date: 2021-01-11T18:41:14-05:00
New Revision: c6ffe4d76fbf6ae505c0abccaf29017414265e32

URL: https://github.com/llvm/llvm-project/commit/c6ffe4d76fbf6ae505c0abccaf29017414265e32
DIFF: https://github.com/llvm/llvm-project/commit/c6ffe4d76fbf6ae505c0abccaf29017414265e32.diff

LOG: [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

Added: 
    

Modified: 
    clang/include/clang/Basic/DiagnosticSemaKinds.td
    clang/lib/Sema/SemaExpr.cpp
    clang/test/Sema/incompatible-sign.c
    clang/test/Sema/incompatible-sign.cpp
    clang/test/SemaObjC/objc-cf-audited-warning.m

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index c048fc89813f..19b003398b02 100644
--- 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 
diff erent type}0,1"
   "|%
diff {casting $ to type $|casting between types}0,1}2"
-  " converts between pointers to integer types with 
diff erent sign">,
+  " converts between pointers to integer types %select{with 
diff erent 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 
diff erent types}0,1"
-  "|%
diff {passing $ to parameter of type $|"
-  "passing to parameter of 
diff erent type}0,1"
-  "|%
diff {returning $ from a function with result type $|"
-  "returning from function with 
diff erent return type}0,1"
-  "|%
diff {converting $ to type $|converting between types}0,1"
-  "|%
diff {initializing $ with an expression of type $|"
-  "initializing with expression of 
diff erent type}0,1"
-  "|%
diff {sending $ to parameter of type $|"
-  "sending to parameter of 
diff erent type}0,1"
-  "|%
diff {casting $ to type $|casting between types}0,1}2"
-  " converts between pointers to integer types with 
diff erent 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 
diff erent types}0,1"

diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index da555c95eaf0..571c4fb091ab 100644
--- 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 74e8b5dcc1fc..64c93d1e5920 100644
--- 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 
diff erent sign}}
-  struct { signed char *p; } s = { &c }; // expected-warning {{converts between pointers to integer types with 
diff erent sign}}
-  p = &c; // expected-warning {{converts between pointers to integer types with 
diff erent sign}}
-  plainCharToSignedChar(&c); // expected-warning {{converts between pointers to integer types with 
diff erent sign}}
-  return &c; // expected-warning {{converts between pointers to integer types with 
diff erent 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 
diff erent sign}}
-  (void) (char *[]){ [42] = uc }; // expected-warning {{converts between pointers to integer types with 
diff erent sign}}
-  p = uc; // expected-warning {{converts between pointers to integer types with 
diff erent sign}}
-  unsignedCharToPlainChar(uc); // expected-warning {{converts between pointers to integer types with 
diff erent sign}}
-  return uc; // expected-warning {{converts between pointers to integer types with 
diff erent 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 ebe7585b81e0..6ad1466ba9e4 100644
--- 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 
diff erent 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 
diff erent 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 db782299b8ab..7a5fd8e153ec 100644
--- a/clang/test/SemaObjC/objc-cf-audited-warning.m
+++ b/clang/test/SemaObjC/objc-cf-audited-warning.m
@@ -20,5 +20,5 @@
 
 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 
diff erent 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}}
 }