[libcxx-commits] [PATCH] D110647: [SystemZ][z/OS] Resolve missing table_size symbol

Arthur O'Dwyer via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Wed Oct 13 14:37:15 PDT 2021


Quuxplusone added a comment.

Adding my own distracting comment. ;) I figure @ldionne is the only one who can really give this the green light.



================
Comment at: libcxx/include/__locale:712-718
     static locale::id id;
 
 #ifdef _CACHED_RUNES
-    static const size_t table_size = _CACHED_RUNES;
+    static _LIBCPP_CONSTEXPR const size_t table_size = _CACHED_RUNES;
 #else
-    static const size_t table_size = 256;  // FIXME: Don't hardcode this.
+    static _LIBCPP_CONSTEXPR const size_t table_size = 256;  // FIXME: Don't hardcode this.
 #endif
----------------
Peanut gallery says: This does fix the linker problem IIUC, but wouldn't it be more consistent to put an out-of-line definition in `libcxx/src/locale.cpp`? We do that for
```
locale::id ctype<char>::id;
```
(cf. line 712 above), and also for
```
const ctype_base::mask ctype_base::space;
const ctype_base::mask ctype_base::print;
const ctype_base::mask ctype_base::cntrl;
~~~
```
Vice versa, if we're finally able to use `constexpr` for this stuff so that it doesn't need to be in `locale.cpp`, then maybe can we use `constexpr` consistently for all of the above stuff too? or we can't do that because ABI?


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

https://reviews.llvm.org/D110647



More information about the libcxx-commits mailing list