[PATCH] D111416: [Demangle] Add support for D simple basic and compound types

[Luís Ferreira via Phabricator via llvm-commits]

ljmf00 added a comment.

In D111416#3059334 <https://reviews.llvm.org/D111416#3059334>, @dblaikie wrote:

> In D111416#3056708 <https://reviews.llvm.org/D111416#3056708>, @ljmf00 wrote:
>
>> In D111416#3052693 <https://reviews.llvm.org/D111416#3052693>, @dblaikie wrote:
>>
>>> Missing test coverage - all the cases added should be tested.
>>
>> Well, I thought about this but currently, the demangler is not prepending the type to the demangled symbol so, there is no way to visually test this until https://reviews.llvm.org/D111421. That is why those tests get introduced there instead (types are shown in function parameters). I can add tests to check if this, at least, doesn't fail (nullptr) although I thought that adding tests for all of the types expecting the same output was useless when https://reviews.llvm.org/D111421 get introduced.
>
> Perhaps the patches are out of order then, or split too much - code should be tested when it is introduced.

They are not out of order, according to the ABI specification grammar for name mangling: https://dlang.org/spec/abi.html#name_mangling
This is a problem on this particular demangler, since it doesn't include return types, or variable types. I can add tests for this, anyway, but as I described earlier, it won't be distinguishable at this point.
This behaviour is tested when function symbols are parsable (https://reviews.llvm.org/D111421). Function symbols are dependent of of TypeFunction node (https://dlang.org/spec/abi.html#TypeFunction) and subsequently dependent of Type node, which is what this patch implements.

There is two possible options I can think of:
A. Add intentional support for return types / variable types to the demangled symbol and add visible tests accordingly.
B. Add invisible tests to do increase coverage and remove them lately when https://reviews.llvm.org/D111421 get introduced.

To clarify what do I mean about invisible tests is:

  {"_D8demangle1ai", "demangle.a"},
  {"_D8demangle1aOi", "demangle.a"},
  {"_D8demangle1ae", "demangle.a"},

Those three tests include different mangled symbols but the same demangled symbol.

What are the implications of A. ? It will introduce some complexity to the demangler and doesn't add much benefit to the end user, since you can't have same symbol name with different types.
I guess another point that can be raised is consistency among the other demanglers on LLVM: do they include the return type of a function symbol or type of a variable? For what I analyzed from Rust, it doesn't, but I'm not too familiar with Rust and can't read/understand its mangled names easily to know if it includes the type of a symbol. If you can help me on giving me examples, we can discuss better on what is better to do here :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111416