Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
It seems awkward to me to be returning empty ValueObjectSP's by returning nullptr and then relying on conversion. We don't do it that way in most of the other formatters. If there wasn't a strong reason for doing it this way I think it would be clearer to return the object you're supposed to return.
Other than that this looks fine.
source/Plugins/Language/CPlusPlus/LibCxxTuple.cpp | ||
---|---|---|
47 ↗ | (On Diff #107282) | nullptr -> ValueObjectSP. It makes it clearer reading the thing what it is. |
53 ↗ | (On Diff #107282) | It looks a little weird to return nullptr for a function returning a ValueObjectSP. It would be clearer to return ValueObjectSP(), and that's what we do pretty much everywhere else. |
55 ↗ | (On Diff #107282) | ditto |
There wasn't a strong reason for using nullptr, I can convert those to ValueObjectSP().
Given that this was your only objection to this patch, I'm going to assume it is ok to land after this.