[libcxx-commits] [PATCH] D158919: [libunwind][WebAssembly] Support Wasm EH

[Saleem Abdulrasool via Phabricator via libcxx-commits]

compnerd added a comment.

`__USING_SJLJ_EXCEPTIONS__` is compiler defined, it isn't a build system thing.  In general, we would like to be able to build the targets that we are claiming to support.



================
Comment at: libunwind/include/unwind_itanium.h:27
   uintptr_t private_[6];
-#else
+#elif !defined(__USING_WASM_EXCEPTIONS__)
   uintptr_t private_1; // non-zero means forced unwind
----------------
This is very confusing.  Does WASM use SEH?  This path is normally for Windows x86.


================
Comment at: libunwind/src/Unwind-wasm.c:1
+#include "config.h"
+#include "unwind.h"
----------------
Missing copyright header.


================
Comment at: libunwind/src/Unwind-wasm.c:44
+  // only do the cleanup phase.
+  _Unwind_Reason_Code ret = __gxx_personality_wasm0(
+      1, _UA_SEARCH_PHASE, exception_object->exception_class, exception_object,
----------------
Could just return the function rather than assign it to a variable.


================
Comment at: libunwind/src/Unwind-wasm.c:76
+  // is index 1 in the personality function.
+  if (index != 1)
+    return;
----------------
Why not use the positive form?

```
if (index == 1)
  ((struct _Unwind_landingPadContext *)context)->selector = value;
```


================
Comment at: libunwind/src/config.h:101
 #endif
+#elif defined(__wasm__)
 #else
----------------
It might be useful to provide warnings should those macros be used on WASM?  Right now the use of `_LIBUNWIND_WEAK_ALIAS` will fail with a weird preprocessor error.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158919