teemperor requested changes to this revision.
teemperor added a comment.
This revision now requires changes to proceed.
A few comments, but otherwise this seems good.
================
Comment at: lldb/include/lldb/Target/Language.h:214
+ virtual llvm::StringRef NilReferenceSummaryString() { return {}; }
+
----------------
`/// Returns the summary string that should be used for a ValueObject for which
IsNilReference() is true` or something like that.
Also I think this probably should have a `Get` prefix like the other methods in
this class.
================
Comment at: lldb/source/DataFormatters/ValueObjectPrinter.cpp:367
+ else
+ summary.assign("0x0");
+ } else if (IsUninitialized())
----------------
JDevlieghere wrote:
> If we had a C runtime we could print "NULL", but I don't think it's worth
> adding that just for this. Another alternative would be to just have a switch
> based on the `LanguageType`, but that seems like a pretty bad idea from a
> layering perspective.
Not sure what to think about hardcoding `0x0` here. You can specify all kind of
formatters that would format the value here differently and this seems kinda
inconsistent.
```
(lldb) expr --format d -- nullptr
(nullptr_t) $2 = 0
(lldb) expr --format b -- nullptr
(nullptr_t) $3 =
0b0000000000000000000000000000000000000000000000000000000000000000
```
Could we just fall to the default formatter if we don't have a special summary
from the plugin?
================
Comment at: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:1138
+bool CPlusPlusLanguage::IsNilReference(ValueObject &valobj) {
+ if (valobj.GetCompilerType().GetMinimumLanguage() !=
+ eLanguageTypeC_plus_plus ||
----------------
Nit:
`!Language::LanguageIsCPlusPlus(valobj.GetCompilerType().GetMinimumLanguage())`
? (as we might return a specific C++ standard in the future from
GetMinimumLanguage).
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77153/new/
https://reviews.llvm.org/D77153
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
