[PATCH] D62960: SVE opaque type for C intrinsics demo

[Richard Sandiford via Phabricator via cfe-commits]

rsandifo-arm marked an inline comment as done.
rsandifo-arm added inline comments.


================
Comment at: lib/AST/ItaniumMangle.cpp:2680
+    break;
+#include "clang/Basic/AArch64SVEACLETypes.def"
   }
----------------
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.



Repository:
  rC Clang

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

https://reviews.llvm.org/D62960