Make diagnostics API safer to use

[aprantl]

Summary

I received a crash report in DiagnosticManager that was causes by a nullptr diagnostic having been added. The API allows passing in a null unique_ptr, but all the methods are written assuming that all pointers a dereferencable. This patch makes it impossible to add a null diagnostic.

rdar://107633615

[JDevlieghere]

LGTM

[mib]

LGTM!

lldb/include/lldb/Expression/DiagnosticManager.h

public:
   void AddDiagnostic(llvm::StringRef message, DiagnosticSeverity severity,
                      DiagnosticOrigin origin,
                      uint32_t compiler_id = LLDB_INVALID_COMPILER_ID) {
     m_diagnostics.emplace_back(
         std::make_unique<Diagnostic>(message, severity, origin, compiler_id));
   }
 
   void AddDiagnostic(std::unique_ptr<Diagnostic> diagnostic) {
+    if (diagnostic)
       m_diagnostics.push_back(std::move(diagnostic));
   }
 
   size_t Printf(DiagnosticSeverity severity, const char *format, ...)
       __attribute__((format(printf, 3, 4)));
   void PutString(DiagnosticSeverity severity, llvm::StringRef str);
 
   void AppendMessageToDiagnostic(llvm::StringRef str) {
     if (!m_diagnostics.empty())

lldb/unittests/Expression/DiagnosticManagerTest.cpp

TEST(DiagnosticManagerTest, HasFixits) {
   // Adding a diag with fixits. The manager should still return true.
   mgr.AddDiagnostic(std::make_unique<FixItDiag>("fixit", true));
   EXPECT_TRUE(mgr.HasFixIts());
 }
 
 TEST(DiagnosticManagerTest, GetStringNoDiags) {
   DiagnosticManager mgr;
   EXPECT_EQ("", mgr.GetString());
+  std::unique_ptr<Diagnostic> empty;
+  mgr.AddDiagnostic(std::move(empty));
+  EXPECT_EQ("", mgr.GetString());
 }
 
 TEST(DiagnosticManagerTest, GetStringBasic) {
   DiagnosticManager mgr;
   mgr.AddDiagnostic(
       std::make_unique<TextDiag>("abc", eDiagnosticSeverityError));
   EXPECT_EQ("error: abc\n", mgr.GetString());
 }