pmatos marked 2 inline comments as done.
pmatos added inline comments.
================
Comment at: clang/include/clang/Basic/BuiltinsWebAssembly.def:205-210
+TARGET_BUILTIN(__builtin_wasm_table_set, "viii", "t", "reference-types")
+TARGET_BUILTIN(__builtin_wasm_table_get, "iii", "t", "reference-types")
+TARGET_BUILTIN(__builtin_wasm_table_size, "ii", "nt", "reference-types")
+TARGET_BUILTIN(__builtin_wasm_table_grow, "iiii", "nt", "reference-types")
+TARGET_BUILTIN(__builtin_wasm_table_fill, "viiii", "t", "reference-types")
+TARGET_BUILTIN(__builtin_wasm_table_copy, "viiiii", "t", "reference-types")
----------------
aaron.ballman wrote:
> All of these should be documented in `docs/LanguageExtensions.rst` (you can
> make a Web Assembly section if one doesn't already exist; we've historically
> been bad about documenting our builtins).
Working on this - thanks for the reminder.
================
Comment at: clang/test/SemaCXX/wasm-refs-and-tables.cpp:6-7
+
+// Using c++11 to test dynamic exception specifications (which are not
+// allowed in c++17).
+
----------------
aaron.ballman wrote:
> Much of this file is the same test coverage as in the C case; I'd recommend
> combining the .c and .cpp files into one test with two RUN lines, and use
> `-verify=whatever` to distinguish between C vs C++ vs both diagnostic
> behaviors. The C++ specific code can then be split into a much smaller
> .cpp-specific file.
I have don't this know. I was not aware we could pass an argument to verify -
thanks.
================
Comment at: clang/test/SemaCXX/wasm-refs-and-tables.cpp:14
+
+__externref_t *t1; // expected-error {{pointer to WebAssembly
reference type is not allowed}}
+__externref_t **t2; // expected-error {{pointer to WebAssembly
reference type is not allowed}}
----------------
aaron.ballman wrote:
> Anywhere you'd testing pointer behavior for C++, you should have a test with
> an lvalue reference as well. I presume those will behave the same as
> pointers? It'd probably be wise to have tests for rvalue references as well.
Here I guess you're talking about testing __externref_t &. However, this might
be outside the scope of the patch which deals only with tables.
I could add further lvalue and rvalue reference tests to externref and funcref
in another patch. What do you think?
================
Comment at: clang/test/SemaCXX/wasm-refs-and-tables.cpp:16-17
+__externref_t **t2; // expected-error {{pointer to WebAssembly
reference type is not allowed}}
+__externref_t ******t3; // expected-error {{pointer to WebAssembly
reference type is not allowed}}
+static __externref_t t4[3]; // expected-error {{only zero-length
WebAssembly tables are currently supported}}
+static __externref_t t5[]; // expected-error {{only zero-length
WebAssembly tables are currently supported}}
----------------
aaron.ballman wrote:
> pmatos wrote:
> > aaron.ballman wrote:
> > > This seems really... confused. We can't form a pointer to the type, but
> > > we can form an array of the type (which decays into a pointer when you
> > > sneeze near it, and it's not clear whether that should be allowed or not)
> > > so long as it's a zero-length array (which is an extension in C and C++,
> > > so do we need to silence pedantic warnings in WASM for this?).
> > As it stands, tables are declared as static arrays of size zero. Then to
> > access them we need to use builtins. No subscripting, no comparison, no
> > pointer decay, etc. No passing into functions, returning from functions,
> > etc. Nothing besides using them as arguments to wasm_table... builtins.
> Okay, that's less confused now, thank you! What should the `-pedantic`
> behavior be for this:
> ```
> static __externref_t table[0];
> ```
> I presume you don't want the usual "zero size arrays are an extension"
> warning?
I have fixed this but unsure how to best merge it with the remainder of the
tests, so created a new test file.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D139010/new/
https://reviews.llvm.org/D139010
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
