[Lldb-commits] [PATCH] D77153: [lldb/DataFormatters] Display null C++ pointers as nullptr

Raphael Isemann via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Wed Nov 11 01:52:58 PST 2020

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).



More information about the lldb-commits mailing list