[clang] [clang] Format bitfield width diagnostics with thousands-separators (PR #117763)
Timm Baeder via cfe-commits
cfe-commits at lists.llvm.org
Thu Nov 28 02:01:03 PST 2024
Timm =?utf-8?q?Bäder?= <tbaeder at redhat.com>
Message-ID:
In-Reply-To: <llvm.org/llvm/llvm-project/pull/117763 at github.com>
https://github.com/tbaederr updated https://github.com/llvm/llvm-project/pull/117763
>From 07b326b59bf9a8e385840a590c5162b9d1914650 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbaeder at redhat.com>
Date: Tue, 26 Nov 2024 19:26:32 +0100
Subject: [PATCH 1/2] [clang] Format bitfield width diagnostics with
thousands-separators
---
.../clang/Basic/DiagnosticSemaKinds.td | 2 +-
clang/lib/Sema/SemaDecl.cpp | 27 ++++++++++++++-----
clang/test/SemaCXX/bitfield-layout.cpp | 4 +--
3 files changed, 24 insertions(+), 9 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 6ff24c2bc8faad..5ab0885c8414fd 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6400,7 +6400,7 @@ def err_incorrect_number_of_vector_initializers : Error<
// Used by C++ which allows bit-fields that are wider than the type.
def warn_bitfield_width_exceeds_type_width: Warning<
"width of bit-field %0 (%1 bits) exceeds the width of its type; value will "
- "be truncated to %2 bit%s2">, InGroup<BitFieldWidth>;
+ "be truncated to %2 bits">, InGroup<BitFieldWidth>;
def err_bitfield_too_wide : Error<
"%select{bit-field %1|anonymous bit-field}0 is too wide (%2 bits)">;
def warn_bitfield_too_small_for_enum : Warning<
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 74b0e5ad23bd48..7378edc1c5cecb 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -18307,16 +18307,22 @@ ExprResult Sema::VerifyBitField(SourceLocation FieldLoc,
if (Value.isSigned() && Value.isNegative()) {
if (FieldName)
return Diag(FieldLoc, diag::err_bitfield_has_negative_width)
- << FieldName << toString(Value, 10);
+ << FieldName
+ << toString(Value, 10, Value.isSigned(),
+ /*formatAsCLiteral=*/false, /*UpperCase=*/true,
+ /*InsertSeparators=*/true);
return Diag(FieldLoc, diag::err_anon_bitfield_has_negative_width)
- << toString(Value, 10);
+ << toString(Value, 10, Value.isSigned(), /*formatAsCLiteral=*/false,
+ /*UpperCase=*/true, /*InsertSeparators=*/true);
}
// The size of the bit-field must not exceed our maximum permitted object
// size.
if (Value.getActiveBits() > ConstantArrayType::getMaxSizeBits(Context)) {
return Diag(FieldLoc, diag::err_bitfield_too_wide)
- << !FieldName << FieldName << toString(Value, 10);
+ << !FieldName << FieldName
+ << toString(Value, 10, Value.isSigned(), /*formatAsCLiteral=*/false,
+ /*UpperCase=*/true, /*InsertSeparators=*/true);
}
if (!FieldTy->isDependentType()) {
@@ -18335,7 +18341,10 @@ ExprResult Sema::VerifyBitField(SourceLocation FieldLoc,
unsigned DiagWidth =
CStdConstraintViolation ? TypeWidth : TypeStorageSize;
return Diag(FieldLoc, diag::err_bitfield_width_exceeds_type_width)
- << (bool)FieldName << FieldName << toString(Value, 10)
+ << (bool)FieldName << FieldName
+ << toString(Value, 10, Value.isSigned(),
+ /*formatAsCLiteral=*/false, /*UpperCase=*/true,
+ /*InsertSeparators=*/true)
<< !CStdConstraintViolation << DiagWidth;
}
@@ -18343,9 +18352,15 @@ ExprResult Sema::VerifyBitField(SourceLocation FieldLoc,
// specified bits as value bits: that's all integral types other than
// 'bool'.
if (BitfieldIsOverwide && !FieldTy->isBooleanType() && FieldName) {
+ llvm::APInt TypeWidthAP(sizeof(TypeWidth) * 8, TypeWidth,
+ /*IsSigned=*/false);
Diag(FieldLoc, diag::warn_bitfield_width_exceeds_type_width)
- << FieldName << toString(Value, 10)
- << (unsigned)TypeWidth;
+ << FieldName
+ << toString(Value, 10, Value.isSigned(), /*formatAsCLiteral=*/false,
+ /*UpperCase=*/true, /*InsertSeparators=*/true)
+ << toString(TypeWidthAP, 10, /*Signed=*/false,
+ /*formatAsCLiteral=*/false, /*UpperCase=*/true,
+ /*InsertSeparators=*/true);
}
}
diff --git a/clang/test/SemaCXX/bitfield-layout.cpp b/clang/test/SemaCXX/bitfield-layout.cpp
index 7efd1d38c682f5..951a4f72de1566 100644
--- a/clang/test/SemaCXX/bitfield-layout.cpp
+++ b/clang/test/SemaCXX/bitfield-layout.cpp
@@ -35,14 +35,14 @@ CHECK_SIZE(Test4, 8);
CHECK_ALIGN(Test4, 8);
struct Test5 {
- char c : 0x100000001; // expected-warning {{width of bit-field 'c' (4294967297 bits) exceeds the width of its type; value will be truncated to 8 bits}}
+ char c : 0x100000001; // expected-warning {{width of bit-field 'c' (4'294'967'297 bits) exceeds the width of its type; value will be truncated to 8 bits}}
};
// Size and align don't really matter here, just make sure we don't crash.
CHECK_SIZE(Test5, 1);
CHECK_ALIGN(Test5, 1);
struct Test6 {
- char c : (unsigned __int128)0xffffffffffffffff + 2; // expected-error {{bit-field 'c' is too wide (18446744073709551617 bits)}}
+ char c : (unsigned __int128)0xffffffffffffffff + 2; // expected-error {{bit-field 'c' is too wide (18'446'744'073'709'551'617 bits)}}
};
// Size and align don't really matter here, just make sure we don't crash.
CHECK_SIZE(Test6, 1);
>From 3490634cdea96669d1f585c0bf4095f0a905bed5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbaeder at redhat.com>
Date: Thu, 28 Nov 2024 11:00:29 +0100
Subject: [PATCH 2/2] Add %format specifier
---
clang/include/clang/Basic/Diagnostic.h | 18 +++++++++++-
.../clang/Basic/DiagnosticSemaKinds.td | 8 ++---
clang/lib/Basic/Diagnostic.cpp | 29 +++++++++++++++++++
clang/lib/Sema/SemaDecl.cpp | 25 ++++------------
.../TableGen/ClangDiagnosticsEmitter.cpp | 7 ++++-
5 files changed, 61 insertions(+), 26 deletions(-)
diff --git a/clang/include/clang/Basic/Diagnostic.h b/clang/include/clang/Basic/Diagnostic.h
index d271accca3d3b7..01da0192b6193e 100644
--- a/clang/include/clang/Basic/Diagnostic.h
+++ b/clang/include/clang/Basic/Diagnostic.h
@@ -18,6 +18,7 @@
#include "clang/Basic/DiagnosticOptions.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/Specifiers.h"
+#include "llvm/ADT/APSInt.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/FunctionExtras.h"
@@ -284,7 +285,9 @@ class DiagnosticsEngine : public RefCountedBase<DiagnosticsEngine> {
ak_qualtype_pair,
/// Attr *
- ak_attr
+ ak_attr,
+
+ ak_apsint,
};
/// Represents on argument value, which is a union discriminated
@@ -1366,6 +1369,12 @@ inline const StreamingDiagnostic &operator<<(const StreamingDiagnostic &DB,
return DB;
}
+inline const StreamingDiagnostic &operator<<(const StreamingDiagnostic &DB,
+ const llvm::APSInt *I) {
+ DB.AddTaggedVal(reinterpret_cast<intptr_t>(I), DiagnosticsEngine::ak_apsint);
+ return DB;
+}
+
// We use enable_if here to prevent that this overload is selected for
// pointers or other arguments that are implicitly convertible to bool.
template <typename T>
@@ -1582,6 +1591,13 @@ class Diagnostic {
DiagStorage.DiagArgumentsVal[Idx]);
}
+ const llvm::APSInt *getArgAPSInt(unsigned Idx) const {
+ assert(getArgKind(Idx) == DiagnosticsEngine::ak_apsint &&
+ "invalid argument accessor!");
+ return reinterpret_cast<const llvm::APSInt *>(
+ DiagStorage.DiagArgumentsVal[Idx]);
+ }
+
/// Return the specified non-string argument in an opaque form.
/// \pre getArgKind(Idx) != DiagnosticsEngine::ak_std_string
uint64_t getRawArg(unsigned Idx) const {
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 5ab0885c8414fd..703e59dfd7c6a8 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6387,7 +6387,7 @@ def err_init_objc_class : Error<
def err_implicit_empty_initializer : Error<
"initializer for aggregate with no elements requires explicit braces">;
def err_bitfield_has_negative_width : Error<
- "bit-field %0 has negative width (%1)">;
+ "bit-field %0 has negative width (%format1)">;
def err_anon_bitfield_has_negative_width : Error<
"anonymous bit-field has negative width (%0)">;
def err_bitfield_has_zero_width : Error<"named bit-field %0 has zero width">;
@@ -6399,10 +6399,10 @@ def err_incorrect_number_of_vector_initializers : Error<
// Used by C++ which allows bit-fields that are wider than the type.
def warn_bitfield_width_exceeds_type_width: Warning<
- "width of bit-field %0 (%1 bits) exceeds the width of its type; value will "
- "be truncated to %2 bits">, InGroup<BitFieldWidth>;
+ "width of bit-field %0 (%format1 bits) exceeds the width of its type; value will "
+ "be truncated to %format2 bit%s2">, InGroup<BitFieldWidth>;
def err_bitfield_too_wide : Error<
- "%select{bit-field %1|anonymous bit-field}0 is too wide (%2 bits)">;
+ "%select{bit-field %1|anonymous bit-field}0 is too wide (%format2 bits)">;
def warn_bitfield_too_small_for_enum : Warning<
"bit-field %0 is not wide enough to store all enumerators of %1">,
InGroup<BitFieldEnumConversion>, DefaultIgnore;
diff --git a/clang/lib/Basic/Diagnostic.cpp b/clang/lib/Basic/Diagnostic.cpp
index 2d0e358116362c..cb4c5b62137686 100644
--- a/clang/lib/Basic/Diagnostic.cpp
+++ b/clang/lib/Basic/Diagnostic.cpp
@@ -769,6 +769,15 @@ static void HandleSelectModifier(const Diagnostic &DInfo, unsigned ValNo,
DInfo.FormatDiagnostic(Argument, EndPtr, OutStr);
}
+static void HandleFormatModifier(const Diagnostic &DInfo,
+ const llvm::APSInt &Val, const char *Argument,
+ unsigned ArgumentLen,
+ SmallVectorImpl<char> &OutStr) {
+ llvm::raw_svector_ostream Out(OutStr);
+ Out << toString(Val, 10, Val.isSigned(), /*formatAsCLiteral=*/false,
+ /*UpperCase=*/false, /*InsertSeparators=*/true);
+}
+
/// HandleIntegerSModifier - Handle the integer 's' modifier. This adds the
/// letter 's' to the string if the value is not 1. This is used in cases like
/// this: "you idiot, you have %4 parameter%s4!".
@@ -1160,6 +1169,10 @@ FormatDiagnostic(const char *DiagStr, const char *DiagEnd,
HandleOrdinalModifier((unsigned)Val, OutStr);
} else if (ModifierIs(Modifier, ModifierLen, "human")) {
HandleIntegerHumanModifier(Val, OutStr);
+ } else if (ModifierIs(Modifier, ModifierLen, "format")) {
+ llvm::APSInt ValAP = llvm::APSInt(
+ llvm::APInt(64, Val, /*IsSigned=*/true), /*IsUnsigned=*/false);
+ HandleFormatModifier(*this, ValAP, Argument, ArgumentLen, OutStr);
} else {
assert(ModifierLen == 0 && "Unknown integer modifier");
llvm::raw_svector_ostream(OutStr) << Val;
@@ -1180,10 +1193,26 @@ FormatDiagnostic(const char *DiagStr, const char *DiagEnd,
HandleOrdinalModifier(Val, OutStr);
} else if (ModifierIs(Modifier, ModifierLen, "human")) {
HandleIntegerHumanModifier(Val, OutStr);
+ } else if (ModifierIs(Modifier, ModifierLen, "format")) {
+ llvm::APSInt ValAP = llvm::APSInt(
+ llvm::APInt(64, Val, /*IsSigned=*/false), /*IsUnsigned=*/true);
+ HandleFormatModifier(*this, ValAP, Argument, ArgumentLen, OutStr);
+ } else {
+ assert(ModifierLen == 0 && "Unknown integer modifier");
+ llvm::raw_svector_ostream(OutStr) << Val;
+ }
+ break;
+ }
+
+ case DiagnosticsEngine::ak_apsint: {
+ const llvm::APSInt *Val = getArgAPSInt(ArgNo);
+ if (ModifierIs(Modifier, ModifierLen, "format")) {
+ HandleFormatModifier(*this, *Val, Argument, ArgumentLen, OutStr);
} else {
assert(ModifierLen == 0 && "Unknown integer modifier");
llvm::raw_svector_ostream(OutStr) << Val;
}
+ // FIXME: Support the other modifiers for APSInt as well.
break;
}
// ---- TOKEN SPELLINGS ----
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 7378edc1c5cecb..36f3af7a169633 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -18307,22 +18307,15 @@ ExprResult Sema::VerifyBitField(SourceLocation FieldLoc,
if (Value.isSigned() && Value.isNegative()) {
if (FieldName)
return Diag(FieldLoc, diag::err_bitfield_has_negative_width)
- << FieldName
- << toString(Value, 10, Value.isSigned(),
- /*formatAsCLiteral=*/false, /*UpperCase=*/true,
- /*InsertSeparators=*/true);
- return Diag(FieldLoc, diag::err_anon_bitfield_has_negative_width)
- << toString(Value, 10, Value.isSigned(), /*formatAsCLiteral=*/false,
- /*UpperCase=*/true, /*InsertSeparators=*/true);
+ << FieldName << &Value;
+ return Diag(FieldLoc, diag::err_anon_bitfield_has_negative_width) << &Value;
}
// The size of the bit-field must not exceed our maximum permitted object
// size.
if (Value.getActiveBits() > ConstantArrayType::getMaxSizeBits(Context)) {
return Diag(FieldLoc, diag::err_bitfield_too_wide)
- << !FieldName << FieldName
- << toString(Value, 10, Value.isSigned(), /*formatAsCLiteral=*/false,
- /*UpperCase=*/true, /*InsertSeparators=*/true);
+ << !FieldName << FieldName << &Value;
}
if (!FieldTy->isDependentType()) {
@@ -18341,10 +18334,7 @@ ExprResult Sema::VerifyBitField(SourceLocation FieldLoc,
unsigned DiagWidth =
CStdConstraintViolation ? TypeWidth : TypeStorageSize;
return Diag(FieldLoc, diag::err_bitfield_width_exceeds_type_width)
- << (bool)FieldName << FieldName
- << toString(Value, 10, Value.isSigned(),
- /*formatAsCLiteral=*/false, /*UpperCase=*/true,
- /*InsertSeparators=*/true)
+ << (bool)FieldName << FieldName << &Value
<< !CStdConstraintViolation << DiagWidth;
}
@@ -18355,12 +18345,7 @@ ExprResult Sema::VerifyBitField(SourceLocation FieldLoc,
llvm::APInt TypeWidthAP(sizeof(TypeWidth) * 8, TypeWidth,
/*IsSigned=*/false);
Diag(FieldLoc, diag::warn_bitfield_width_exceeds_type_width)
- << FieldName
- << toString(Value, 10, Value.isSigned(), /*formatAsCLiteral=*/false,
- /*UpperCase=*/true, /*InsertSeparators=*/true)
- << toString(TypeWidthAP, 10, /*Signed=*/false,
- /*formatAsCLiteral=*/false, /*UpperCase=*/true,
- /*InsertSeparators=*/true);
+ << FieldName << &Value << (unsigned)TypeWidth;
}
}
diff --git a/clang/utils/TableGen/ClangDiagnosticsEmitter.cpp b/clang/utils/TableGen/ClangDiagnosticsEmitter.cpp
index 6a4a64a0813063..516b3e1f4074a9 100644
--- a/clang/utils/TableGen/ClangDiagnosticsEmitter.cpp
+++ b/clang/utils/TableGen/ClangDiagnosticsEmitter.cpp
@@ -413,6 +413,7 @@ enum ModifierType {
MT_Diff,
MT_Ordinal,
MT_Human,
+ MT_Format,
MT_S,
MT_Q,
MT_ObjCClass,
@@ -433,6 +434,8 @@ static StringRef getModifierName(ModifierType MT) {
return "ordinal";
case MT_Human:
return "human";
+ case MT_Format:
+ return "format";
case MT_S:
return "s";
case MT_Q:
@@ -1030,6 +1033,7 @@ Piece *DiagnosticTextBuilder::DiagText::parseDiagText(StringRef &Text,
.Case("s", MT_S)
.Case("ordinal", MT_Ordinal)
.Case("human", MT_Human)
+ .Case("format", MT_Format)
.Case("q", MT_Q)
.Case("objcclass", MT_ObjCClass)
.Case("objcinstance", MT_ObjCInstance)
@@ -1132,7 +1136,8 @@ Piece *DiagnosticTextBuilder::DiagText::parseDiagText(StringRef &Text,
case MT_ObjCClass:
case MT_ObjCInstance:
case MT_Ordinal:
- case MT_Human: {
+ case MT_Human:
+ case MT_Format: {
Parsed.push_back(New<PlaceholderPiece>(ModType, parseModifier(Text)));
continue;
}
More information about the cfe-commits
mailing list