[clang] 5e03565 - [clang/Diagnostic] Use `optional` to disambiguate between a `StoredDiagMessage` that is not set vs set as empty string

[Argyrios Kyrtzidis via cfe-commits]

Author: Argyrios Kyrtzidis
Date: 2023-03-03T12:48:48-08:00
New Revision: 5e035651fd3acbb2645abbe80cae332d90eac78a

URL: https://github.com/llvm/llvm-project/commit/5e035651fd3acbb2645abbe80cae332d90eac78a
DIFF: https://github.com/llvm/llvm-project/commit/5e035651fd3acbb2645abbe80cae332d90eac78a.diff

LOG: [clang/Diagnostic] Use `optional` to disambiguate between a `StoredDiagMessage` that is not set vs set as empty string

"But when would you have a completely empty diagnostic message", you ask dear reader?
That is when there is an empty "#warning" in code.

rdar://106155415

Differential Revision: https://reviews.llvm.org/D145256

Added: 
    

Modified: 
    clang/include/clang/Basic/Diagnostic.h
    clang/lib/Basic/Diagnostic.cpp
    clang/unittests/Basic/DiagnosticTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Basic/Diagnostic.h b/clang/include/clang/Basic/Diagnostic.h
index b9ba459d1358b..5606a22fe9d68 100644
--- a/clang/include/clang/Basic/Diagnostic.h
+++ b/clang/include/clang/Basic/Diagnostic.h
@@ -1565,7 +1565,7 @@ inline DiagnosticBuilder DiagnosticsEngine::Report(unsigned DiagID) {
 /// currently in-flight diagnostic.
 class Diagnostic {
   const DiagnosticsEngine *DiagObj;
-  StringRef StoredDiagMessage;
+  std::optional<StringRef> StoredDiagMessage;
 
 public:
   explicit Diagnostic(const DiagnosticsEngine *DO) : DiagObj(DO) {}

diff  --git a/clang/lib/Basic/Diagnostic.cpp b/clang/lib/Basic/Diagnostic.cpp
index dbe62ecb50d33..3474acbced195 100644
--- a/clang/lib/Basic/Diagnostic.cpp
+++ b/clang/lib/Basic/Diagnostic.cpp
@@ -793,8 +793,8 @@ static const char *getTokenDescForDiagnostic(tok::TokenKind Kind) {
 /// array.
 void Diagnostic::
 FormatDiagnostic(SmallVectorImpl<char> &OutStr) const {
-  if (!StoredDiagMessage.empty()) {
-    OutStr.append(StoredDiagMessage.begin(), StoredDiagMessage.end());
+  if (StoredDiagMessage.has_value()) {
+    OutStr.append(StoredDiagMessage->begin(), StoredDiagMessage->end());
     return;
   }
 

diff  --git a/clang/unittests/Basic/DiagnosticTest.cpp b/clang/unittests/Basic/DiagnosticTest.cpp
index f4ce7e187f8f2..7469019391716 100644
--- a/clang/unittests/Basic/DiagnosticTest.cpp
+++ b/clang/unittests/Basic/DiagnosticTest.cpp
@@ -9,6 +9,7 @@
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticError.h"
 #include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Basic/DiagnosticLex.h"
 #include "gtest/gtest.h"
 #include <optional>
 
@@ -128,4 +129,26 @@ TEST(DiagnosticTest, diagnosticError) {
   EXPECT_EQ(*Value, std::make_pair(20, 1));
   EXPECT_EQ(Value->first, 20);
 }
+
+TEST(DiagnosticTest, storedDiagEmptyWarning) {
+  DiagnosticsEngine Diags(new DiagnosticIDs(), new DiagnosticOptions);
+
+  class CaptureDiagnosticConsumer : public DiagnosticConsumer {
+  public:
+    SmallVector<StoredDiagnostic> StoredDiags;
+
+    void HandleDiagnostic(DiagnosticsEngine::Level level,
+                          const Diagnostic &Info) override {
+      StoredDiags.push_back(StoredDiagnostic(level, Info));
+    }
+  };
+
+  CaptureDiagnosticConsumer CaptureConsumer;
+  Diags.setClient(&CaptureConsumer, /*ShouldOwnClient=*/false);
+  Diags.Report(diag::pp_hash_warning) << "";
+  ASSERT_TRUE(CaptureConsumer.StoredDiags.size() == 1);
+
+  // Make sure an empty warning can round-trip with \c StoredDiagnostic.
+  Diags.Report(CaptureConsumer.StoredDiags.front());
+}
 }