[libcxx-commits] [PATCH] D100504: [libc++abi][AIX] initial patch for EH on AIX

[Chris Bowler via Phabricator via libcxx-commits]

cebowleratibm added a comment.

Early comments.  Continuing to work through the review.



================
Comment at: libcxxabi/src/cxa_personality.cpp:1295
+
+// Code for the personality of the state table.
+
----------------
Because the "state table" we're referring to is an IBM proprietary implementation there ought to be an abstract description of what it is and which compilers generate it.


================
Comment at: libcxxabi/src/cxa_personality.cpp:1326
+
+typedef void (*destruct_f)(void*);
+
----------------
suggest namespacing all of the types that pertain to the "state table" implementation.


================
Comment at: libcxxabi/src/cxa_personality.cpp:1335
+  };
+  intptr_t count;
+  size_t elemSize;
----------------
I suggest making this:

```
union {
  FSMEntryCount flag;
  intptr_t count;
}

```
which makes overlapping use of the memory more clear.


================
Comment at: libcxxabi/src/cxa_personality.cpp:1337
+  size_t elemSize;
+  uint16_t flags;
+  uint16_t nextState;
----------------
should the flags be of type FSMEntryFlag?  Noting FSMEntryFlag has type int16_t


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100504