[libcxx-commits] [PATCH] D112553: [libc++] Make is_error_condition_enum_v and is_error_code_enum_v bool, not size_t

[Joe Loser via Phabricator via libcxx-commits]

jloser added a comment.

In D112553#3087946 <https://reviews.llvm.org/D112553#3087946>, @Quuxplusone wrote:

> Arguably this should add a regression test. Certainly seems straightforward enough, though, right?
> As usual I'm a little unsure of what the ABI implications are (notice `is_error_code_enum_v` is not `_LIBCPP_HIDE_FROM_ABI`), but this is also clearly a bugfix and I assume we'd bite the bullet if there were one.

I was on the fence given how "trivially obvious" the change is, but looks like we did get it wrong at one point...so, I added a simple test for both variable templates.

I agree regarding the ABI break this technically is, but, like you said, it's a bugfix, so it is what it is. I recall in one of my recent PRs we talked about symbol visibility wrt variable templates, but don't think any of us have an immediate planned follow-up. Louis and I have talked offline briefly about some ideas of testing for symbol visibility before we go change the library as a one in one sweeping fashion.

@ldionne - should we make these variable templates as `_LIBCPP_HIDE_FROM_ABI` while we're here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112553