jimingham wrote: > > > > I'd rather keep the flexibility, if you don't mind doing the work to > > > > add that. For now, it seems okay for the current libc++/libstdc++ ones > > > > to declare what they support (which is 1 pointer depth). If someone has > > > > a reason to come back and extend these ones in the future, that's fine > > > > but a separate piece of work. > > > > > > > > > After rethinking about the option, it will cause some compatibility > > > issues. Currently, --skip-pointers is boolean option and python > > > formatters use the flag lldb.eTypeOptionSkipPointers to disable/enable > > > it. If we switch it to accept a numeric number, we probably need a new > > > SBAPI to specify the parameter, and this will break existing users who > > > use the `lldb.eTypeOptionSkipPointers` flag. > > > > > > `type summary add` and friends don't have that many options. Adding a > > "pointer matching depth" option, and having --skip-pointers and > > `--pointer-match-depth` be in separate option groups so you can't > > accidentally provide them together would also work. > > I find this is tricky because all the flags for data formatters are set with > `TypeSummaryImpl::Flags` which is a 4 byte unsigned integer. I could add a > new unsigned integer in `TypeSummaryImpl` for that. But for users who use > SBAPI to create formatters, they need to do something like > `CreateWithSummaryString(..., options)` and then > `SetPointerMatchDepth(depth)` to specify the pointer match depth because we > cannot break the existing `CreateWithSummaryString` APIs. That looks not > desired, or maybe not that bad. What do you think?
If you want to preserve the current behavior, then CreateWithSummaryString should set the summary up to recurse infinitely. Then people who wanted new behavior would have to call SetPointerMatchDepth. Since that's new behavior, it's fine that the old API's don't support it. We allow overloads that are going to be unambiguous on the Python side - so we could also make a version that takes an extra depth parameter, you wouldn't have to add the API. TTTT, whenever we've made an APi that takes some set of options in the SB API's and we've done it with some bools or some flags, we always end up regretting it. We have an SBTypeSummaryOptions, maybe we should add the flags ivar to that and some API's to set it's various bits, and add the depth to that, and provide versions of the API that take this options object. But that's a bunch more work you maybe haven't signed up for... In which case, adding the depth variant would be fine too. https://github.com/llvm/llvm-project/pull/124048 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
