From c6ffe4d76fbf6ae505c0abccaf29017414265e32 Mon Sep 17 00:00:00 2001 From: Hubert Tong Date: Mon, 11 Jan 2021 18:36:28 -0500 Subject: [PATCH] [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 --- .../clang/Basic/DiagnosticSemaKinds.td | 18 ++++------------- clang/lib/Sema/SemaExpr.cpp | 10 ++++++++++ clang/test/Sema/incompatible-sign.c | 20 +++++++++---------- clang/test/Sema/incompatible-sign.cpp | 4 ++-- clang/test/SemaObjC/objc-cf-audited-warning.m | 2 +- 5 files changed, 27 insertions(+), 27 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index c048fc89813f8..19b003398b025 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 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>; -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; 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 da555c95eaf03..571c4fb091abd 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 74e8b5dcc1fcc..64c93d1e5920d 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 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 ebe7585b81e03..6ad1466ba9e48 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 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 db782299b8ab0..7a5fd8e153ec5 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 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}} }