[PATCH] D139010: [clang][WebAssembly] Implement support for table types and builtins

[Paulo Matos via Phabricator via cfe-commits]

pmatos added a comment.

Thanks for the comments - I am working on addressing these at the moment.
The LLVM part of the patch is just some refactoring and therefore should be pretty trivial, pinging @tlively in case he has some time.



================
Comment at: clang/test/Sema/wasm-refs-and-tables.c:17
+static __externref_t t6[] = {0}; // expected-error {{only zero-length WebAssembly tables are currently supported}}
+__externref_t t7[0];             // expected-error {{WebAssembly table must be static}}
+static __externref_t t8[0][0];   // expected-error {{multi-dimensional arrays of WebAssembly references are not allowed}}
----------------
aaron.ballman wrote:
> So why is `extern __externref_t r2;` allowed? Is it because it's not an array declaration?
I am not sure I understand the question. The externref value can be declared in another module and here we just define that. Array declarations of externref just define tables of externref values.


================
Comment at: clang/test/Sema/wasm-refs-and-tables.c:48
+void illegal_argument_4(__externref_t ***table);    // expected-error {{pointer to WebAssembly reference type is not allowed}}
+void illegal_argument_5(__externref_t (*table)[0]); // expected-error {{cannot form a pointer to a WebAssembly table}}
+
----------------
aaron.ballman wrote:
> How about:
> ```
> void foo(__externref_t table[0]);
> ```
> I'd expect this to also not be allowed (that's just a fancy spelling for a weird pointer).
That's correct, that's not allowed. Added the test case.


================
Comment at: clang/test/Sema/wasm-refs-and-tables.c:104
+  table[0] = ref;                 // expected-error {{cannot subscript a WebAssembly table}}
+
+  return ref;
----------------
aaron.ballman wrote:
> Please don't hate me, but... what about:
> ```
> int i = 0;
> __externref_t oh_no_vlas[i];
> ```
> 
:) Test added.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139010