[Lldb-commits] [clang] [lldb] [UBSan][BoundsSafety] Implement support for more expressive "trap reasons" (PR #154618)

[Dan Liew via lldb-commits]

================
@@ -1534,6 +1536,103 @@ inline DiagnosticBuilder DiagnosticsEngine::Report(unsigned DiagID) {
   return Report(SourceLocation(), DiagID);
 }
 
+//===----------------------------------------------------------------------===//
+// RuntimeTrapDiagnosticBuilder and helper classes
+//===----------------------------------------------------------------------===//
+
+/// Helper class for \class RuntimeTrapDiagnosticBuilder. This class stores the
+/// "trap reason" built by \class RuntimeTrapDiagnosticBuilder. This consists of
+/// a trap message and trap category.
+///
+/// It is intended that this object be allocated on the stack.
+class TrapReason {
+public:
+  TrapReason() {}
+  /// \return The trap message. Note the lifetime of the underlying storage for
+  /// the returned StringRef lives in this class which means the returned
+  /// StringRef should not be used after this class is destroyed.
+  StringRef getMessage() const { return Message; }
+
+  /// \return the trap category (e.g. "Undefined Behavior Sanitizer")
+  StringRef getCategory() const { return Category; }
+
+  bool isEmpty() const { return Message.size() == 0 && Category.size() == 0; }
+
+  /// \return a pointer to this object if it contains a non-empty trap reason,
+  /// otherwise return `nullptr`. This is a convenient helper for passing
+  /// TrapReason objects to function calls that have a `TrapReason*` parameter.
+  TrapReason *getPtr() {
+    if (isEmpty())
+      return nullptr;
+    return this;
+  }
+
+private:
+  llvm::SmallString<64> Message;
+  // The Category doesn't need its own storage because the StringRef points
+  // to a global constant string.
+  StringRef Category;
+
+  // Only this class can set the private fields.
+  friend class RuntimeTrapDiagnosticBuilder;
+};
+
+/// Class to make it convenient to initialize TrapReason objects which can be
+/// used to attach the "trap reason" to trap instructions.
+///
+/// Although this class inherits from \class DiagnosticBuilder it has slightly
+/// different semantics.
+///
+/// * This class should only be used with trap diagnostics (declared in
+/// `DiagnosticTrapKinds.td`).
+/// * The `RuntimeTrapDiagnosticBuilder` does not emit diagnostics to the normal
+///   diagnostics consumers on destruction like normal Diagnostic builders.
+///   Instead on destruction it assigns to the TrapReason object passed in to
+///   the constructor.
+///
+/// Given that this class inherits from `DiagnosticBuilder` it inherits all of
+/// its abilities to format diagnostic messages and consume various types in
+/// class (e.g. Type, Exprs, etc.). This makes it particularly suited to
+/// printing types and expressions from the AST while codegen-ing runtime
+/// checks.
+///
+///
+/// Example use via the `CodeGenModule::RuntimeDiag` helper.
+///
+/// \code
+/// {
+///   TrapReason TR;
+///   CGM.RuntimeDiag(diag::trap_diagnostic, TR) << 0 << SomeExpr << SomeType;
+///   consume(TR.getPtr());
+/// }
+/// \endcode
+///
+///
+class RuntimeTrapDiagnosticBuilder : public DiagnosticBuilder {
----------------
delcypher wrote:

I figured it out. I forgot that when I moved the class I also moved it into a different namespace so I forgot to update the forward declaration in `Diagnostic.h` to be in the correct namespace. So doing this "works":

```
index af26a04d9488..b957ca331781 100644
--- a/clang/include/clang/Basic/Diagnostic.h
+++ b/clang/include/clang/Basic/Diagnostic.h
@@ -60,6 +60,9 @@ class Preprocessor;
 class SourceManager;
 class StoredDiagnostic;
 
+namespace CodeGen {
+  class TrapReasonBuilder;
+} // namespace CodeGen
 namespace tok {
 
 enum TokenKind : unsigned short;
@@ -1234,6 +1237,7 @@ class DiagnosticBuilder : public StreamingDiagnostic {
   friend class DiagnosticsEngine;
   friend class PartialDiagnostic;
   friend class Diagnostic;
+  friend class CodeGen::TrapReasonBuilder;
 
   mutable DiagnosticsEngine *DiagObj = nullptr;
 
@@ -1260,13 +1264,14 @@ class DiagnosticBuilder : public StreamingDiagnostic {
 
   DiagnosticBuilder() = default;
 
-protected:
+
   DiagnosticBuilder(DiagnosticsEngine *DiagObj, SourceLocation DiagLoc,
                     unsigned DiagID);
 
   DiagnosticsEngine *getDiagnosticsEngine() const { return DiagObj; }
   unsigned getDiagID() const { return DiagID; }
 
+protected:
   /// Clear out the current diagnostic.
   void Clear() const {
     DiagObj = nullptr;
```

I think it's very ugly though to have the `CodeGen` namespace mentioned in the `Diagnostic.h` so I'd much rather not rely on using `friend class` and instead just rely on using the new `protected` accessor methods I added.

https://github.com/llvm/llvm-project/pull/154618