Use a manipulator to add a value to the current diagnostic flag.

[dnovillo]

Summary

This addresses the feedback to
http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20140421/103598.html

Richard, Alp, is this along the lines of what you folks were thinking?

[rsmith]

LGTM with some tiny tweaks, thanks!

include/clang/Basic/Diagnostic.h

696

The .str() should not be necessary here.

1016

Please make this constructor explicit.

1123

clear is a more common way of spelling this than erase.

include/clang/Basic/Diagnostic.h

public:
   /// \brief Issue the message to the client.
   ///
   /// This actually returns an instance of DiagnosticBuilder which emits the
   /// diagnostics (through @c ProcessDiag) when it is destroyed.
   ///
   /// \param DiagID A member of the @c diag::kind enum.
   /// \param Loc Represents the source location associated with the diagnostic,
   /// which can be an invalid location if no position information is available.
-  /// \param Val A string that represents the value that triggered
-  /// this diagnostic. If given, this value will be emitted as "=value"
-  /// after the flag name.
-  inline DiagnosticBuilder Report(SourceLocation Loc, unsigned DiagID,
-                                  StringRef Val);
   inline DiagnosticBuilder Report(SourceLocation Loc, unsigned DiagID);
   inline DiagnosticBuilder Report(unsigned DiagID);
 
   void Report(const StoredDiagnostic &storedDiag);
 
   /// \brief Determine whethere there is already a diagnostic in flight.
   bool isDiagnosticInFlight() const { return CurDiagID != ~0U; }
 
public:
   /// diagnostic. A copy of this string will be stored in the
   /// DiagnosticsEngine object itself.
   void SetDelayedDiagnostic(unsigned DiagID, StringRef Arg1 = "",
                             StringRef Arg2 = "");
   
   /// \brief Clear out the current diagnostic.
   void Clear() { CurDiagID = ~0U; }
 
-  /// \brief Return the overridden name for this diagnostic flag.
+  /// \brief Return the value associated to this diagnostic flag.
   StringRef getFlagNameValue() const { return StringRef(FlagNameValue); }
 
+  /// \brief Set the value associated to this diagnostic flag.
+  void setFlagNameValue(StringRef V) { FlagNameValue = V.str(); }
+
 private:
   /// \brief Report the delayed diagnostic.
   void ReportDelayed();
 
   // This is private state used by DiagnosticBuilder.  We put it here instead of
   // in DiagnosticBuilder in order to keep DiagnosticBuilder a small lightweight
   // object.  This implementation choice means that we can only have one
   // diagnostic "in flight" at a time, but this seems to be a reasonable
public:
 
   bool hasMaxRanges() const {
     return NumRanges == DiagnosticsEngine::MaxRanges;
   }
 
   bool hasMaxFixItHints() const {
     return NumFixits == DiagnosticsEngine::MaxFixItHints;
   }
+
+  void addFlagValue(StringRef V) const { DiagObj->setFlagNameValue(V); }
+};
+
+struct AddFlagValue {
+  AddFlagValue(StringRef V) : Val(V) {}
+  StringRef Val;
 };
 
+/// \brief Register a value for the flag in the current diagnostic. This
+/// value will be shown as the suffix "=value" after the flag name. It is
+/// useful in cases where the diagnostic flag accepts values (e.g.,
+/// -Rpass or -Wframe-larger-than).
+inline const DiagnosticBuilder &operator<<(const DiagnosticBuilder &DB,
+                                           const AddFlagValue V) {
+  DB.addFlagValue(V.Val);
+  return DB;
+}
+
 inline const DiagnosticBuilder &operator<<(const DiagnosticBuilder &DB,
                                            StringRef S) {
   DB.AddString(S);
   return DB;
 }
 
 inline const DiagnosticBuilder &operator<<(const DiagnosticBuilder &DB,
                                            const char *Str) {

 
 inline const DiagnosticBuilder &operator<<(const DiagnosticBuilder &DB,
                                            const FixItHint &Hint) {
   if (!Hint.isNull())
     DB.AddFixItHint(Hint);
   return DB;
 }
 
-inline DiagnosticBuilder
-DiagnosticsEngine::Report(SourceLocation Loc, unsigned DiagID, StringRef Val) {
+inline DiagnosticBuilder DiagnosticsEngine::Report(SourceLocation Loc,
+                                                   unsigned DiagID) {
   assert(CurDiagID == ~0U && "Multiple diagnostics in flight at once!");
   CurDiagLoc = Loc;
   CurDiagID = DiagID;
-  FlagNameValue = Val.str();
+  FlagNameValue.erase();
   return DiagnosticBuilder(this);
 }
 
-inline DiagnosticBuilder DiagnosticsEngine::Report(SourceLocation Loc,
-                                                   unsigned DiagID) {
-  return Report(Loc, DiagID, "");
-}
-
 inline DiagnosticBuilder DiagnosticsEngine::Report(unsigned DiagID) {
-  return Report(SourceLocation(), DiagID, "");
+  return Report(SourceLocation(), DiagID);
 }
 
 //===----------------------------------------------------------------------===//
 // Diagnostic
 //===----------------------------------------------------------------------===//
 
 /// A little helper class (which is basically a smart pointer that forwards
 /// info from DiagnosticsEngine) that allows clients to enquire about the

lib/CodeGen/CodeGenAction.cpp

if (CodeGenOpts.OptimizationRemarkPattern &&
     if (Line > 0) {
       // If -gcolumn-info was not used, Column will be 0. This upsets the
       // source manager, so if Column is not set, set it to 1.
       if (Column == 0)
         Column = 1;
       Loc = SourceMgr.translateFileLineCol(FileMgr.getFile(Filename), Line,
                                            Column);
     }
-    Diags.Report(Loc, diag::remark_fe_backend_optimization_remark,
-                 D.getPassName())
-        << D.getMsg().str();
+    Diags.Report(Loc, diag::remark_fe_backend_optimization_remark)
+        << AddFlagValue(D.getPassName()) << D.getMsg().str();
 
     if (Line == 0)
       // If we could not extract a source location for the diagnostic,
       // inform the user how they can get source locations back.
       //
       // FIXME: We should really be generating !srcloc annotations when
       // -Rpass is used. !srcloc annotations need to be emitted in
       // approximately the same spots as !dbg nodes.