[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

Tue Sep 20 00:03:33 PDT 2022

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

================
Comment at: clang/lib/AST/ASTContext.cpp:2258
+    Width = 0;                                                                 \
+    Align = 8; /* ? */                                                         \
+    break;
----------------
pmatos wrote:
> tlively wrote:
> > I assume things will break if you say 0 here, but would 1 work?
> Yes, 1 seems to work. I am not 100% sure but since externref cannot be written to memory, does it matter?
Nope... I take that back. `builtins-wasm.c` seems to break with anything under 8 with the error: 

```
clang: /home/pmatos/dev/llvm-project/llvm/include/llvm/Support/Alignment.h:77: llvm::Align::Align(uint64_t): Assertion `Value > 0 && "Value must not be 0"' failed.
```

Still need to investigate why a value of `Align = 1;` here ends up causing a zero alignment value in `Alignment.h`.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122215