[PATCH] D76997: Fix StringRef::strLen in windows with clang++ C++17

[Isuru Fernando via Phabricator via llvm-commits]

isuruf marked an inline comment as done.
isuruf added inline comments.


================
Comment at: llvm/include/llvm/ADT/StringRef.h:84
       return __builtin_strlen(Str);
+#elif __cplusplus > 201402L && !defined(_MSC_VER)
+      return std::char_traits<char>::length(Str);
----------------
isuruf wrote:
> rnk wrote:
> > aaron.ballman wrote:
> > > I still think the addition of `!defined(_MSC_VER)` is incorrect and that LLVM's CMake needs to be updated to pass `/Zc:__cplusplus` to MSVC.
> > > 
> > > However, I think there's a different change to make possibly. We require MSVC 2017 with the latest updates installed (https://llvm.org/docs/GettingStartedVS.html#software), but `_MSC_VER == 1916` is 2017 Update 9 (Visual Studio 2017 version 15.9.11). I currently have Visual Studio 2017 version 15.9.22 installed, so it seems like the version check for the builtin branch can turn into `defined(_MSC_VER)` and the changes to the STL branch can be removed.
> > Oh, I thought that would eliminate the need for this ifdef check. Do we actually support < 1916 MSVC versions? I guess we do, so this is necessary. :( Sorry that makes my original suggestion less helpful.
> I'm using `_MSC_VER == 1911` and LLVM builds fine with that version. It doesn't have the correct constexpr value. Hence this differential.
I'm using a cross-compiling setup with clang-cl and MSVC headers and libraries. 1911 was the last version that Microsoft published as a package that I can use on Linux easily.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76997