erik.pilkington added inline comments.
================ Comment at: lib/AST/ItaniumMangle.cpp:2680 + break; +#include "clang/Basic/AArch64SVEACLETypes.def" } ---------------- rsandifo-arm wrote: > erik.pilkington wrote: > > jfb wrote: > > > @rjmccall you probably should review this part. > > Sorry for the drive by comment, but: All of these mangling should really be > > using the "vendor extension" production IMO: > > > > `<type> ::= u <source-name>` > > > > As is, these manglings intrude on the users's namespace, (i.e. if they had > > a type named `objc_selector` or something), and confuse demanglers which > > incorrectly assume these are substitutable (vendor extension builtin types > > are substitutable too though, but that should be handled here). > It isn't obvious from the patch, but the SVE names that we're mangling are > predefined names like __SVInt8_t. rather than user-facing names like svint8_t > The predefined names and their mangling are defined by the platform ABI > (https://developer.arm.com/docs/100986/0000), so it wouldn't be valid for > another part of the implementation to use those names for something else. > > I realise you were making a general point here though, sorry. > The mangling in the document you linked does use the vendor extension production here though, i.e. the example is `void f(int8x8_t)`, which mangles to _Z1f**u10__Int8x8_t**. It is true that this shouldn't ever collide with another mangling in practice, but my point is there isn't any need to smuggle it into the mangling by pretending it's a user defined type, when the itanium grammar and related tools have a special way for vendors to add builtin types. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62960/new/ https://reviews.llvm.org/D62960 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
