[PATCH] D130689: [LLVM] Update C++ standard to 17

Mon Aug 1 01:06:58 PDT 2022

nikic added a comment.

In D130689#3690258 <https://reviews.llvm.org/D130689#3690258>, @thieta wrote:

> In D130689#3689157 <https://reviews.llvm.org/D130689#3689157>, @thakis wrote:
>
>> Is it expected and intentional that this increases the mac deployment target to 10.12?
>
> I wasn't aware of that - but I think it's expected since the check in RWMutex checks for the C++ standard and doesn't care about the deployment target for macOS. It seems pretty easy to change, but I wonder if that matters? 10.12 was released in 2016 so it's pretty old and this wouldn't affect most users of LLVM.
>
> My gut feeling say that we should be fine with requiring 10.12 and if that becomes a big problem during the development phase someone could propose a patch to improve the check in RWMutex.
>
> But in that case we should probably have a check for the deployment target as part of the cmake config and error if CXX_STANDARD > 17 and OSX_DEPLOYMENT_TARGET < 10.12.

Given https://github.com/llvm/llvm-project/blob/2bb7c54621f31a957302a4deb3d25b752acb07bd/llvm/include/llvm/Support/RWMutex.h#L22-L27, it seems like this is supposed to be supported. This is probably just a matter of moving the shared_mutex use behind the LLVM_USE_RW_MUTEX_IMPL guard?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689