[libcxx-commits] [PATCH] D103558: Remove VLA from libcxx locale header
Daniel McIntosh via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Fri Jun 4 10:06:11 PDT 2021
DanielMcIntosh-IBM added inline comments.
================
Comment at: libcxx/include/locale:1463
// printed as an unsigned value.
- const unsigned __nbuf = (numeric_limits<unsigned long>::digits / 3) // 1 char per 3 bits
- + ((numeric_limits<unsigned long>::digits % 3) != 0) // round up
- + ((__iob.flags() & ios_base::showbase) != 0) // base prefix
- + 1; // terminating null character
+ _LIBCPP_CONSTEXPR const unsigned __nbuf
+ = (numeric_limits<unsigned long>::digits / 3) // 1 char per 3 bits
----------------
Quuxplusone wrote:
> I wonder if this should be `_LIBCPP_CONSTEXPR const` (which seems redundant) or merely `static const` (for C++03 compatibility) or merely `const`. But I guess as long as it compiles it's fine any way.
`const` and `static const` by themselves wont force `__nbuf` to be a compile-time constant. If `__nbuf` is not [[ https://en.cppreference.com/w/cpp/language/constant_initialization | constant-initialized ]], then the `__nbuf` expression used to set the size of `__nar` will silently stop being an [[ https://en.cppreference.com/w/cpp/language/constant_expression | integral constant expression ]] (it will stop satisfying exception a to rule 9 of what isn't a core constant expression). This is why we were able to use `__iob.flags()` in the initializer without generating any errors.
`constexpr` by itself would be enough, but can't be used prior to C++11, so we need to use `_LIBCPP_CONSTEXPR`. However, we still can't remove the `const` because pre-c++11, that would leave `__nbuf` with neither `const` nor `constexpr`, which would again stop `__nbuf` from satisfying exception a to rule 9.
While the `_LIBCPP_CONSTEXPR` isn't strictly necessary, it prevents future issues like this one by generating an error if `__nbuf` isn't a compile-time constant.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D103558/new/
https://reviews.llvm.org/D103558
More information about the libcxx-commits
mailing list