[libcxx-commits] [PATCH] D124010: [llvm-cxxfilt] Unable to demangle a template argument which happens to be a null pointer

[David Blaikie via Phabricator via libcxx-commits]

dblaikie added a comment.

In D124010#3474930 <https://reviews.llvm.org/D124010#3474930>, @gbreynoo wrote:

> Apologies for the delay in response and thank you for your input @dblaikie and @urnathan.
>
> In D124010#3459710 <https://reviews.llvm.org/D124010#3459710>, @dblaikie wrote:
>
>> Should Clang be changing to not use this mangling, so it uses a mangling that matches GCC?
>>
>> And the demangle AST you're creating here is for an enum, and a cast from 0 to nullptr - but I'm not sure that's portable (even if it is inconsistent between producers) - nullptr_t isn't necessarily an enum, and I don't think C++ requires that you can cast 0 to nullptr_t?
>
> I'm not sure what you mean regarding clang's use of this mangling. From my understanding this is legal in the specification and so should be handled appropriately as input?

Oh, sorry, I wasn't saying clang should be changed instead of this patch, but in addition to/as well as this patch. It may be legal, but if Clang and GCC use different manglings I assume that means there would be ABI incompatibility in some cases, where a function defined in code compiled with GCC couldn't be correctly called from code compiled with Clang, for instance.



================
Comment at: llvm/include/llvm/Demangle/ItaniumDemangle.h:4357-4360
+    // A template argument which happens to be a null pointer is mangled as if
+    // it were a literal 0 of the appropriate pointer type.
+    if (consumeIf("Dn0E"))
+      return make<EnumLiteral>(make<NameType>("std::nullptr_t"), "0");
----------------
gbreynoo wrote:
> urnathan wrote:
> > Why demangle this differently to just 'nullptr'?  I notice GNU's demangler does that:
> > ```
> > zathras:11>c++filt _Z1fIDnLDn0EEvv
> > void f<decltype(nullptr), (decltype(nullptr))0>()
> > ```
> > It would seem 
> > ```
> > if (consumeIf("Dn") && (consumeIf ('0'), consumeIf ('E'))
> >   return make<NameType>("nullptr");
> > return nullptr;
> > ```
> > would suffice?
> This would output nullptr rather than decltype(nullptr) or nullptr_t. Looking at the spec "a template argument which happens to be a null pointer should mangled as if it were a literal 0 of the appropriate pointer type" which in the case of LDn0E is nullptr_t.
I think @urnathan is intending to suggest that what this patch currently produces "(decltype(nullptr))0" instead we could/should produce "nullptr". This seems like it would be valid, yeah? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124010