Skip to content

Commit

Permalink
[clang] Fix message text for -Wpointer-sign to account for plain char
Browse files Browse the repository at this point in the history
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
  • Loading branch information
hubert-reinterpretcast committed Jan 11, 2021
1 parent f635bcd commit c6ffe4d
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 27 deletions.
18 changes: 4 additions & 14 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
10 changes: 10 additions & 0 deletions clang/lib/Sema/SemaExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
20 changes: 10 additions & 10 deletions clang/test/Sema/incompatible-sign.c
Original file line number Diff line number Diff line change
Expand Up @@ -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}}
}
4 changes: 2 additions & 2 deletions clang/test/Sema/incompatible-sign.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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}}
}
2 changes: 1 addition & 1 deletion clang/test/SemaObjC/objc-cf-audited-warning.m
Original file line number Diff line number Diff line change
Expand Up @@ -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}}
}

0 comments on commit c6ffe4d

Please sign in to comment.