[Lldb-commits] [PATCH] D53530: Fix (and improve) the support for C99 variable length array types

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

I now spent (way too much :-) time experimenting with alternative implementations of this patch. In https://reviews.llvm.org/D53961 there is a version that changes GetNumChildren() to take an execution context. I don't think that doing it this way is a good solution either and here's why:

If we move the logic to GetNumChildren; we can print the array elements, but the type remains unchanged, which makes for a worse user experience.

With VLAs implemented as dynamic types we get

  (lldb) fr v vla
  (int [4]) vla = ([0] = 1, [1] = 2)

if we only change GetNumChildren() we get

  (lldb) fr v vla
  (int []) vla = ([0] = 1, [1] = 2)

which is less nice, since we no longer see the size of the array.

What i'm proposing is to keep the implementation as a dynamic type in `ItaniumABILanguageRuntime.cpp` but reduce the surface area of the execution context pass-through in `DWARFASTParserClang.cpp`. Similar to what I did in https://reviews.llvm.org/D53961 we can add a `ResolveDynamicArrayUID(uid, exe_ctx)` method that only creates an array type. Since we need to re-create a fresh clang type at every execution context anyway, going through the single-function array-only DWARF parser seems like the right trade-off to me. Let me know what you think!


https://reviews.llvm.org/D53530