kastiglione 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: ---------------- did you mean to use this, or should it be deleted? ================ Comment at: lldb/source/Commands/CommandObjectDWIMPrint.cpp:129-130 + static llvm::SmallPtrSet<Target *, 4> targets_warned; + if (targets_warned.contains(target_ptr)) + return; + ---------------- 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. ================ 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; ---------------- 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. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153489/new/ https://reviews.llvm.org/D153489 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
