Conversation
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
|
cc @rossberg |
| case EXTERN_GLOBAL: return global()->copy(); | ||
| case EXTERN_TABLE: return table()->copy(); | ||
| case EXTERN_MEMORY: return memory()->copy(); | ||
| default: assert(false); |
There was a problem hiding this comment.
Hm, wouldn't having a branch with no return trip up other compilers?
There was a problem hiding this comment.
Funnily enough, I've added all those asserts (with the exception of the one in src/wasm-v8.cc:2199, which I added for completeness) to silence the no return errors in GCC:
error: control reaches end of non-void function [-Werror=return-type]
so this works fine for both: clang and gcc, at least using default CFLAGS.
But now that you pointed this out, I realized that this will indeed break when building with -DNDEBUG (i.e. when assert() is omitted), but this is already the case in master, since the same pattern is used in a few other places, e.g. in src/wasm-bin.cc:220:
Line 220 in dc8cc29
Perhaps all those assert(false) should be replaced with UNIMPLEMENTED()? What do you think?
There was a problem hiding this comment.
Sounds good, though I don't think we want to abuse UNIMPLEMENTED, but define a similar UNREACHABLE. Do you mind including that with this PR?
| case EXTERN_GLOBAL: return global()->copy(); | ||
| case EXTERN_TABLE: return table()->copy(); | ||
| case EXTERN_MEMORY: return memory()->copy(); | ||
| default: assert(false); |
There was a problem hiding this comment.
Sounds good, though I don't think we want to abuse UNIMPLEMENTED, but define a similar UNREACHABLE. Do you mind including that with this PR?
2 participants
Signed-off-by: Piotr Sikora piotrsikora@google.com