[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

[Paulo Matos via Phabricator via llvm-commits]

pmatos marked an inline comment as done.
pmatos added a comment.

@aaron.ballman many thanks for the thorough reviews on the patches. Happy to see this moving in the right direction.



================
Comment at: clang/lib/AST/MicrosoftMangle.cpp:2480-2481
 #include "clang/Basic/RISCVVTypes.def"
+#define WASM_TYPE(Name, Id, SingletonId) case BuiltinType::Id:
+#include "clang/Basic/WebAssemblyReferenceTypes.def"
   case BuiltinType::ShortAccum:
----------------
aaron.ballman wrote:
> pmatos wrote:
> > aaron.ballman wrote:
> > > aaron.ballman wrote:
> > > > Is it reasonable that this simply cannot mangle in Microsoft ABI mode?
> > > Still wondering about this
> > @aaron.ballman Why wouldn't externref_t be able to mangle in Microsoft ABI mode? Quite clueless when it comes to ABIs.
> Including the wasm types here means that in Microsoft mode, use of those types in a context which requires mangling will diagnose (see this snippet just below):
> ```
>     unsigned DiagID = Diags.getCustomDiagID(
>         DiagnosticsEngine::Error, "cannot mangle this built-in %0 type yet");
>     Diags.Report(Range.getBegin(), DiagID)
>         << T->getName(Context.getASTContext().getPrintingPolicy()) << Range;
> ```
> So I'm wondering whether you consider that to be reasonable behavior or not. If you don't want the diagnostic, then you should add a vendor extension mangling for the type.
Oh - now I understand your point. I had forgotten I had initially written this 'workaround' to worry with later. Yes, let me add a vendor extension mangling for Microsoft ABI.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122215