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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D77153#1955829 <https://reviews.llvm.org/D77153#1955829>, @friss wrote:

> I was checking whether there is a way to catch null pointer before a type summary is even chosen. And found out that such logic exists, but only for Objective-C. Languages have a `IsNilReference()` virtual method that can tell whether a `ValueObject` is a nil reference. It's only implemented for ObjectiveC and value objects that test positive for this method are displayed as "nil" is the generic code of `ValueObjectPrinter.cpp`. I can see a few options:
>
> - I can rework the patch to affect only the libc++ string formatter which was the issue I was trying to solve in the first place
> - I can implement `IsNilReference()` for C++ and change `ValueObjectPrinter` to print C++ "nil references" as "nullptr".
> - I do the above and add another customization point in the Language plugins which tells `ValueObjectPrinter` out to display "nil" references, so that there's no Language specific code in `ValueObjectPrinter`.


All of those options sound fine to me. Making that generic is obviously preferable, but I don't know how much work that would be.

The thing that I found surprising when looking at this is that `std::string` has the exact same summary as a `std::string *` which points to it. I would have expected at least some subtle hint (perhaps by prepending the summary with a hex pointer value?) to indicate that we're looking at a pointer. This would be doable if the generic code had some way to understand pointers (and their nullness).

> A few additional questions come to mind though:
> 
> - Was the intent if `IsNilReference()` to be Objective-C specific in the first place?
> - Should we do the same thing for C and NULL?
> - C, C++ and Objective-C(++) being highly intermingled, would something like this even do the right thing in all cases?

Good questions, and I don't really have an answer for that. I don't think that `IsNilReference` should be c++-specific. Printing C pointers as NULL would be nice, but i don't know if we have any summaries for C types anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77153