kastiglione added inline comments.
================
Comment at: lldb/source/Commands/CommandObjectDWIMPrint.cpp:81-83
+ << "note: object description requested, but type doesn't implement "
+ "a custom object description. Consider using \"p\" instead of "
+ "\"po\"\n";
----------------
I wonder if there are users who might ignore this, and then find it annoying to
see it repeatedly. Should it be a once per session warning? Once per class? Or
controlled by a setting.
================
Comment at: lldb/source/Commands/CommandObjectDWIMPrint.cpp:153-154
- valobj_sp->Dump(result.GetOutputStream(), dump_options);
+ MaybeAddPoHintAndDump(*valobj_sp.get(), dump_options, language, is_po,
+ result.GetOutputStream());
+
----------------
I know it would be less "DRY", but it also feels weird to me to pass in a bool
that controls whether the function does anything. At the callsite, I think the
semantics would be more clear as:
```
if (is_po)
MaybeAddPoHintAndDump(*valobj_sp.get(), dump_options, language,
result.GetOutputStream());
```
================
Comment at: lldb/source/Commands/CommandObjectDWIMPrint.cpp:177
if (valobj_sp->GetError().GetError() != UserExpression::kNoResult)
- valobj_sp->Dump(result.GetOutputStream(), dump_options);
+ MaybeAddPoHintAndDump(*valobj_sp.get(), dump_options, language, is_po,
+ result.GetOutputStream());
----------------
================
Comment at: lldb/source/Commands/CommandObjectDWIMPrint.h:46-51
+ /// Add a hint if object description was requested, but no description
+ /// function was implemented, and dump valobj to output_stream after.
+ static void MaybeAddPoHintAndDump(ValueObject &valobj,
+ const DumpValueObjectOptions &dump_options,
+ lldb::LanguageType language, bool is_po,
+ Stream &output_stream);
----------------
Should this hint be part of dwim-print only? At this point I see `expression`
as a command you use to do exactly what you ask it to do.
================
Comment at: lldb/source/Commands/CommandObjectFrame.cpp:708
rec_value_sp->GetPreferredDisplayLanguage());
+ rec_value_sp->GetPreferredDisplayLanguage();
options.SetRootValueObjectName(rec_value_sp->GetName().AsCString());
----------------
Why is this line added? If this getter has side effects, we should create an
appropriately named function that performs the side effects.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D153489/new/
https://reviews.llvm.org/D153489
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
