[PATCH] D50413: [libunwind][include] Add some missing definitions to <unwind.h>.

Kristina Brooks via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 30 16:17:31 PDT 2018


kristina added a comment.

Probably not, I'm personally against adding more and more typedefs for things that are just `stdint/inttypes.h` stuff and I say here the change is insignificant enough to warrant having to define even more types. It's not causing any problems but unless there are actual custom composite types that belong in that header, I don't see the benefit of them, if everyone starts wanting to typedef trivial types in `unwind.h`, it could be a maintainability issue. Not talking about this patch specifically but about a precedent of whether trivial types should get more and more aliases, polluting the header, and whether sticking to standard types and aliasing them in implementation is perhaps a better approach. Since the header gets installed everywhere and there isn't something like `unifdef` removing otherwise dead code. SEH patch was of much bigger significance so in that case it was fine (though I still like insisting on avoiding custom typedefs like `DWORD` for `uint32_t` just because rest of LLVM follows the same practice, while new composite types can be a necessity, more and more trivial type aliases isn't).

That's my stance on it. Patch itself is sound however, ignoring what I said above.


Repository:
  rUNW libunwind

https://reviews.llvm.org/D50413





More information about the llvm-commits mailing list