Add %separated specifier

Author: tbaederr

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 {

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;

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));

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;
     }
   }
 

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;
     }