[Lldb-commits] [PATCH] D94077: Support unscoped enumeration members in the expression evaluator.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D94077#2486232 <https://reviews.llvm.org/D94077#2486232>, @werat wrote:

> In D94077#2481942 <https://reviews.llvm.org/D94077#2481942>, @labath wrote:
>
>> This suffers from the same problem as the other patch, where the other index classes (apple_names and debug_names) will essentially never be able to implement this feature. (Unless they re-index the dwarf themselves, that is, but this would defeat the purpose of having the index in the first place.)
>
> I'm not sure I completely understand.  Why does changing the `manual_dwarf_index` mean that the other index classes can't ever implement this feature? If, let's say, `debug_names` decides to support enum constants, then its data layout should be changed to either include enumerators as globals, or add a new section, or something else. Then it can be properly handled in LLDB too (maybe changing the way `manual_dwarf_index` works too to keep things consistent). As I understand, `manual_dwarf_index` can be changed anytime, since we're building it ourselves.
>
> Do I misunderstand how these indexes work and interact with each other?

Maybe "(n)ever" was too strong a word. Still, there is a fundamental difference between the "manual" index and the other two indexes:

- the manual index is entirely an lldb concept. We can change it whenever we want. We don't need to ask anyone for permission to do that, coordinate with anyone, or care about what anyone else is doing.
- the contents of the other two indexes is given by a specification (it case of apple_names, it's a local llvm spec, but it still suffers from similar problems, albeit less pronounced). We cannot unilaterally change anything about them. We first need to change the relevant specification, which requires gathering consensus and convincing people that this use case is really worth the extra amount of space that the new entries will introduce. Then we need to wait until a new version of the standard is published and producers (compilers) to start using it. And even them our job is not finished, because we still need to figure out what do we want to do with debug info produced by older producers which still adhere to the old version of the spec.

This is why I am pushing back against the "simple" solution of having the manual index index more things -- it fragments the feature set for users who have the index vs. those who not.

This doesn't mean we cannot change the manual index at all -- it's possible some feature cannot be implemented differently. But it's also possible the feature can be implemented differently, in a way that makes works with all indexes. Or it's possible the feature is not worth the divergence, or the increase in the manual index memory footprint that it produces (i think it would be interesting to get some measurements on that, particularly as one can also expect a similar increase in the on-disk indexes, if they are to support this). So, I think this deserves a wider discussion and exploration of other alternatives.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94077