[libcxx-commits] [PATCH] D70631: Microsoft's floating-point to_chars powered by Ryu and Ryu Printf

[Afanasyev Ivan via Phabricator via libcxx-commits]

ivafanas added inline comments.


================
Comment at: libcxx/include/charconv:733
+
+    if constexpr (_IsSame<_Floating, float>::value) {
+        _Adjusted_mantissa = _Ieee_mantissa << 1; // align to hexit boundary (23 isn't divisible by 4)
----------------
ldionne wrote:
> ivafanas wrote:
> > STL_MSFT wrote:
> > > ivafanas wrote:
> > > > As I remember `if constexpr` is available since C++17:
> > > > https://en.cppreference.com/w/cpp/language/if
> > > > 
> > > > And `charconv` header was made usable with C++11 in libc++:
> > > > https://reviews.llvm.org/D59598
> > > > 
> > > > May be it is a good idea to save header compatibility with C++11.
> > > MSVC and Clang actually support `if constexpr` in earlier language versions (I am not sure about C++11 since MSVC supports only C++14), with a warning that can be suppressed.
> > > 
> > > I don't understand why someone would want to use an old language mode with a new library mode, but this shouldn't be a blocking issue for that.
> > Hi,
> > 
> > > MSVC and Clang actually support if constexpr in earlier language versions
> > 
> > [[ http://libcxx.llvm.org/docs/ | Official documentation says ]] that GCC 5.0+ is supported but @EricWF noted [[ https://reviews.llvm.org/D63296#1542570 | here ]] about GCC 5.1+. According to cppreference `if constexpr` is supported since GCC 7+:
> > https://en.cppreference.com/w/cpp/compiler_support
> > 
> > If things haven't changed too much there might be GCC 5.* or GCC 6.* with C++11 but without `if constexpr`.
> > 
> > > I don't understand why someone would want to use an old language mode with a new library mode
> > 
> > There is an explanation why libc++ needs `charconv` backport for C++11:
> > https://reviews.llvm.org/D59598
> > 
> > > this shouldn't be a blocking issue for that
> > 
> > I'm not an expert in libc++ and do not have strong opinion.
> > Suppose that libc++ developers should have deep knowledge on that.
> @ivafanas 
> 
> I don't think it's reasonable or useful to backport this huge patch to C++11. We'll just disable the feature pre C++17.
Understood.
Thank you!


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

https://reviews.llvm.org/D70631