pmatos marked 3 inline comments as done.
pmatos added inline comments.
================
Comment at: clang/docs/LanguageExtensions.rst:2288
+argument is the index to which to store the value into, and the
+third argument is a value of reference type to store in the table.
+It returns nothing.
----------------
aaron.ballman wrote:
> pmatos wrote:
> > aaron.ballman wrote:
> > > This sounds like any reference type will work, e.g.,
> > > ```
> > > static __externref_t table[0];
> > > void func(int i) {
> > > int &ref = i;
> > > __builtin_wasm_table_set(table, i, ref);
> > > }
> > > ```
> > > so might want to say "value of ``_externref_t`` type" instead?
> > Any reference type will work, i.e. externref or funcref. You can also store
> > funcrefs in a wasm table. But of course, it needs to be well-typed. You
> > cannot store a funcref value in an externref table and vice-versa.
> My point is more that "any reference type" suggests you can use an arbitrary
> C++ reference type like `int &` and I think we mean something more specific
> than that, right?
Yes, of course, I always mean a WebAssembly reference type in this context.
================
Comment at: clang/docs/LanguageExtensions.rst:2364-2365
+range, the third argument is the value to set in the new entries, and
+the fourth and the last argument is the size of the range. It returns
+nothing.
+
----------------
aaron.ballman wrote:
> pmatos wrote:
> > aaron.ballman wrote:
> > > What happens if the range is invalid for the table size? e.g., the user
> > > never called `__builtin_wasm_table_grow` before calling
> > > `__builtin_wasm_table_fill`?
> > The host executing the WebAssembly instance will trap. This is part of the
> > WebAssembly spec.
> Hmmm, do you think we should mention that here? Alternatively, should we have
> a link to other WebAssembly documentation so we can say "see <blah> for more
> details?" to avoid replicating docs too much?
Good idea. I will handle that.
================
Comment at: clang/test/SemaCXX/wasm-refs-and-tables.cpp:35
+task<__externref_t[]> g() {
+ co_return table;
+}
----------------
aaron.ballman wrote:
> pmatos wrote:
> > aaron.ballman wrote:
> > > pmatos wrote:
> > > > @aaron.ballman I tried and failed to create a good testcase for
> > > > co_return. However creating coroutines seems to be an stdlib thing
> > > > which I am not sure how to test here. Do you have any suggestions here?
> > > ```
> > > #include "Inputs/std-coroutine.h"
> > >
> > > using std::suspend_always;
> > >
> > > struct promise_table {
> > > __externref_t[] get_return_object();
> > > suspend_always initial_suspend();
> > > suspend_always final_suspend() noexcept;
> > > void return_value(__externref_t[]);
> > > void unhandled_exception();
> > > };
> > >
> > > template <typename... T>
> > > struct std::coroutine_traits<__externref_t[], T...> { using promise_type
> > > = promise_table; };
> > >
> > > static __externref_t table[0];
> > > __externref_t[] func() {
> > > co_return table; // Cannot return a WebAssembly table?
> > > }
> > > ```
> > > Perhaps something along these lines?
> > But you cannot write promise_table because a table cannot be an argument in
> > return_value. Also, you cannot return a table in get_return_object. Doesn't
> > this invalidate straight away the use of a table with co-routines?
> Ah! Yeah, I think that does. Okay, let's punt on coroutines, that's turning
> out to be a slog.
OK - thanks.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D139010/new/
https://reviews.llvm.org/D139010
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
