Change ValueObjectPrinter to use references to m_orig_valobj, and ensure m_valobj can't be null

lldb/include/lldb/DataFormatters/DumpValueObjectOptions.h

@@ -34,7 +34,7 @@ class DumpValueObjectOptions {
     bool CanAllowExpansion() const;
 
     bool CanAllowExpansion(bool is_root, TypeSummaryImpl *entry,
-                          ValueObject *valobj, const std::string &summary);
+                          ValueObject &valobj, const std::string &summary);
   };
 
   struct PointerAsArraySettings {

lldb/include/lldb/DataFormatters/ValueObjectPrinter.h

@@ -21,10 +21,15 @@
 namespace lldb_private {
 
 class ValueObjectPrinter {
+  /// The ValueObjectPrinter is a one-shot printer for ValueObjects.  It
+  /// does not retain the ValueObject it is printing, that is the job of
+  /// its caller.  It also doesn't attempt to track changes in the
+  /// ValueObject, e.g. changing synthetic child providers or changing
+  /// dynamic vrs. static vrs. synthetic settings.
 public:
-  ValueObjectPrinter(ValueObject *valobj, Stream *s);
+  ValueObjectPrinter(ValueObject &valobj, Stream *s);
 
-  ValueObjectPrinter(ValueObject *valobj, Stream *s,
+  ValueObjectPrinter(ValueObject &valobj, Stream *s,
                      const DumpValueObjectOptions &options);
 
   ~ValueObjectPrinter() = default;
@@ -39,21 +44,37 @@ class ValueObjectPrinter {
 
   // only this class (and subclasses, if any) should ever be concerned with the
   // depth mechanism
-  ValueObjectPrinter(ValueObject *valobj, Stream *s,
+  ValueObjectPrinter(ValueObject &valobj, Stream *s,
                      const DumpValueObjectOptions &options,
                      const DumpValueObjectOptions::PointerDepth &ptr_depth,
                      uint32_t curr_depth,
                      InstancePointersSetSP printed_instance_pointers);
 
   // we should actually be using delegating constructors here but some versions
   // of GCC still have trouble with those
-  void Init(ValueObject *valobj, Stream *s,
+  void Init(ValueObject &valobj, Stream *s,
             const DumpValueObjectOptions &options,
             const DumpValueObjectOptions::PointerDepth &ptr_depth,
             uint32_t curr_depth,
             InstancePointersSetSP printed_instance_pointers);
 
-  bool GetMostSpecializedValue();
+  /// Cache the ValueObject we are actually going to print.  If this
+  /// ValueObject has a Dynamic type, we return that, if either the original
+  /// ValueObject or its Dynamic type has a Synthetic provider, return that.
+  /// This will never return an empty ValueObject, since we use the ValueObject
+  /// to carry errors.
+  /// Note, this gets called when making the printer object, and uses the
+  /// use dynamic and use synthetic settings of the ValueObject being printed,
+  /// so changes made to these settings won't affect already made
+  /// ValueObjectPrinters. SetupMostSpecializedValue();
+
+  /// Access the cached "most specialized value" - that is the one to use for
+  /// printing the value object's value.  However, be sure to use
+  /// GetValueForChildGeneration when you are generating the children of this
+  /// value.
+  ValueObject &GetMostSpecializedValue();
+
+  void SetupMostSpecializedValue();
 
   const char *GetDescriptionForDisplay();
 
@@ -95,13 +116,13 @@ class ValueObjectPrinter {
 
   bool ShouldExpandEmptyAggregates();
 
-  ValueObject *GetValueObjectForChildrenGeneration();
+  ValueObject &GetValueObjectForChildrenGeneration();
 
   void PrintChildrenPreamble(bool value_printed, bool summary_printed);
 
   void PrintChildrenPostamble(bool print_dotdotdot);
 
-  lldb::ValueObjectSP GenerateChild(ValueObject *synth_valobj, size_t idx);
+  lldb::ValueObjectSP GenerateChild(ValueObject &synth_valobj, size_t idx);
 
   void PrintChild(lldb::ValueObjectSP child_sp,
                   const DumpValueObjectOptions::PointerDepth &curr_ptr_depth);
@@ -121,8 +142,10 @@ class ValueObjectPrinter {
 private:
   bool ShouldShowName() const;
 
-  ValueObject *m_orig_valobj;
-  ValueObject *m_valobj;
+  ValueObject &m_orig_valobj;
+  ValueObject *m_cached_valobj; /// Cache the current "most specialized" value.
+                                /// Don't use this directly, use
+                                /// GetMostSpecializedValue.
   Stream *m_stream;
   DumpValueObjectOptions m_options;
   Flags m_type_flags;

lldb/source/Commands/CommandObjectFrame.cpp

@@ -177,7 +177,10 @@ class CommandObjectFrameDiagnose : public CommandObjectParsed {
 
     DumpValueObjectOptions options;
     options.SetDeclPrintingHelper(helper);
-    ValueObjectPrinter printer(valobj_sp.get(), &result.GetOutputStream(),
+    // We've already handled the case where the value object sp is null, so
+    // this is just to make sure future changes don't skip that:
+    assert(valobj_sp.get() && "Must have a valid ValueObject to print");
+    ValueObjectPrinter printer(*valobj_sp, &result.GetOutputStream(),
                                options);
     printer.PrintValueObject();
   }

lldb/source/Core/ValueObject.cpp

@@ -2649,7 +2649,7 @@ void ValueObject::Dump(Stream &s, const DumpValueObjectOptions &options) {
 #ifdef LLDB_ENABLE_SWIFT
   auto swift_scratch_ctx_lock = SwiftScratchContextLock(GetSwiftExeCtx(*this));
 #endif // LLDB_ENABLE_SWIFT
-  ValueObjectPrinter printer(this, &s, options);
+  ValueObjectPrinter printer(*this, &s, options);
   printer.PrintValueObject();
 }
 

lldb/source/DataFormatters/TypeSummary.cpp

@@ -80,7 +80,10 @@ bool StringSummaryFormat::FormatObject(ValueObject *valobj, std::string &retval,
     sc = frame->GetSymbolContext(lldb::eSymbolContextEverything);
 
   if (IsOneLiner()) {
-    ValueObjectPrinter printer(valobj, &s, DumpValueObjectOptions());
+    // We've already checked the case of a NULL valobj above.  Let's put in an
+    // assert here to make sure someone doesn't take that out:
+    assert(valobj && "Must have a valid ValueObject to summarize");
+    ValueObjectPrinter printer(*valobj, &s, DumpValueObjectOptions());
     printer.PrintChildrenOneLiner(HideNames(valobj));
     retval = std::string(s.GetString());
     return true;