[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#3059520 <https://reviews.llvm.org/D111416#3059520>, @dblaikie wrote:

> In D111416#3059466 <https://reviews.llvm.org/D111416#3059466>, @ljmf00 wrote:
>
>> 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 :)
>
> I think we're getting into the weeds a bit.
>
> A few things that might help: Whole grammar rules don't have to be implemented atomically - a patch could add support for, say, only a basic integer type if that's accessible/testable right now but other types aren't accessible/testable. Then when function types (basic functionality of that could be tested only with integer types?) are added and other types are accessible via that - other types could be added and tested.
>
> When I said out of order, what I meant was maybe some other functionality could be added first (and tested) that would then ennable these type parsing cases to be tested when they're added on top. For instance, maybe function types can be added without these other types being supported - starting with a `void()` function.

Ok, that makes sense to me. So adding only a few types here to prove this functionality, adding basic testing for them and introduce the rest of the types after function types get introduced and test them.

> But, to answer the other question, yes return types are rendered when they are mangled. In C++ templates get mangled return types, and they are rendered in the demangling:
>
>   $ cat mangle.cpp
>   void f1() { }
>   template<typename T>
>   void f2() { }
>   int main() {
>     f2<int>();
>   }
>   $ clang++-tot mangle.cpp -c && nm mangle.o | llvm-cxxfilt
>   0000000000000010 T main
>   0000000000000000 T f1()
>   0000000000000000 W void f2<int>()

Ok, didn't know that. This can be introduced in a future patch then and do what you suggested, right now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111416