[clang] 3b00e48 - Further update -Wbitfield-constant-conversion for 1-bit bitfield
Aaron Ballman via cfe-commits
cfe-commits at lists.llvm.org
Wed Aug 31 06:25:06 PDT 2022
Author: Aaron Ballman
Date: 2022-08-31T09:23:45-04:00
New Revision: 3b00e486797c0a28f0b5216ea507329d098ce573
URL: https://github.com/llvm/llvm-project/commit/3b00e486797c0a28f0b5216ea507329d098ce573
DIFF: https://github.com/llvm/llvm-project/commit/3b00e486797c0a28f0b5216ea507329d098ce573.diff
LOG: Further update -Wbitfield-constant-conversion for 1-bit bitfield
https://reviews.llvm.org/D131255 (82afc9b169a67e8b8a1862fb9c41a2cd974d6691)
began warning about conversion causing data loss for a single-bit
bit-field. However, after landing the changes, there were reports about
significant false positives from some code bases.
This alters the approach taken in that patch by introducing a new
warning group (-Wsingle-bit-bitfield-constant-conversion) which is
grouped under -Wbitfield-constant-conversion to allow users to
selectively disable the single-bit warning without losing the other
constant conversion warnings.
Differential Revision: https://reviews.llvm.org/D132851
Added:
Modified:
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaChecking.cpp
clang/test/CXX/drs/dr6xx.cpp
clang/test/Sema/constant-conversion.c
clang/test/SemaCXX/constant-conversion.cpp
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 944d88d230171..2251993a7c72b 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -106,9 +106,13 @@ Improvements to Clang's diagnostics
- ``-Wformat`` now recognizes ``%b`` for the ``printf``/``scanf`` family of
functions and ``%B`` for the ``printf`` family of functions. Fixes
`Issue 56885: <https://github.com/llvm/llvm-project/issues/56885>`_.
-- ``-Wbitfield-constant-conversion`` now diagnoses implicit truncation when 1 is
- assigned to a 1-bit signed integer bitfield. This fixes
- `Issue 53253 <https://github.com/llvm/llvm-project/issues/53253>`_.
+- Introduced ``-Wsingle-bit-bitfield-constant-conversion``, grouped under
+ ``-Wbitfield-constant-conversion``, which diagnoses implicit truncation when
+ ``1`` is assigned to a 1-bit signed integer bitfield. This fixes
+ `Issue 53253 <https://github.com/llvm/llvm-project/issues/53253>`_. To reduce
+ potential false positives, this diagnostic will not diagnose use of the
+ ``true`` macro (from ``<stdbool.h>>`) in C language mode despite the macro
+ being defined to expand to ``1``.
- ``-Wincompatible-function-pointer-types`` now defaults to an error in all C
language modes. It may be downgraded to a warning with
``-Wno-error=incompatible-function-pointer-types`` or disabled entirely with
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index ba2fef2a80858..825092b34339c 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -47,7 +47,10 @@ def BinaryLiteral : DiagGroup<"binary-literal", [CXX14BinaryLiteral,
CXXPre14CompatBinaryLiteral,
GNUBinaryLiteral]>;
def GNUCompoundLiteralInitializer : DiagGroup<"gnu-compound-literal-initializer">;
-def BitFieldConstantConversion : DiagGroup<"bitfield-constant-conversion">;
+def SingleBitBitFieldConstantConversion :
+ DiagGroup<"single-bit-bitfield-constant-conversion">;
+def BitFieldConstantConversion : DiagGroup<"bitfield-constant-conversion",
+ [SingleBitBitFieldConstantConversion]>;
def BitFieldEnumConversion : DiagGroup<"bitfield-enum-conversion">;
def BitFieldWidth : DiagGroup<"bitfield-width">;
def CompoundTokenSplitByMacro : DiagGroup<"compound-token-split-by-macro">;
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index ae10b13c8c769..160b931007adf 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3846,6 +3846,9 @@ def warn_impcast_integer_64_32 : Warning<
def warn_impcast_integer_precision_constant : Warning<
"implicit conversion from %2 to %3 changes value from %0 to %1">,
InGroup<ConstantConversion>;
+def warn_impcast_single_bit_bitield_precision_constant : Warning<
+ "implicit truncation from %2 to a one-bit wide bit-field changes value from "
+ "%0 to %1">, InGroup<SingleBitBitFieldConstantConversion>;
def warn_impcast_bitfield_precision_constant : Warning<
"implicit truncation from %2 to bit-field changes value from %0 to %1">,
InGroup<BitFieldConstantConversion>;
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index e2e3d64fcade7..8073223116c29 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -13047,6 +13047,18 @@ static bool AnalyzeBitFieldAssignment(Sema &S, FieldDecl *Bitfield, Expr *Init,
unsigned OriginalWidth = Value.getBitWidth();
+ // In C, the macro 'true' from stdbool.h will evaluate to '1'; To reduce
+ // false positives where the user is demonstrating they intend to use the
+ // bit-field as a Boolean, check to see if the value is 1 and we're assigning
+ // to a one-bit bit-field to see if the value came from a macro named 'true'.
+ bool OneAssignedToOneBitBitfield = FieldWidth == 1 && Value == 1;
+ if (OneAssignedToOneBitBitfield && !S.LangOpts.CPlusPlus) {
+ SourceLocation MaybeMacroLoc = OriginalInit->getBeginLoc();
+ if (S.SourceMgr.isInSystemMacro(MaybeMacroLoc) &&
+ S.findMacroSpelling(MaybeMacroLoc, "true"))
+ return false;
+ }
+
if (!Value.isSigned() || Value.isNegative())
if (UnaryOperator *UO = dyn_cast<UnaryOperator>(OriginalInit))
if (UO->getOpcode() == UO_Minus || UO->getOpcode() == UO_Not)
@@ -13067,9 +13079,11 @@ static bool AnalyzeBitFieldAssignment(Sema &S, FieldDecl *Bitfield, Expr *Init,
std::string PrettyValue = toString(Value, 10);
std::string PrettyTrunc = toString(TruncatedValue, 10);
- S.Diag(InitLoc, diag::warn_impcast_bitfield_precision_constant)
- << PrettyValue << PrettyTrunc << OriginalInit->getType()
- << Init->getSourceRange();
+ S.Diag(InitLoc, OneAssignedToOneBitBitfield
+ ? diag::warn_impcast_single_bit_bitield_precision_constant
+ : diag::warn_impcast_bitfield_precision_constant)
+ << PrettyValue << PrettyTrunc << OriginalInit->getType()
+ << Init->getSourceRange();
return true;
}
diff --git a/clang/test/CXX/drs/dr6xx.cpp b/clang/test/CXX/drs/dr6xx.cpp
index 9d8f4d669b61a..8c0da27e78c97 100644
--- a/clang/test/CXX/drs/dr6xx.cpp
+++ b/clang/test/CXX/drs/dr6xx.cpp
@@ -911,9 +911,9 @@ namespace dr674 { // dr674: 8
namespace dr675 { // dr675: dup 739
template<typename T> struct A { T n : 1; };
#if __cplusplus >= 201103L
- static_assert(A<char>{1}.n < 0, ""); // expected-warning {{implicit truncation from 'int' to bit-field changes value from 1 to -1}}
- static_assert(A<int>{1}.n < 0, ""); // expected-warning {{implicit truncation from 'int' to bit-field changes value from 1 to -1}}
- static_assert(A<long long>{1}.n < 0, ""); // expected-warning {{implicit truncation from 'int' to bit-field changes value from 1 to -1}}
+ static_assert(A<char>{1}.n < 0, ""); // expected-warning {{implicit truncation from 'int' to a one-bit wide bit-field changes value from 1 to -1}}
+ static_assert(A<int>{1}.n < 0, ""); // expected-warning {{implicit truncation from 'int' to a one-bit wide bit-field changes value from 1 to -1}}
+ static_assert(A<long long>{1}.n < 0, ""); // expected-warning {{implicit truncation from 'int' to a one-bit wide bit-field changes value from 1 to -1}}
#endif
}
diff --git a/clang/test/Sema/constant-conversion.c b/clang/test/Sema/constant-conversion.c
index d386d21f24aad..eebfeb1e1de89 100644
--- a/clang/test/Sema/constant-conversion.c
+++ b/clang/test/Sema/constant-conversion.c
@@ -1,4 +1,7 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-apple-darwin %s
+// RUN: %clang_cc1 -fsyntax-only -ffreestanding -verify=expected,one-bit -triple x86_64-apple-darwin %s
+// RUN: %clang_cc1 -fsyntax-only -ffreestanding -Wno-single-bit-bitfield-constant-conversion -verify -triple x86_64-apple-darwin %s
+
+#include <stdbool.h>
// This file tests -Wconstant-conversion, a subcategory of -Wconversion
// which is on by default.
@@ -19,12 +22,14 @@ void test(void) {
int b : 1; // The only valid values are 0 and -1.
} s;
- s.b = -3; // expected-warning {{implicit truncation from 'int' to bit-field changes value from -3 to -1}}
- s.b = -2; // expected-warning {{implicit truncation from 'int' to bit-field changes value from -2 to 0}}
- s.b = -1; // no-warning
- s.b = 0; // no-warning
- s.b = 1; // expected-warning {{implicit truncation from 'int' to bit-field changes value from 1 to -1}}
- s.b = 2; // expected-warning {{implicit truncation from 'int' to bit-field changes value from 2 to 0}}
+ s.b = -3; // expected-warning {{implicit truncation from 'int' to bit-field changes value from -3 to -1}}
+ s.b = -2; // expected-warning {{implicit truncation from 'int' to bit-field changes value from -2 to 0}}
+ s.b = -1; // no-warning
+ s.b = 0; // no-warning
+ s.b = 1; // one-bit-warning {{implicit truncation from 'int' to a one-bit wide bit-field changes value from 1 to -1}}
+ s.b = true; // no-warning (we suppress it manually to reduce false positives)
+ s.b = false; // no-warning
+ s.b = 2; // expected-warning {{implicit truncation from 'int' to bit-field changes value from 2 to 0}}
}
enum Test2 { K_zero, K_one };
diff --git a/clang/test/SemaCXX/constant-conversion.cpp b/clang/test/SemaCXX/constant-conversion.cpp
index 80deabf79a467..9be8b139e79e2 100644
--- a/clang/test/SemaCXX/constant-conversion.cpp
+++ b/clang/test/SemaCXX/constant-conversion.cpp
@@ -22,3 +22,12 @@ void too_big_for_char(int param) {
char ok3 = true ? 0 : 99999 + 1;
char ok4 = true ? 0 : nines() + 1;
}
+
+void test_bitfield() {
+ struct S {
+ int one_bit : 1;
+ } s;
+
+ s.one_bit = 1; // expected-warning {{implicit truncation from 'int' to a one-bit wide bit-field changes value from 1 to -1}}
+ s.one_bit = true; // no-warning
+}
More information about the cfe-commits
mailing list