Timm =?utf-8?q?Bäder?= <tbae...@redhat.com> Message-ID: In-Reply-To: <llvm.org/llvm/llvm-project/pull/117...@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?= <tbae...@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 95af9476cf74cb79f149450aca29c87badc02f1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbae...@redhat.com> Date: Thu, 28 Nov 2024 11:00:29 +0100 Subject: [PATCH 2/2] Add %separated specifier --- clang/include/clang/Basic/Diagnostic.h | 19 +++++++++++- .../clang/Basic/DiagnosticSemaKinds.td | 8 ++--- clang/lib/Basic/Diagnostic.cpp | 29 +++++++++++++++++++ clang/lib/Sema/SemaDecl.cpp | 25 ++++------------ .../TableGen/ClangDiagnosticsEmitter.cpp | 7 ++++- 5 files changed, 62 insertions(+), 26 deletions(-) diff --git a/clang/include/clang/Basic/Diagnostic.h b/clang/include/clang/Basic/Diagnostic.h index d271accca3d3b7..d59974227b3686 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,10 @@ class DiagnosticsEngine : public RefCountedBase<DiagnosticsEngine> { ak_qualtype_pair, /// Attr * - ak_attr + ak_attr, + + /// APSInt * + ak_apsint, }; /// Represents on argument value, which is a union discriminated @@ -1366,6 +1370,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 +1592,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..f9db05140877d4 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 (%separated1)">; 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 (%separated1 bits) exceeds the width of its type; value will " + "be truncated to %separated2 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 (%separated2 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..1e8eb976ffc482 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 HandleSeparatedModifier(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, "separated")) { + llvm::APSInt ValAP = llvm::APSInt( + llvm::APInt(64, Val, /*IsSigned=*/true), /*IsUnsigned=*/false); + HandleSeparatedModifier(*this, ValAP, Argument, ArgumentLen, OutStr); } else { assert(ModifierLen == 0 && "Unknown integer modifier"); llvm::raw_svector_ostream(OutStr) << Val; @@ -1180,12 +1193,28 @@ FormatDiagnostic(const char *DiagStr, const char *DiagEnd, HandleOrdinalModifier(Val, OutStr); } else if (ModifierIs(Modifier, ModifierLen, "human")) { HandleIntegerHumanModifier(Val, OutStr); + } else if (ModifierIs(Modifier, ModifierLen, "separated")) { + llvm::APSInt ValAP = llvm::APSInt( + llvm::APInt(64, Val, /*IsSigned=*/false), /*IsUnsigned=*/true); + HandleSeparatedModifier(*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, "separated")) { + HandleSeparatedModifier(*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 ---- case DiagnosticsEngine::ak_tokenkind: { tok::TokenKind Kind = static_cast<tok::TokenKind>(getRawArg(ArgNo)); 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..48a3dd2fa0b7e3 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_Separated, MT_S, MT_Q, MT_ObjCClass, @@ -433,6 +434,8 @@ static StringRef getModifierName(ModifierType MT) { return "ordinal"; case MT_Human: return "human"; + case MT_Separated: + return "separated"; 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("separated", MT_Separated) .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_Separated: { Parsed.push_back(New<PlaceholderPiece>(ModType, parseModifier(Text))); continue; } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
