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

Mon Sep 12 23:57:03 PDT 2022

pmatos marked 10 inline comments as done.
pmatos added inline comments.

================
Comment at: clang/include/clang/AST/Type.h:1972-1973
+  /// Check if this is a WebAssembly Reference Type.
+  bool isWebAssemblyReferenceType() const;
+  bool isWebAssemblyExternrefType() const;
   /// Determines if this is a sizeless type supported by the
----------------
aaron.ballman wrote:
> It's unfortunate to name this with `ReferenceType` given that we already have a considerable number of APIs that assume "reference type" to mean `&` or `&&`. We run into similar problems with "pointer type" and objective-c pointers.
> 
> Basically, I worry we're setting ourselves up for another `isObjCObjectPointerType()`/`isPointerType()` situation.
I understand this concern. However, unsure what else to call it given that's what it is. It's a WebAssembly Reference Type, which indeed is different from a C/C++ ReferenceType. Could call it OpaqueType but that would be even worse. Not only it's not called an opaque type in Wasm, it's also not what LLVM users will think of as Opaque Types.

================
Comment at: clang/include/clang/Basic/Builtins.def:50
 //  p -> pid_t
+//  e -> wasm externref
 //  . -> "...".  This may only occur at the end of the function list.
----------------
aaron.ballman wrote:
> tlively wrote:
> > 
> How often do you expect to use this marking? If it's only going to be used once or twice, it might make sense to not steal a letter but instead use custom checking for the signature.
Ah yes, that probably makes sense. I will use a custom typechecker instead.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122215