Page MenuHomePhabricator

__builtin_strlen is not supported for MSVC e.g and it is not correct to use it without knowledge of this "builtin" in advance
AbandonedPublic

Authored by danlark on Feb 10 2019, 8:34 PM.

Details

Summary

There is no analogue of constexpr __builtin_strlen for MSVC, so make it through defines as it was done for char_traits<wchar_t>

Diff Detail

Event Timeline

danlark created this revision.Feb 10 2019, 8:34 PM

What version of MSVC are you trying to use?

Last I checked we don't support MSVC.

__string
268 ↗(On Diff #186180)

This doesn't work for GCC.

What version of MSVC are you trying to use?

Last I checked we don't support MSVC.

We are using Microsoft Visual C++ 2017 14.16.27012 -- almost the last version

Marshall asked us to send all the patches that we did to make libcxx work for Windows

EricWF added inline comments.Feb 10 2019, 9:09 PM
__string
269 ↗(On Diff #186180)

How about

#ifndef _LIBCPP_COMPILER_MSVC
return __builtin_strlen(__s);
#else
...
#endif
danlark updated this revision to Diff 186182.Feb 10 2019, 9:13 PM
danlark marked 2 inline comments as done.
danlark marked an inline comment as done.Feb 10 2019, 9:18 PM
danlark added inline comments.
__string
269 ↗(On Diff #186180)

done

danlark updated this revision to Diff 186183.Feb 10 2019, 9:32 PM
EricWF added inline comments.Feb 10 2019, 10:37 PM
include/__string
216

Please keep the declaration the same.

EricWF requested changes to this revision.Feb 10 2019, 10:40 PM

MSVC on godbolt provides a constexpr __builtin_strlen. https://godbolt.org/z/n_3vRY

This revision now requires changes to proceed.Feb 10 2019, 10:40 PM

MSVC on godbolt provides a constexpr __builtin_strlen. https://godbolt.org/z/n_3vRY

I believe it is supported from C++17 --
https://godbolt.org/z/8xsDxu -- C++17
https://godbolt.org/z/SNUPsj -- C++14

danlark abandoned this revision.May 28 2020, 7:08 AM