[Lldb-commits] [PATCH] D75929: [DebugInfo] Support DWARFv5 index sections.

Tue Mar 17 09:38:19 PDT 2020

labath added a comment.

In D75929#1924373 <https://reviews.llvm.org/D75929#1924373>, @ikudrin wrote:

> @dblaikie, @labath, as far as I can understand, the patch complies with your vision. The main difference is that the enum is still intended for internal use only, but it probably should not go to the public part before the proposed values are accepted. Anyway, even while the proposal of the combined index is not approved, I believe that the patch is useful per se because it allows reading standard index sections and can later be easily extended for combined indexes. The patch does not restrict that movement. Please, correct me if I misunderstand anything.

Sorry about the delay. It took a while to get this to the top of my stack.

Yes, this "complies with my/our vission", but looking at the `UnknownColumnIds` field, I am starting to have second thoughts about that vision. :( Being able to represent "unknown" columns and at the same time mapping all columns to a internal representation makes things a bit awkward.

The reason this wasn't a problem for location lists is that you cannot "skip over" an unknown DW_LLE entry -- it terminates the parse right away.

However, that is not the case for debug_?u_index, where you can easily ignore unknown columns. In that light, I am wondering if it wouldn't be better after all to stick to the on-disk column numbers internally (but maybe still keep the "unified" v5 enum in the public interface). @dblaikie, what do you make of that?

(btw, is there a test case for the "unknown column" code path?)

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

https://reviews.llvm.org/D75929