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

[Paulo Matos via Phabricator via cfe-commits]

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


================
Comment at: clang/test/Sema/builtins-wasm.c:12-13
+  __builtin_wasm_table_size(table, table);            // expected-error {{too many arguments to function call, expected 1, have 2}}
+  void *a = __builtin_wasm_table_size(table);         // expected-error {{incompatible integer to pointer conversion initializing 'void *' with an expression of type 'int'}}
+  __externref_t b = __builtin_wasm_table_size(table); // expected-error {{initializing '__externref_t' with an expression of incompatible type 'int'}}
+  int c = __builtin_wasm_table_size(table);
----------------
aaron.ballman wrote:
> Instead of relying on assignment diagnostics to test the return type of the call, why not use the language? (Same can be used for the other, similar tests.)
> ```
> #define EXPR_HAS_TYPE(expr, type) _Generic((expr), type : 1, default : 0)
> 
> _Static_assert(EXPR_HAS_TYPE(__builtin_wasm_table_size(table), int), "");
> ```
OK - I will apply those changes. I hadn't seen this way of doing this before. Thanks.


================
Comment at: clang/test/Sema/builtins-wasm.c:21
+  __builtin_wasm_table_grow(ref, ref, size);             // expected-error {{1st argument must be a WebAssembly table}}
+  __builtin_wasm_table_grow(table, table, size);         // expected-error {{2nd argument must match the element type of the WebAssembly table in the 1st argument}}
+  __builtin_wasm_table_grow(table, ref, table);          // expected-error {{3rd argument must be an integer}}
----------------
aaron.ballman wrote:
> I can't make sense of this diagnostic -- what is the element type of the web assembly table in the 1st argument? Why is the second argument needed at all if it needs to be derived from the type of the first argument and these things don't represent values (due to being zero-sized)?
The element type needs to be a reference type. Either an externref or a funcref. So, if we have a table of externref, the second element needs to have that type because it's the element we are growing the table with. The element is needed because it's what we'll use to fill the table with.

Well... they do represent values, just not values that exist on webassembly module. They are host values. They are for example javascript strings, or numbers, or objects of any kind. They are just opaque in the webassembly module but if retrieved by the javascript side they are just normal sized objects.


================
Comment at: clang/test/Sema/builtins-wasm.c:32
+  __builtin_wasm_table_fill(table, table, ref, nelem);           // expected-error {{2nd argument must be an integer}}
+  __builtin_wasm_table_fill(table, index, index, ref);           // expected-error {{3rd argument must match the element type of the WebAssembly table in the 1st argument}}
+  __builtin_wasm_table_fill(table, index, ref, table);           // expected-error {{4th argument must be an integer}}
----------------
aaron.ballman wrote:
> Similar question here about the third argument.
Exactly the same story as above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139010