[PATCH] D12131: Make [Sema]DiagnosticBuilder move-only, instead of having a sneaky mutating copy ctor.

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 18 18:35:39 PDT 2015


rsmith added inline comments.

================
Comment at: include/clang/Basic/Diagnostic.h:936-937
@@ -935,3 +935,4 @@
 public:
   /// Copy constructor.  When copied, this "takes" the diagnostic info from the
   /// input and neuters it.
+  DiagnosticBuilder(DiagnosticBuilder &&D) {
----------------
Comment is out of date.

================
Comment at: include/clang/Sema/Sema.h:1085-1092
@@ -1084,10 +1084,10 @@
 
     /// Teach operator<< to produce an object of the correct type.
     template<typename T>
     friend const SemaDiagnosticBuilder &operator<<(
         const SemaDiagnosticBuilder &Diag, const T &Value) {
       const DiagnosticBuilder &BaseDiag = Diag;
       BaseDiag << Value;
       return Diag;
     }
   };
----------------
If we only need to support value category preservation for `SemaDiagnosticBuilder`, can we duplicate this template for the `SemaDiagnosticBuilder &&operator<<(SemaDiagnosticBuilder &&Diag, const T &Value)` case?

================
Comment at: lib/Parse/Parser.cpp:163-170
@@ -162,5 +162,10 @@
 
-  DiagnosticBuilder DB =
-      Spelling
-          ? Diag(EndLoc, DiagID) << FixItHint::CreateInsertion(EndLoc, Spelling)
-          : Diag(Tok, DiagID);
+  DiagnosticBuilder DB = [&]() {
+    if (!Spelling)
+      return Diag(Tok, DiagID);
+
+    auto D = Diag(EndLoc, DiagID);
+    D << FixItHint::CreateInsertion(EndLoc, Spelling);
+    return D;
+  }();
+
----------------
This one might be more readable as

  DiagnosticBuilder DB = std::move(Spelling ? ... : ...);

Thoughts?


http://reviews.llvm.org/D12131





More information about the cfe-commits mailing list