[Lldb-commits] [PATCH] D153489: [lldb] Print hint if object description is requested but not implemented

[Augusto Noronha via Phabricator via lldb-commits]

augusto2112 added inline comments.


================
Comment at: lldb/include/lldb/Target/Target.h:496
 
+  std::once_flag target_once_flag;
   // These two functions fill out the Broadcaster interface:
----------------
kastiglione wrote:
> did you mean to use this, or should it be deleted?
Leftover from a different implementation, I'll delete it.


================
Comment at: lldb/source/Commands/CommandObjectDWIMPrint.cpp:129-130
+      static llvm::SmallPtrSet<Target *, 4> targets_warned;
+      if (targets_warned.contains(target_ptr))
+        return;
+
----------------
kastiglione wrote:
> question: would people want to be warned once per target, or once per type?
> 
> If I do like the warning, I might want to know every time I `po` a type that doesn't support it.
In my opinion once per target should be enough, if we do it for every type that could potentially be too noisy, and hopefully after seeing the hint a few times users will start remembering to try `p` too.


================
Comment at: lldb/source/Commands/CommandObjectDWIMPrint.cpp:158-162
+        StreamString temp_result_stream;
+        valobj_sp->Dump(temp_result_stream, dump_options);
+        llvm::StringRef output = temp_result_stream.GetString();
+        maybe_add_hint(output);
+        result.GetOutputStream() << output;
----------------
kastiglione wrote:
> what do you think of passing in the `result`'s stream into `maybe_add_hint`? Perhaps I am overlooking something, but I wonder if it would simplify the code to reuse the one stream, instead of separating and then combining two streams.
I need the two streams to print it in the correct order (hint first, result later)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153489/new/

https://reviews.llvm.org/D153489