[Lldb-commits] [lldb] [lldb] Make SBType::FindDirectNestedType work with expression ASTs (PR #89183)

Pavel Labath via lldb-commits lldb-commits at lists.llvm.org
Thu Apr 18 04:42:26 PDT 2024


labath wrote:

> > > One thing I'd like to get feedback on is whether this should be looking into base classes. The discussion on the original PR focuses on lexically nested types, but going through base classes (following usual language rules) is another form of nesting. Both the existing and the new implementations don't do that, but I think it would be more natural to do it. Of course, then the function can definitely return more than one result...
> > 
> > 
> > What you're describing here is in line with member lookup described in the Standard. While it might be a useful facility, it's at odds with the intent of `FindDirectNestedTypes()` to be a small building block for efficient AST traversal. For the use case I have for this function, any amount of lookup is going to regress the performance of the formatter, and subsequently responsiveness of the debugger. So if you're going to pursue this, I suggest exposing this as a new function.
> 
> I don't expect the simple `DeclContext` lookup proposed here to be expensive. What _is_ expensive from the data-formatters point of view is either full-blown expression evaluation or a global search in DWARF for some type.

Agreed. This should be the same lookup that clang does every time is sees a `::`. Going through (potentially many) calls to FindTypes would be a different story. The reason I was asking is because I was not sure if this behavior was just a side-effect of how the original implementation worked, or if it was the intention all along. Judging by the response, it's the latter.

> 
> Re. looking at base classes, don't have a strong opinion on it. There's some precedent for looking at base classes already in `TypeSystemClang` (e.g., `GetIndexOfChildWithName`). But as you say @labath, that would require an API change

That's makes sense. Even if it was not the intention, the API makes this the only reasonable implementation. I have no use for the base-class traversal (may change in the future). If there's is a need for it, we can add a different API.

https://github.com/llvm/llvm-project/pull/89183


More information about the lldb-commits mailing list