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

[Tobias Hieta via Phabricator via cfe-commits]

thieta added inline comments.



Comment at: llvm/docs/ReleaseNotes.rst:57
+
+* GCC >= 7.1
+* Clang >= 5.0

xbolva00 wrote:
> Do we have bots which uses last supported compiler versions? GCC 7.1, Clang 
> 5.0 and MSVC 16.7.
> 
> It is bad to promise something and then breakages here and there, for 
> example: https://github.com/llvm/llvm-project/issues/57057 so probably no 
> such bots.
I am not sure. I think it would be good if you posted that to discourse so that 
bot owners can reply to that. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added inline comments.



Comment at: llvm/docs/ReleaseNotes.rst:57
+
+* GCC >= 7.1
+* Clang >= 5.0

Do we have bots which uses last supported compiler versions? GCC 7.1, Clang 5.0 
and MSVC 16.7.

It is bad to promise something and then breakages here and there, for example: 
https://github.com/llvm/llvm-project/issues/57057 so probably no such bots.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

Hi - I tried to incorporate all the feedback here and added a post to discourse 
suggesting tweaks to the developer policy - please have a look and review it: 
https://discourse.llvm.org/t/rfc-updates-to-developer-policy-around-c-standards-bump/64383


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Re rollout: I suggested further up to put this behind an opt-in variable. That 
would've allowed people to test this on their setups. I still think that's a 
good suggestion, and it would've identified the MSVC issue at least.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

I think a week or two of moratorium would be good for sure. I think we can 
table this discussion for now and I will write a RFC post in discourse when I 
have time and we can discuss the details there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D130689#3710340 , @MaskRay wrote:

> In D130689#3710291 , @aaron.ballman 
> wrote:
>
>> In D130689#3710281 , @royjacobson 
>> wrote:
>>
>>> In D130689#3709834 , @thieta 
>>> wrote:
>>>
 In D130689#3709742 , 
 @aaron.ballman wrote:

> One thing I think would be a definite improvement is to have done an RFC 
> on Discourse for these changes so that downstreams have a chance to weigh 
> in on the impact. The patch was put up on Jul 28 and landed about a week 
> later without any notification to the rest of the community who might not 
> be watching cfe-commits -- that's a very fast turnaround and very little 
> notification for such a significant change.

 Yeah this is on me. Honestly I didn't expect it to be that much of a 
 problem but rather the toolchain requirement we posted as part of it would 
 be the big hurdle where bot owners would have to upgrade to get the right 
 versions. But lesson learned  and we should add some more delays in the 
 policy here: https://llvm.org/docs/DeveloperPolicy.html#id23 and cover the 
 C++ standards upgrade.
>>>
>>> Two points I want to add that I think would've been useful as well -
>>>
>>> 1. In addition to the toolchain soft errors, add a version check + #warning 
>>> to some llvm header. This would be useful as it is more visible than the 
>>> CMake warning and it could show up for cases where LLVM is used as a 
>>> library+headers and not built from sources.
>>> 2. Delay actual usage of new language features until after the next 
>>> release. Currently I see people pushing lots of cleanup commits that could 
>>> hurt bug backports. It also has the benefit of making the transition more 
>>> gradual.
>>
>> Strong +1 to point #2 especially. This is something we could have plausibly 
>> reverted to work through the kinks rather than doing that work live and 
>> while under duress, but it became implausible pretty quickly because 
>> everyone started landing their C++17 NFC changes. Those kinds of changes 
>> almost always can wait until after we've validated that the switch has gone 
>> smoothly.
>
> Point #2 can be advised but may not have too much a difference. I work on a 
> large monolithic code base and have good experience that people quickly use 
> new features (sometimes inadvertently) with a new release of clang/mlir/etc 
> or use/stick with an unsupported use for an extended period of time. It's 
> very difficult to prevent either activity.

Agreed that people will start using those features organically, but it's way 
easier to revert 1-5 patches than 20-30. I'm not worried about when we need to 
revert a few weeks after landing the switch, I'm worried about situations like 
this where we knew within a few days that we had serious problems. This landed 
on Saturday and we had people pushing c++17 specific NFC changes that same day.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D130689#3710291 , @aaron.ballman 
wrote:

> In D130689#3710281 , @royjacobson 
> wrote:
>
>> In D130689#3709834 , @thieta wrote:
>>
>>> In D130689#3709742 , 
>>> @aaron.ballman wrote:
>>>
 One thing I think would be a definite improvement is to have done an RFC 
 on Discourse for these changes so that downstreams have a chance to weigh 
 in on the impact. The patch was put up on Jul 28 and landed about a week 
 later without any notification to the rest of the community who might not 
 be watching cfe-commits -- that's a very fast turnaround and very little 
 notification for such a significant change.
>>>
>>> Yeah this is on me. Honestly I didn't expect it to be that much of a 
>>> problem but rather the toolchain requirement we posted as part of it would 
>>> be the big hurdle where bot owners would have to upgrade to get the right 
>>> versions. But lesson learned  and we should add some more delays in the 
>>> policy here: https://llvm.org/docs/DeveloperPolicy.html#id23 and cover the 
>>> C++ standards upgrade.
>>
>> Two points I want to add that I think would've been useful as well -
>>
>> 1. In addition to the toolchain soft errors, add a version check + #warning 
>> to some llvm header. This would be useful as it is more visible than the 
>> CMake warning and it could show up for cases where LLVM is used as a 
>> library+headers and not built from sources.
>> 2. Delay actual usage of new language features until after the next release. 
>> Currently I see people pushing lots of cleanup commits that could hurt bug 
>> backports. It also has the benefit of making the transition more gradual.
>
> Strong +1 to point #2 especially. This is something we could have plausibly 
> reverted to work through the kinks rather than doing that work live and while 
> under duress, but it became implausible pretty quickly because everyone 
> started landing their C++17 NFC changes. Those kinds of changes almost always 
> can wait until after we've validated that the switch has gone smoothly.

Point #2 can be advised but may not have too much a difference. I work on a 
large monolithic code base and have good experience that people quickly use new 
features (sometimes inadvertently) with a new release of clang/mlir/etc or 
use/stick with an unsupported use for an extended period of time. It's very 
difficult to prevent either activity.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D130689#3710281 , @royjacobson 
wrote:

> In D130689#3709834 , @thieta wrote:
>
>> In D130689#3709742 , 
>> @aaron.ballman wrote:
>>
>>> One thing I think would be a definite improvement is to have done an RFC on 
>>> Discourse for these changes so that downstreams have a chance to weigh in 
>>> on the impact. The patch was put up on Jul 28 and landed about a week later 
>>> without any notification to the rest of the community who might not be 
>>> watching cfe-commits -- that's a very fast turnaround and very little 
>>> notification for such a significant change.
>>
>> Yeah this is on me. Honestly I didn't expect it to be that much of a problem 
>> but rather the toolchain requirement we posted as part of it would be the 
>> big hurdle where bot owners would have to upgrade to get the right versions. 
>> But lesson learned  and we should add some more delays in the policy here: 
>> https://llvm.org/docs/DeveloperPolicy.html#id23 and cover the C++ standards 
>> upgrade.
>
> Two points I want to add that I think would've been useful as well -
>
> 1. In addition to the toolchain soft errors, add a version check + #warning 
> to some llvm header. This would be useful as it is more visible than the 
> CMake warning and it could show up for cases where LLVM is used as a 
> library+headers and not built from sources.
> 2. Delay actual usage of new language features until after the next release. 
> Currently I see people pushing lots of cleanup commits that could hurt bug 
> backports. It also has the benefit of making the transition more gradual.

Strong +1 to point #2 especially. This is something we could have plausibly 
reverted to work through the kinks rather than doing that work live and while 
under duress, but it became implausible pretty quickly because everyone started 
landing their C++17 NFC changes. Those kinds of changes almost always can wait 
until after we've validated that the switch has gone smoothly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson added a comment.

In D130689#3709834 , @thieta wrote:

> In D130689#3709742 , @aaron.ballman 
> wrote:
>
>> One thing I think would be a definite improvement is to have done an RFC on 
>> Discourse for these changes so that downstreams have a chance to weigh in on 
>> the impact. The patch was put up on Jul 28 and landed about a week later 
>> without any notification to the rest of the community who might not be 
>> watching cfe-commits -- that's a very fast turnaround and very little 
>> notification for such a significant change.
>
> Yeah this is on me. Honestly I didn't expect it to be that much of a problem 
> but rather the toolchain requirement we posted as part of it would be the big 
> hurdle where bot owners would have to upgrade to get the right versions. But 
> lesson learned  and we should add some more delays in the policy here: 
> https://llvm.org/docs/DeveloperPolicy.html#id23 and cover the C++ standards 
> upgrade.

Two points I want to add that I think would've been useful as well -

1. In addition to the toolchain soft errors, add a version check + #warning to 
some llvm header. This would be useful as it is more visible than the CMake 
warning and it could show up for cases where LLVM is used as a library+headers 
and not built from sources.
2. Delay actual usage of new language features until after the next release. 
Currently I see people pushing lots of cleanup commits that could hurt bug 
backports. It also has the benefit of making the transition more gradual.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

In D130689#3709742 , @aaron.ballman 
wrote:

> One thing I think would be a definite improvement is to have done an RFC on 
> Discourse for these changes so that downstreams have a chance to weigh in on 
> the impact. The patch was put up on Jul 28 and landed about a week later 
> without any notification to the rest of the community who might not be 
> watching cfe-commits -- that's a very fast turnaround and very little 
> notification for such a significant change.

Yeah this is on me. Honestly I didn't expect it to be that much of a problem 
but rather the toolchain requirement we posted as part of it would be the big 
hurdle where bot owners would have to upgrade to get the right versions. But 
lesson learned  and we should add some more delays in the policy here: 
https://llvm.org/docs/DeveloperPolicy.html#id23 and cover the C++ standards 
upgrade.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D130689#3706424 , @aaron.ballman 
wrote:

> In D130689#3706377 , @thieta wrote:
>
>> In D130689#3706336 , 
>> @aaron.ballman wrote:
>>
>>> That's the only reason this hasn't been reverted already. Landing sweeping 
>>> changes on a weekend is a good way to reduce the pain, but we really need 
>>> to be sure someone watches the build lab and reacts when subsequent changes 
>>> break everything like this.
>>
>> Agreed, I think we need to update the protocol for changing the C++ standard 
>> in the future to account for more testing beforehand. I might push some 
>> changes to the policy document when all this has settled down to see if we 
>> can make sure it will be smoother the time we move to C++20. It's 
>> unfortunate that some stuff broke considering we where running some bots 
>> before it was merged and it didn't show any errors. And local windows builds 
>> for me have been clean as well.
>
> +1, thank you for thinking about how we can improve this process in the 
> future! Given that C++17 adoption across compilers has been far better than 
> C++20, I suspect the next time we bump the language version will be even more 
> of a challenge with these sort of weird issues.

One thing I think would be a definite improvement is to have done an RFC on 
Discourse for these changes so that downstreams have a chance to weigh in on 
the impact. The patch was put up on Jul 28 and landed about a week later 
without any notification to the rest of the community who might not be watching 
cfe-commits -- that's a very fast turnaround and very little notification for 
such a significant change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Tobias Hieta via Phabricator via cfe-commits]

thieta added inline comments.



Comment at: llvm/cmake/modules/CheckCompilerVersion.cmake:16
 # _MSC_VER == 1927 MSVC++ 14.27 Visual Studio 2019 Version 16.7
-set(MSVC_MIN 19.20)
+set(MSVC_MIN 19.27)
 set(MSVC_SOFT_ERROR 19.27)

glandium wrote:
> thieta wrote:
> > glandium wrote:
> > > You didn't update llvm/cmake/platforms/WinMsvc.cmake accordingly
> > AHA! That explains the fms-compatibility issue. Will you push a NFC commit 
> > or do you want me to do that?
> I don't have push access.
fixed it here: 
https://github.com/llvm/llvm-project/commit/15eaefa5fe3608b03f1abefc31129efaf9eab88e


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Mike Hommey via Phabricator via cfe-commits]

glandium added inline comments.



Comment at: llvm/cmake/modules/CheckCompilerVersion.cmake:16
 # _MSC_VER == 1927 MSVC++ 14.27 Visual Studio 2019 Version 16.7
-set(MSVC_MIN 19.20)
+set(MSVC_MIN 19.27)
 set(MSVC_SOFT_ERROR 19.27)

thieta wrote:
> glandium wrote:
> > You didn't update llvm/cmake/platforms/WinMsvc.cmake accordingly
> AHA! That explains the fms-compatibility issue. Will you push a NFC commit or 
> do you want me to do that?
I don't have push access.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Tobias Hieta via Phabricator via cfe-commits]

thieta added inline comments.



Comment at: llvm/cmake/modules/CheckCompilerVersion.cmake:16
 # _MSC_VER == 1927 MSVC++ 14.27 Visual Studio 2019 Version 16.7
-set(MSVC_MIN 19.20)
+set(MSVC_MIN 19.27)
 set(MSVC_SOFT_ERROR 19.27)

glandium wrote:
> You didn't update llvm/cmake/platforms/WinMsvc.cmake accordingly
AHA! That explains the fms-compatibility issue. Will you push a NFC commit or 
do you want me to do that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Mike Hommey via Phabricator via cfe-commits]

glandium added inline comments.



Comment at: llvm/cmake/modules/CheckCompilerVersion.cmake:16
 # _MSC_VER == 1927 MSVC++ 14.27 Visual Studio 2019 Version 16.7
-set(MSVC_MIN 19.20)
+set(MSVC_MIN 19.27)
 set(MSVC_SOFT_ERROR 19.27)

You didn't update llvm/cmake/platforms/WinMsvc.cmake accordingly


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D130689#3700094 , @thakis wrote:

> In D130689#3696186 , @thieta wrote:
>
>> @nikic @thakis I fixed this issue in https://reviews.llvm.org/D131063 and it 
>> can be built with CXX_STANDARD=17 and OSX_DEPLOYMENT_TARGET=10.11.
>
> Thanks! We took this opportunity to bump our clang deployment target from 
> 10.7 to 10.12 (which makes a few other minor things easier too), so we don't 
> need this change any more. But it's a good change anyways :)

FWIW I ran into an issue when doing Universal macOS builds (by building for 
arm64 and x86_64 at the same time) where I now had to raise my deployment 
target to 10.14 - see https://github.com/llvm/llvm-project/issues/57017 for 
more details about that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini added a comment.

> There should be a better way than this. Comprehensive pre-merge testing of 
> all PRs etc.

We already have pre-commit tests on Phabricator on Windows and Linux, but 
that's not exhaustive and for many reasons I don't believe this is realistic in 
any way: we will always have some post-commit buildbots.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Trass3r via Phabricator via cfe-commits]

Trass3r added a comment.

In D130689#3707296 , @mehdi_amini 
wrote:

> land the change, wait for a couple of hours to ensure that all bots have 
> picked up the revision, then revert

There should be a better way than this. Comprehensive pre-merge testing of all 
PRs etc.
But I know that the build times are a problem here. Even with incremental 
builds and ccache most commits cause a rebuild of many files.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Re MSVC: For what it's worth, our Chromium windows bot (which builds trunk llvm 
and then chromium with that) is happy. It builds with MSVC 2019. Here's the 
build log (which includes cmake invocations): 
https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket/8806536120229097233/+/u/gclient_runhooks/stdout?format=raw

(Look for `DCMAKE_BUILD_TYPE=Release'`.)

The bot builds clang;lld;clang-tools-extra. It also builds compiler-rt, but 
using the runtimes build, so I think compiler-rt is built with the just-built 
clang.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini added a comment.

> Agreed, I think we need to update the protocol for changing the C++ standard 
> in the future to account for more testing beforehand.

The way I would do it is: wait for a Sunday so that the bots aren't loaded, 
land the change, wait for a couple of hours to ensure that all bots have picked 
up the revision, then revert and survey the results for the runs. Communicating 
ahead of time on this also so that downstream can pickup the revision for their 
own tests if they want.
This should provide a pretty good signal ahead of time of the "real" migration 
hopefully!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

In D130689#3706424 , @aaron.ballman 
wrote:

> +1, thank you for thinking about how we can improve this process in the 
> future! Given that C++17 adoption across compilers has been far better than 
> C++20, I suspect the next time we bump the language version will be even more 
> of a challenge with these sort of weird issues.

I'll happily ignore that for another 3 years 😅

> Yeah, this is rather deeply concerning to be honest. I triple-checked and the 
> only changes in my tree were yours to compiler-rt and when I looked at the 
> configure logs, they clearly showed `compiler-rt project is disabled` in the 
> logs. It is really weird that a change to compiler-rt's cmake would have any 
> impact on Clang's ability to build when compiler-rt is disabled. I think 
> that's worth some investigative work.

Hmmm - something is really funny, with my cache nuking change it seems like 
this bot is back to working again: 
https://lab.llvm.org/buildbot/#/builders/123/builds/12164

> It seems like we might need that, but it also seems like it would be good to 
> understand why compiler-rt is impacting the rest of Clang when it's disabled. 
> That said, the goal is to get the build lab back to green as quickly as 
> possible, so I think it may make sense to land sooner rather than later.

I'll hold off a bit on this - it seems like my commit above have fixed at least 
some of the issues. There are still some msvc bots failing - but they seem to 
be failing because they have a too old version of MSVC.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D130689#3706377 , @thieta wrote:

> In D130689#3706336 , @aaron.ballman 
> wrote:
>
>> That's the only reason this hasn't been reverted already. Landing sweeping 
>> changes on a weekend is a good way to reduce the pain, but we really need to 
>> be sure someone watches the build lab and reacts when subsequent changes 
>> break everything like this.
>
> Agreed, I think we need to update the protocol for changing the C++ standard 
> in the future to account for more testing beforehand. I might push some 
> changes to the policy document when all this has settled down to see if we 
> can make sure it will be smoother the time we move to C++20. It's unfortunate 
> that some stuff broke considering we where running some bots before it was 
> merged and it didn't show any errors. And local windows builds for me have 
> been clean as well.

+1, thank you for thinking about how we can improve this process in the future! 
Given that C++17 adoption across compilers has been far better than C++20, I 
suspect the next time we bump the language version will be even more of a 
challenge with these sort of weird issues.

>>> Can you try to locally rebuild with this patch 
>>> https://reviews.llvm.org/D131382 ?
>>
>> That improves things but the build still isn't clean:
>> ...
>> (FWIW, I don't know if any of the Windows builders in the lab are building 
>> with /WX)
>
> I am not sure either - but at least this removed the problem with the 
> std=c++17 not being passed around correctly.

Agreed, I can look into fixing up those warnings when I have a spare moment if 
nobody else gets to them first.

>>> I think all the runtime errors is because of that one above - basically we 
>>> don't set std=c++17 for any of the compiler-rt projects.
>>
>> I wasn't building compiler-rt, so no idea why this improved things for me. 
>> FWIW, he's my CMake config: `-DCMAKE_EXPORT_COMPILE_COMMANDS=ON 
>> -DLLVM_TARGETS_TO_BUILD="X86" -DLLVM_ENABLE_IDE=ON 
>> -DLLVM_ENABLE_PROJECTS="clang;clang-tools-extra" 
>> -DLLVM_PARALLEL_COMPILE_JOBS=112 -DLLVM_PARALLEL_LINK_JOBS=16`
>
> That is very odd. I would suspect that the problem was the compiler-rt not 
> getting the right flag. But it seems to influence something else as well?

Yeah, this is rather deeply concerning to be honest. I triple-checked and the 
only changes in my tree were yours to compiler-rt and when I looked at the 
configure logs, they clearly showed `compiler-rt project is disabled` in the 
logs. It is really weird that a change to compiler-rt's cmake would have any 
impact on Clang's ability to build when compiler-rt is disabled. I think that's 
worth some investigative work.

>>> I also think we should merge https://reviews.llvm.org/D131367 for now - we 
>>> can revert that later on if we think it adds to much complexity, since it 
>>> will delete the bad cache values automatcially.
>>
>> Seems reasonable to me.
>
> I landed this now - hopefully that fixes some of the issues seen in the bots 
> - but we might need https://reviews.llvm.org/D131382 as well before they go 
> green. I am not sure what the protocol is here, since @MaskRay suggested the 
> change maybe we should wait for him to land it, or should we get it in ASAP 
> to see if that fixes the bots?

It seems like we might need that, but it also seems like it would be good to 
understand why compiler-rt is impacting the rest of Clang when it's disabled. 
That said, the goal is to get the build lab back to green as quickly as 
possible, so I think it may make sense to land sooner rather than later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

In D130689#3706336 , @aaron.ballman 
wrote:

> That's the only reason this hasn't been reverted already. Landing sweeping 
> changes on a weekend is a good way to reduce the pain, but we really need to 
> be sure someone watches the build lab and reacts when subsequent changes 
> break everything like this.

Agreed, I think we need to update the protocol for changing the C++ standard in 
the future to account for more testing beforehand. I might push some changes to 
the policy document when all this has settled down to see if we can make sure 
it will be smoother the time we move to C++20. It's unfortunate that some stuff 
broke considering we where running some bots before it was merged and it didn't 
show any errors. And local windows builds for me have been clean as well.

>> Can you try to locally rebuild with this patch 
>> https://reviews.llvm.org/D131382 ?
>
> That improves things but the build still isn't clean:
> ...
> (FWIW, I don't know if any of the Windows builders in the lab are building 
> with /WX)

I am not sure either - but at least this removed the problem with the std=c++17 
not being passed around correctly.

>> I think all the runtime errors is because of that one above - basically we 
>> don't set std=c++17 for any of the compiler-rt projects.
>
> I wasn't building compiler-rt, so no idea why this improved things for me. 
> FWIW, he's my CMake config: `-DCMAKE_EXPORT_COMPILE_COMMANDS=ON 
> -DLLVM_TARGETS_TO_BUILD="X86" -DLLVM_ENABLE_IDE=ON 
> -DLLVM_ENABLE_PROJECTS="clang;clang-tools-extra" 
> -DLLVM_PARALLEL_COMPILE_JOBS=112 -DLLVM_PARALLEL_LINK_JOBS=16`

That is very odd. I would suspect that the problem was the compiler-rt not 
getting the right flag. But it seems to influence something else as well?

>> I also think we should merge https://reviews.llvm.org/D131367 for now - we 
>> can revert that later on if we think it adds to much complexity, since it 
>> will delete the bad cache values automatcially.
>
> Seems reasonable to me.

I landed this now - hopefully that fixes some of the issues seen in the bots - 
but we might need https://reviews.llvm.org/D131382 as well before they go 
green. I am not sure what the protocol is here, since @MaskRay suggested the 
change maybe we should wait for him to land it, or should we get it in ASAP to 
see if that fixes the bots?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D130689#3706303 , @thieta wrote:

> In D130689#3706263 , @aaron.ballman 
> wrote:
>
>> 
>
>
>
>> Something odd is going on here and we might want to consider a revert of 
>> this patch until we resolve it. When I do a git pull and cmake files change, 
>> Visual Studio's built-in CMake support automatically re-runs the configure 
>> step. This should be all that's necessary to switch the toolchain, but it 
>> isn't for some reason (today's build is also failing for me with C++17 
>> related issues after I did another pulldown this morning). I deleted my 
>> cache explicitly and regenerated CMake from scratch and am still getting the 
>> same build errors. The failures I am getting are the same as what's shown by 
>> the sanitizer bot for Windows: 
>> https://lab.llvm.org/buildbot/#/builders/127/builds/33980/steps/4/logs/stdio 
>> (I'm using VS 2019 16.11.17 FWIW).
>>
>> I hope we can resolve this quickly as basically no MSVC builds are green 
>> right now in the build lab.
>
> While we can revert this one - we also need to revert all changes that add 
> C++17 features at this point as well. That will be a lot of churn. Let's see 
> if we can figure out what's wrong first.

That's the only reason this hasn't been reverted already. Landing sweeping 
changes on a weekend is a good way to reduce the pain, but we really need to be 
sure someone watches the build lab and reacts when subsequent changes break 
everything like this.

> Can you try to locally rebuild with this patch 
> https://reviews.llvm.org/D131382 ?

That improves things but the build still isn't clean:

  Severity  CodeDescription Project FileLineSuppression 
State
  Warning   C4996   
'std::codecvt_utf8': warning STL4017: 
std::wbuffer_convert, std::wstring_convert, and the  header 
(containing std::codecvt_mode, std::codecvt_utf8, std::codecvt_utf16, and 
std::codecvt_utf8_utf16) are deprecated in C++17. (The std::codecvt class 
template is NOT deprecated.) The C++ Standard doesn't provide equivalent 
non-deprecated functionality; consider using MultiByteToWideChar() and 
WideCharToMultiByte() from  instead. You can define 
_SILENCE_CXX17_CODECVT_HEADER_DEPRECATION_WARNING or 
_SILENCE_ALL_CXX17_DEPRECATION_WARNINGS to acknowledge that you have received 
this warning.F:\source\llvm-project\llvm\out\build\x64-Debug\llvm
F:\source\llvm-project\third-party\benchmark\src\sysinfo.cc 429 
  Warning   C4996   
'std::wstring_convert,std::allocator>':
 warning STL4017: std::wbuffer_convert, std::wstring_convert, and the  
header (containing std::codecvt_mode, std::codecvt_utf8, std::codecvt_utf16, 
and std::codecvt_utf8_utf16) are deprecated in C++17. (The std::codecvt class 
template is NOT deprecated.) The C++ Standard doesn't provide equivalent 
non-deprecated functionality; consider using MultiByteToWideChar() and 
WideCharToMultiByte() from  instead. You can define 
_SILENCE_CXX17_CODECVT_HEADER_DEPRECATION_WARNING or 
_SILENCE_ALL_CXX17_DEPRECATION_WARNINGS to acknowledge that you have received 
this warning.F:\source\llvm-project\llvm\out\build\x64-Debug\llvm
F:\source\llvm-project\third-party\benchmark\src\sysinfo.cc 430 
  Warning   C4996   
'std::wstring_convert,std::allocator>::wstring_convert':
 warning STL4017: std::wbuffer_convert, std::wstring_convert, and the  
header (containing std::codecvt_mode, std::codecvt_utf8, std::codecvt_utf16, 
and std::codecvt_utf8_utf16) are deprecated in C++17. (The std::codecvt class 
template is NOT deprecated.) The C++ Standard doesn't provide equivalent 
non-deprecated functionality; consider using MultiByteToWideChar() and 
WideCharToMultiByte() from  instead. You can define 
_SILENCE_CXX17_CODECVT_HEADER_DEPRECATION_WARNING or 
_SILENCE_ALL_CXX17_DEPRECATION_WARNINGS to acknowledge that you have received 
this warning.   F:\source\llvm-project\llvm\out\build\x64-Debug\llvm
F:\source\llvm-project\third-party\benchmark\src\sysinfo.cc 430 
  Warning   C4996   
'std::wstring_convert,std::allocator>::to_bytes':
 warning STL4017: std::wbuffer_convert, std::wstring_convert, and the  
header (containing std::codecvt_mode, std::codecvt_utf8, std::codecvt_utf16, 
and std::codecvt_utf8_utf16) are deprecated in C++17. (The std::codecvt class 
template is NOT deprecated.) The C++ Standard doesn't provide equivalent 
non-deprecated functionality; consider using MultiByteToWideChar() and 
WideCharToMultiByte() from  instead. You can define 
_SILENCE_CXX17_CODECVT_HEADER_DEPRECATION_WARNING or 
_SILENCE_ALL_CXX17_DEPRECATION_WARNINGS to acknowledge that you have received 
this warning.  F:\source\llvm-project\llvm\out\build\x64-Debug\llvm
F:\source\llvm-project\third-party\benchmark\src\sysinfo.cc 432 
  Warning   C4996   'std::iterator': warning STL4015: The std::iterator class 
template (used as a base c

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

[James Henderson via Phabricator via cfe-commits]

jhenderson added a comment.

FWIW, I've got a Visual Studio configuration (admittedly using a direct Visual 
Studio generator, not ninja), and I don't have any issues - C++17 is being 
used. However, I currently only have LLD as an additional project enabled, and 
don't build compiler-rt.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

In D130689#3706263 , @aaron.ballman 
wrote:

> 



> Something odd is going on here and we might want to consider a revert of this 
> patch until we resolve it. When I do a git pull and cmake files change, 
> Visual Studio's built-in CMake support automatically re-runs the configure 
> step. This should be all that's necessary to switch the toolchain, but it 
> isn't for some reason (today's build is also failing for me with C++17 
> related issues after I did another pulldown this morning). I deleted my cache 
> explicitly and regenerated CMake from scratch and am still getting the same 
> build errors. The failures I am getting are the same as what's shown by the 
> sanitizer bot for Windows: 
> https://lab.llvm.org/buildbot/#/builders/127/builds/33980/steps/4/logs/stdio 
> (I'm using VS 2019 16.11.17 FWIW).
>
> I hope we can resolve this quickly as basically no MSVC builds are green 
> right now in the build lab.

While we can revert this one - we also need to revert all changes that add 
C++17 features at this point as well. That will be a lot of churn. Let's see if 
we can figure out what's wrong first.

Can you try to locally rebuild with this patch https://reviews.llvm.org/D131382 
?

I think all the runtime errors is because of that one above - basically we 
don't set std=c++17 for any of the compiler-rt projects.

I also think we should merge https://reviews.llvm.org/D131367 for now - we can 
revert that later on if we think it adds to much complexity, since it will 
delete the bad cache values automatcially.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D130689#3706276 , @barannikov88 
wrote:

> In D130689#3706263 , @aaron.ballman 
> wrote:
>
>> The failures I am getting are the same as what's shown by the sanitizer bot 
>> for Windows: 
>> https://lab.llvm.org/buildbot/#/builders/127/builds/33980/steps/4/logs/stdio 
>> (I'm using VS 2019 16.11.17 FWIW).
>
> The log says:
>
>> The contents of  are available only with C++17 or later.
>
> if that helps.

Yes, and in my build logs, even after an explicit reconfigure where I deleted 
the cache, I'm seeing `-std:c++14` being passed to cl.exe; so something about 
these changes is not working properly with Visual Studio.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sergei Barannikov via Phabricator via cfe-commits]

barannikov88 added a comment.

In D130689#3706263 , @aaron.ballman 
wrote:

> The failures I am getting are the same as what's shown by the sanitizer bot 
> for Windows: 
> https://lab.llvm.org/buildbot/#/builders/127/builds/33980/steps/4/logs/stdio 
> (I'm using VS 2019 16.11.17 FWIW).

The log says:

> The contents of  are available only with C++17 or later.

if that helps.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D130689#3706200 , @thieta wrote:

> In D130689#3706199 , @cor3ntin 
> wrote:
>
>> Trying to read the logs,, notably 
>> `C:\PROGRA~2\MIB055~1\2019\PROFES~1\VC\Tools\MSVC\1429~1.301\bin\Hostx64\x64\cl.exe
>>  `, it would seem that this particular bot is running a version much older 
>> than the current requirements  (Visual Studio 2019 16.7)
>> Either I'm reading that wrong or the CMake script does not check the msvc 
>> version?
>
> The compilers are definitely version checked here: 
> https://github.com/llvm/llvm-project/blob/main/llvm/cmake/modules/CheckCompilerVersion.cmake#L50
>
> But reading the cmake log it seems like it's using a cache already and it's 
> hard to say if it's using the same version of MSVC.

Something odd is going on here and we might want to consider a revert of this 
patch until we resolve it. When I do a git pull and cmake files change, Visual 
Studio's built-in CMake support automatically re-runs the configure step. This 
should be all that's necessary to switch the toolchain, but it isn't for some 
reason (today's build is also failing for me with C++17 related issues after I 
did another pulldown this morning). I deleted my cache explicitly and 
regenerated CMake from scratch and am still getting the same build errors. The 
failures I am getting are the same as what's shown by the sanitizer bot for 
Windows: 
https://lab.llvm.org/buildbot/#/builders/127/builds/33980/steps/4/logs/stdio 
(I'm using VS 2019 16.11.17 FWIW).

I hope we can resolve this quickly as basically no MSVC builds are green right 
now in the build lab.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

In D130689#3706199 , @cor3ntin wrote:

> Trying to read the logs,, notably 
> `C:\PROGRA~2\MIB055~1\2019\PROFES~1\VC\Tools\MSVC\1429~1.301\bin\Hostx64\x64\cl.exe
>  `, it would seem that this particular bot is running a version much older 
> than the current requirements  (Visual Studio 2019 16.7)
> Either I'm reading that wrong or the CMake script does not check the msvc 
> version?

The compilers are definitely version checked here: 
https://github.com/llvm/llvm-project/blob/main/llvm/cmake/modules/CheckCompilerVersion.cmake#L50

But reading the cmake log it seems like it's using a cache already and it's 
hard to say if it's using the same version of MSVC.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

In D130689#3706135 , @aaron.ballman 
wrote:

> In D130689#3705131 , @royjacobson 
> wrote:
>
>> This seems to have been more disruptive than expected, since an existing 
>> CMakeCache.txt can make LLVM compile in previous C++14 configuration. This 
>> seems to make some of the bots fail in a way that makes the patches making 
>> use of C++17 features seem at fault.
>>
>> See:
>> https://github.com/llvm/llvm-project/commit/ede96de751224487aea122af8bfb4e82bc54840b#commitcomment-80507826
>> https://reviews.llvm.org/rG32fd0b7fd5ab
>>
>> How would you feel about adding something like
>>
>>   #if defined(__cplusplus) && __cplusplus < 201703L
>>   #error "LLVM requires at least C++17"
>>   #endif
>>
>> to some central header, to make this switch more visible?
>
> FWIW, that revert was not necessarily due to cmake cache issues (though those 
> issues may exist, I haven't checked). I reverted that change because it broke 
> the build for me locally as well as caused some bots to go red. My local 
> build definitely rebuilt the cache and still failed.
>
> https://lab.llvm.org/buildbot/#/builders/127/builds/33955 (build with revert, 
> passing)
> https://lab.llvm.org/buildbot/#/builders/127/builds/33954 (previous build, 
> failing due to compile errors)

Trying to read the logs,, notably 
`C:\PROGRA~2\MIB055~1\2019\PROFES~1\VC\Tools\MSVC\1429~1.301\bin\Hostx64\x64\cl.exe
 `, it would seem that this particular bot is running a version much older than 
the current requirements  (Visual Studio 2019 16.7)
Either I'm reading that wrong or the CMake script does not check the msvc 
version?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D130689#3705131 , @royjacobson 
wrote:

> This seems to have been more disruptive than expected, since an existing 
> CMakeCache.txt can make LLVM compile in previous C++14 configuration. This 
> seems to make some of the bots fail in a way that makes the patches making 
> use of C++17 features seem at fault.
>
> See:
> https://github.com/llvm/llvm-project/commit/ede96de751224487aea122af8bfb4e82bc54840b#commitcomment-80507826
> https://reviews.llvm.org/rG32fd0b7fd5ab
>
> How would you feel about adding something like
>
>   #if defined(__cplusplus) && __cplusplus < 201703L
>   #error "LLVM requires at least C++17"
>   #endif
>
> to some central header, to make this switch more visible?

FWIW, that revert was not necessarily due to cmake cache issues (though those 
issues may exist, I haven't checked). I reverted that change because it broke 
the build for me locally as well as caused some bots to go red. My local build 
definitely rebuilt the cache and still failed.

https://lab.llvm.org/buildbot/#/builders/127/builds/33955 (build with revert, 
passing)
https://lab.llvm.org/buildbot/#/builders/127/builds/33954 (previous build, 
failing due to compile errors)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

In D130689#3705579 , @Jake-Egan wrote:

> There is a failure on the AIX bot also:
> ...
> https://lab.llvm.org/buildbot/#/builders/214/builds/2707/steps/5/logs/stdio

Filed an issue here: https://github.com/llvm/llvm-project/issues/56995


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

In D130689#3705474 , @dyung wrote:

> We are seeing an additional failure on an internal linux bot due to the 
> change to using C++17 by default when using GNU ld:
> ...
> Switching between BFD ld and gold still fails (although gold fails for a 
> slightly different reason). Superficially it seems that switching to C++17 
> for some reason might be causing the compiler to emit debug info that these 
> older non-lld linkers cannot understand for some reason?

Filed this issue here: https://github.com/llvm/llvm-project/issues/56994


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Jake Egan via Phabricator via cfe-commits]

Jake-Egan added a comment.

There is a failure on the AIX bot also:

  152.827 [4302/10/270] Linking CXX executable bin/llvm-tblgen
  FAILED: bin/llvm-tblgen 
  : && /opt/IBM/openxlC/17.1.0/bin/ibm-clang++_r  -mcmodel=large -fPIC -Werror 
-Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra 
-Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers 
-pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough 
-Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor 
-Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion 
-Wmisleading-indentation -fdiagnostics-color -ffunction-sections 
-fdata-sections -O3 -DNDEBUG -Wl,-bnoipath  
utils/TableGen/CMakeFiles/llvm-tblgen.dir/AsmMatcherEmitter.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/AsmWriterEmitter.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/AsmWriterInst.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/Attributes.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/CallingConvEmitter.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/CodeEmitterGen.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/CodeGenDAGPatterns.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/CodeGenHwModes.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/CodeGenInstruction.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/CodeGenMapTable.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/CodeGenRegisters.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/CodeGenSchedule.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/CodeGenTarget.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/DAGISelEmitter.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/DAGISelMatcherEmitter.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/DAGISelMatcherGen.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/DAGISelMatcherOpt.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/DAGISelMatcher.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/DecoderEmitter.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/DFAEmitter.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/DFAPacketizerEmitter.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/DirectiveEmitter.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/DisassemblerEmitter.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/DXILEmitter.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/ExegesisEmitter.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/FastISelEmitter.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/GICombinerEmitter.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/GlobalISelEmitter.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/InfoByHwMode.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/InstrInfoEmitter.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/InstrDocsEmitter.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/IntrinsicEmitter.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/OptEmitter.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/OptParserEmitter.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/OptRSTEmitter.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/PredicateExpander.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/PseudoLoweringEmitter.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/CompressInstEmitter.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/RegisterBankEmitter.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/RegisterInfoEmitter.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/SDNodeProperties.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/SearchableTableEmitter.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/SubtargetEmitter.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/SubtargetFeatureInfo.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/TableGen.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/Types.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/VarLenCodeEmitterGen.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/X86DisassemblerTables.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/X86EVEX2VEXTablesEmitter.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/X86FoldTablesEmitter.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/X86MnemonicTables.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/X86ModRMFilters.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/X86RecognizableInstr.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/WebAssemblyDisassemblerEmitter.cpp.o 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/CTagsEmitter.cpp.o  -o 
bin/llvm-tblgen  
-Wl,-blibpath:"\$ORIGIN/../lib:/opt/IBM/xlmass/10.1.0/lib:/usr/lib:/lib"  
lib/libLLVMSupport.a  lib/libLLVMTableGen.a  -lpthreads  
lib/libLLVMTableGenGlobalISel.a  lib/libLLVMTableGen.a  lib/libLLVMSupport.a  
-lrt  -lld  -lpthreads  -lm  /usr/lib/libcurses.a  lib/libLLVMDemangle.a && :
  ld: 0711-317 ERROR: Undefined symbol: ._ZdlPvSt11align_val_t
  ld: 0711-317 ERROR: Undefined symbol: ._ZnwmSt11align_val_t
  ld: 0711-345 Use the -bloadmap or -bnoquiet option to obtain more information.
  .orig: error: linker command failed with exit code 8 (use -v to see 
invocation)

https://lab.llvm.org/buildbot/#/builders/214/build

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

[Douglas Yung via Phabricator via cfe-commits]

dyung added a comment.

We are seeing an additional failure on an internal linux bot due to the change 
to using C++17 by default when using GNU ld:

  [3/7] Generating GwpAsan-x86_64-Test
  FAILED: projects/compiler-rt/lib/gwp_asan/tests/GwpAsan-x86_64-Test 
  cd 
/home/jenkins/j/w/workspace/opensource/opensource_build/build/projects/compiler-rt/lib/gwp_asan/tests
 && /home/jenkins/j/w/workspace/opensource/opensource_build/build/./bin/clang++ 
GwpAsanTestObjects.printf_sanitizer_common.cpp.x86_64.o 
GwpAsanTestObjects.alignment.cpp.x86_64.o 
GwpAsanTestObjects.backtrace.cpp.x86_64.o GwpAsanTestObjects.basic.cpp.x86_64.o 
GwpAsanTestObjects.compression.cpp.x86_64.o 
GwpAsanTestObjects.iterate.cpp.x86_64.o 
GwpAsanTestObjects.crash_handler_api.cpp.x86_64.o 
GwpAsanTestObjects.driver.cpp.x86_64.o 
GwpAsanTestObjects.mutex_test.cpp.x86_64.o 
GwpAsanTestObjects.slot_reuse.cpp.x86_64.o 
GwpAsanTestObjects.thread_contention.cpp.x86_64.o 
GwpAsanTestObjects.harness.cpp.x86_64.o 
GwpAsanTestObjects.enable_disable.cpp.x86_64.o 
GwpAsanTestObjects.late_init.cpp.x86_64.o 
GwpAsanTestObjects.options.cpp.x86_64.o 
GwpAsanTestObjects.gtest-all.cc.x86_64.o 
/home/jenkins/j/w/workspace/opensource/opensource_build/build/projects/compiler-rt/lib/gwp_asan/tests/libRTGwpAsanTest.x86_64.a
 -o 
/home/jenkins/j/w/workspace/opensource/opensource_build/build/projects/compiler-rt/lib/gwp_asan/tests/./GwpAsan-x86_64-Test
 -ldl -lstdc++ --driver-mode=g++ -pthread -m64
  /usr/bin/ld: /usr/bin/ld: DWARF error: invalid or unhandled FORM value: 0x25
  GwpAsanTestObjects.backtrace.cpp.x86_64.o: in function 
`Backtrace_ExceedsStorableLength_Test::TestBody()':
  backtrace.cpp:(.text+0xce6): undefined reference to 
`gwp_asan::AllocationMetadata::kMaxTraceLengthToCollect'
  clang-16: error: linker command failed with exit code 1 (use -v to see 
invocation)

And it seems at least one buildbot is also hitting the same issue:
https://lab.llvm.org/staging/#/builders/180/builds/7174

  [165/1101] Generating GwpAsan-x86_64-Test
  FAILED: projects/compiler-rt/lib/gwp_asan/tests/GwpAsan-x86_64-Test 
  cd 
/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/stage1/projects/compiler-rt/lib/gwp_asan/tests
 && 
/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/stage1/./bin/clang++
 GwpAsanTestObjects.printf_sanitizer_common.cpp.x86_64.o 
GwpAsanTestObjects.alignment.cpp.x86_64.o 
GwpAsanTestObjects.backtrace.cpp.x86_64.o GwpAsanTestObjects.basic.cpp.x86_64.o 
GwpAsanTestObjects.compression.cpp.x86_64.o 
GwpAsanTestObjects.iterate.cpp.x86_64.o 
GwpAsanTestObjects.crash_handler_api.cpp.x86_64.o 
GwpAsanTestObjects.driver.cpp.x86_64.o 
GwpAsanTestObjects.mutex_test.cpp.x86_64.o 
GwpAsanTestObjects.slot_reuse.cpp.x86_64.o 
GwpAsanTestObjects.thread_contention.cpp.x86_64.o 
GwpAsanTestObjects.harness.cpp.x86_64.o 
GwpAsanTestObjects.enable_disable.cpp.x86_64.o 
GwpAsanTestObjects.late_init.cpp.x86_64.o 
GwpAsanTestObjects.options.cpp.x86_64.o 
GwpAsanTestObjects.gtest-all.cc.x86_64.o 
/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/stage1/projects/compiler-rt/lib/gwp_asan/tests/libRTGwpAsanTest.x86_64.a
 -o 
/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/stage1/projects/compiler-rt/lib/gwp_asan/tests/./GwpAsan-x86_64-Test
 -ldl -lstdc++ --driver-mode=g++ -pthread -m64
  /usr/lib64/gcc/x86_64-suse-linux/7/../../../../x86_64-suse-linux/bin/ld: 
/usr/lib64/gcc/x86_64-suse-linux/7/../../../../x86_64-suse-linux/bin/ld: DWARF 
error: invalid or unhandled FORM value: 0x23
  GwpAsanTestObjects.backtrace.cpp.x86_64.o: in function 
`Backtrace_ExceedsStorableLength_Test::TestBody()':
  backtrace.cpp:(.text+0xca6): undefined reference to 
`gwp_asan::AllocationMetadata::kMaxTraceLengthToCollect'
  clang-16: error: linker command failed with exit code 1 (use -v to see 
invocation)

Switching between BFD ld and gold still fails (although gold fails for a 
slightly different reason). Superficially it seems that switching to C++17 for 
some reason might be causing the compiler to emit debug info that these older 
non-lld linkers cannot understand for some reason?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

In D130689#3705236 , @royjacobson 
wrote:

> This affects people on their work branches as well, and it's not obvious that 
> it's a configuration error and not a broken master.
>
> The CMake approach sounds cleaner to me, but I don't know CMake well enough 
> to do it - if you could post a follow up patch I think it would be quite 
> helpful.

Good point - I tried a few things and came up with an approach that works here: 
https://reviews.llvm.org/D131367 - have a look.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson added a comment.

In D130689#3705145 , @thieta wrote:

> In D130689#3705131 , @royjacobson 
> wrote:
>
>> This seems to have been more disruptive than expected, since an existing 
>> CMakeCache.txt can make LLVM compile in previous C++14 configuration. This 
>> seems to make some of the bots fail in a way that makes the patches making 
>> use of C++17 features seem at fault.
>>
>> See:
>> https://github.com/llvm/llvm-project/commit/ede96de751224487aea122af8bfb4e82bc54840b#commitcomment-80507826
>> https://reviews.llvm.org/rG32fd0b7fd5ab
>>
>> How would you feel about adding something like
>>
>>   #if defined(__cplusplus) && __cplusplus < 201703L
>>   #error "LLVM requires at least C++17"
>>   #endif
>>
>> to some central header, to make this switch more visible?
>
> I am not opposed to that directly. But this seems a bit dangerous where bots 
> retain the cmakecache - there must be other cases where we can't really 
> protect in this way.
>
> Another approach would be to unset CMAKE_CXX_STANDARD if it's below 17 in 
> cmake directly.
>
> But in general - I am not a huge fan of CI / bots trying to keep the cache 
> around - many weird issues can arise from this.

This affects people on their work branches as well, and it's not obvious that 
it's a configuration error and not a broken master.

The CMake approach sounds cleaner to me, but I don't know CMake well enough to 
do it - if you could post a follow up patch I think it would be quite helpful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

In D130689#3705131 , @royjacobson 
wrote:

> This seems to have been more disruptive than expected, since an existing 
> CMakeCache.txt can make LLVM compile in previous C++14 configuration. This 
> seems to make some of the bots fail in a way that makes the patches making 
> use of C++17 features seem at fault.
>
> See:
> https://github.com/llvm/llvm-project/commit/ede96de751224487aea122af8bfb4e82bc54840b#commitcomment-80507826
> https://reviews.llvm.org/rG32fd0b7fd5ab
>
> How would you feel about adding something like
>
>   #if defined(__cplusplus) && __cplusplus < 201703L
>   #error "LLVM requires at least C++17"
>   #endif
>
> to some central header, to make this switch more visible?

I am not opposed to that directly. But this seems a bit dangerous where bots 
retain the cmakecache - there must be other cases where we can't really protect 
in this way.

Another approach would be to unset CMAKE_CXX_STANDARD if it's below 17 in cmake 
directly.

But in general - I am not a huge fan of CI / bots trying to keep the cache 
around - many weird issues can arise from this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson added a comment.

This seems to have been more disruptive than expected, since an existing 
CMakeCache.txt can make LLVM compile in previous C++14 configuration. This 
seems to make some of the bots fail in a way that makes the patches making use 
of C++17 features seem at fault.

See:
https://github.com/llvm/llvm-project/commit/ede96de751224487aea122af8bfb4e82bc54840b#commitcomment-80507826
https://reviews.llvm.org/rG32fd0b7fd5ab

How would you feel about adding something like

  #if defined(__cplusplus) && __cplusplus < 201703L
  #error "LLVM requires at least C++17"
  #endif

to some central header, to make this switch more visible?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D130689#3704581 , @dyung wrote:

> Your change is causing a build failure on the PS4 linux build bot using GCC 
> 9.3. Can you take a look?
> https://lab.llvm.org/buildbot/#/builders/139/builds/26186
>
>   FAILED: 
> tools/clang/tools/extra/clang-tidy/bugprone/CMakeFiles/obj.clangTidyBugproneModule.dir/SignalHandlerCheck.cpp.o
>  
>   CCACHE_CPP2=yes CCACHE_HASHDIR=yes /usr/bin/ccache /usr/bin/g++ 
> -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS 
> -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS 
> -I/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/build/tools/clang/tools/extra/clang-tidy/bugprone
>  
> -I/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/llvm-project/clang-tools-extra/clang-tidy/bugprone
>  
> -I/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/build/tools/clang/tools/extra/clang-tidy
>  
> -I/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/llvm-project/clang/include
>  
> -I/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/build/tools/clang/include
>  
> -I/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/build/include
>  
> -I/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/llvm-project/llvm/include
>  -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden 
> -Werror=date-time -Wall -Wextra -Wno-unused-parameter -Wwrite-strings 
> -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long 
> -Wimplicit-fallthrough -Wno-maybe-uninitialized -Wno-class-memaccess 
> -Wno-redundant-move -Wno-pessimizing-move -Wno-noexcept-type 
> -Wdelete-non-virtual-dtor -Wsuggest-override -Wno-comment 
> -Wmisleading-indentation -fdiagnostics-color -ffunction-sections 
> -fdata-sections -fno-common -Woverloaded-virtual -fno-strict-aliasing -O3 
> -DNDEBUG  -fno-exceptions -fno-rtti -UNDEBUG -std=c++17 -MD -MT 
> tools/clang/tools/extra/clang-tidy/bugprone/CMakeFiles/obj.clangTidyBugproneModule.dir/SignalHandlerCheck.cpp.o
>  -MF 
> tools/clang/tools/extra/clang-tidy/bugprone/CMakeFiles/obj.clangTidyBugproneModule.dir/SignalHandlerCheck.cpp.o.d
>  -o 
> tools/clang/tools/extra/clang-tidy/bugprone/CMakeFiles/obj.clangTidyBugproneModule.dir/SignalHandlerCheck.cpp.o
>  -c 
> /home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/llvm-project/clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp
>   
> /home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/llvm-project/clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp:17:45:
>  error: modification of ‘’ is not a constant expression
>  17 | "signal", "abort", "_Exit", "quick_exit"};
> | ^
>   
> /home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/llvm-project/clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp:217:12:
>  error: modification of ‘’ is not a constant expression
> 217 | "write"};
> |^

Fixed by c7ec86b13c461f6a8ce11f8443c1b6242013d26f 
 .
May be related to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102921


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Trass3r via Phabricator via cfe-commits]

Trass3r added a comment.

Also fails on gcc 11.2: https://github.com/Trass3r/llvm-project/runs/7703302032


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Douglas Yung via Phabricator via cfe-commits]

dyung added a comment.

Your change is causing a build failure on the PS4 linux build bot using GCC 
9.3. Can you take a look?
https://lab.llvm.org/buildbot/#/builders/139/builds/26186

  FAILED: 
tools/clang/tools/extra/clang-tidy/bugprone/CMakeFiles/obj.clangTidyBugproneModule.dir/SignalHandlerCheck.cpp.o
 
  CCACHE_CPP2=yes CCACHE_HASHDIR=yes /usr/bin/ccache /usr/bin/g++ 
-DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS 
-D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS 
-I/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/build/tools/clang/tools/extra/clang-tidy/bugprone
 
-I/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/llvm-project/clang-tools-extra/clang-tidy/bugprone
 
-I/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/build/tools/clang/tools/extra/clang-tidy
 
-I/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/llvm-project/clang/include
 
-I/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/build/tools/clang/include
 -I/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/build/include 
-I/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/llvm-project/llvm/include
 -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden 
-Werror=date-time -Wall -Wextra -Wno-unused-parameter -Wwrite-strings 
-Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long 
-Wimplicit-fallthrough -Wno-maybe-uninitialized -Wno-class-memaccess 
-Wno-redundant-move -Wno-pessimizing-move -Wno-noexcept-type 
-Wdelete-non-virtual-dtor -Wsuggest-override -Wno-comment 
-Wmisleading-indentation -fdiagnostics-color -ffunction-sections 
-fdata-sections -fno-common -Woverloaded-virtual -fno-strict-aliasing -O3 
-DNDEBUG  -fno-exceptions -fno-rtti -UNDEBUG -std=c++17 -MD -MT 
tools/clang/tools/extra/clang-tidy/bugprone/CMakeFiles/obj.clangTidyBugproneModule.dir/SignalHandlerCheck.cpp.o
 -MF 
tools/clang/tools/extra/clang-tidy/bugprone/CMakeFiles/obj.clangTidyBugproneModule.dir/SignalHandlerCheck.cpp.o.d
 -o 
tools/clang/tools/extra/clang-tidy/bugprone/CMakeFiles/obj.clangTidyBugproneModule.dir/SignalHandlerCheck.cpp.o
 -c 
/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/llvm-project/clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp
  
/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/llvm-project/clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp:17:45:
 error: modification of ‘’ is not a constant expression
 17 | "signal", "abort", "_Exit", "quick_exit"};
| ^
  
/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/llvm-project/clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp:217:12:
 error: modification of ‘’ is not a constant expression
217 | "write"};
|^


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Tobias Hieta via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb1356504e63a: [LLVM] Update C++ standard to 17 (authored by 
thieta).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

Files:
  bolt/runtime/CMakeLists.txt
  clang/CMakeLists.txt
  lld/CMakeLists.txt
  lldb/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/cmake/modules/CheckCompilerVersion.cmake
  llvm/docs/CodingStandards.rst
  llvm/docs/ReleaseNotes.rst

Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -47,6 +47,18 @@
 Update on required toolchains to build LLVM
 ---
 
+LLVM is now built with C++17 by default. This means C++17 can be used in
+the code base.
+
+The previous "soft" toolchain requirements have now been changed to "hard".
+This means that the the following versions are now required to build LLVM
+and there is no way to suppress this error.
+
+* GCC >= 7.1
+* Clang >= 5.0
+* Apple Clang >= 9.3
+* Visual Studio 2019 >= 16.7
+
 Changes to the LLVM IR
 --
 
Index: llvm/docs/CodingStandards.rst
===
--- llvm/docs/CodingStandards.rst
+++ llvm/docs/CodingStandards.rst
@@ -53,7 +53,7 @@
 C++ Standard Versions
 -
 
-Unless otherwise documented, LLVM subprojects are written using standard C++14
+Unless otherwise documented, LLVM subprojects are written using standard C++17
 code and avoid unnecessary vendor-specific extensions.
 
 Nevertheless, we restrict ourselves to features which are available in the
@@ -63,7 +63,13 @@
 Each toolchain provides a good reference for what it accepts:
 
 * Clang: https://clang.llvm.org/cxx_status.html
-* GCC: https://gcc.gnu.org/projects/cxx-status.html#cxx14
+
+  * libc++: https://libcxx.llvm.org/Status/Cxx17.html
+
+* GCC: https://gcc.gnu.org/projects/cxx-status.html#cxx17
+
+  * libstdc++: https://gcc.gnu.org/onlinedocs/libstdc++/manual/status.html#status.iso.2017
+
 * MSVC: https://msdn.microsoft.com/en-us/library/hh567368.aspx
 
 
Index: llvm/cmake/modules/CheckCompilerVersion.cmake
===
--- llvm/cmake/modules/CheckCompilerVersion.cmake
+++ llvm/cmake/modules/CheckCompilerVersion.cmake
@@ -4,21 +4,19 @@
 
 include(CheckCXXSourceCompiles)
 
-set(GCC_MIN 5.1)
+set(GCC_MIN 7.1)
 set(GCC_SOFT_ERROR 7.1)
-set(CLANG_MIN 3.5)
+set(CLANG_MIN 5.0)
 set(CLANG_SOFT_ERROR 5.0)
-set(APPLECLANG_MIN 6.0)
+set(APPLECLANG_MIN 9.3)
 set(APPLECLANG_SOFT_ERROR 9.3)
 
 # https://en.wikipedia.org/wiki/Microsoft_Visual_C#Internal_version_numbering
-# _MSC_VER == 1920 MSVC++ 14.20 Visual Studio 2019 Version 16.0
 # _MSC_VER == 1927 MSVC++ 14.27 Visual Studio 2019 Version 16.7
-set(MSVC_MIN 19.20)
+set(MSVC_MIN 19.27)
 set(MSVC_SOFT_ERROR 19.27)
 
-# Map the above GCC versions to dates: https://gcc.gnu.org/develop.html#timeline
-set(GCC_MIN_DATE 20150422)
+set(LIBSTDCXX_MIN 7)
 set(LIBSTDCXX_SOFT_ERROR 7)
 
 
@@ -76,22 +74,14 @@
 set(OLD_CMAKE_REQUIRED_FLAGS ${CMAKE_REQUIRED_FLAGS})
 set(OLD_CMAKE_REQUIRED_LIBRARIES ${CMAKE_REQUIRED_LIBRARIES})
 set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -std=c++0x")
-# Test for libstdc++ version of at least 4.8 by checking for _ZNKSt17bad_function_call4whatEv.
-# Note: We should check _GLIBCXX_RELEASE when possible (i.e., for GCC 7.1 and up).
 check_cxx_source_compiles("
 #include 
 #if defined(__GLIBCXX__)
-#if __GLIBCXX__ < ${GCC_MIN_DATE}
+#if !defined(_GLIBCXX_RELEASE) || _GLIBCXX_RELEASE < ${LIBSTDCXX_MIN}
 #error Unsupported libstdc++ version
 #endif
 #endif
-#if defined(__GLIBCXX__)
-extern const char _ZNKSt17bad_function_call4whatEv[];
-const char *chk = _ZNKSt17bad_function_call4whatEv;
-#else
-const char *chk = \"\";
-#endif
-int main() { ++chk; return 0; }
+int main() { return 0; }
 "
   LLVM_LIBSTDCXX_MIN)
 if(NOT LLVM_LIBSTDCXX_MIN)
Index: llvm/CMakeLists.txt
===
--- llvm/CMakeLists.txt
+++ llvm/CMakeLists.txt
@@ -58,7 +58,7 @@
 # Must go after project(..)
 include(GNUInstallDirs)
 
-set(CMAKE_CXX_STANDARD 14 CACHE STRING "C++ standard to conform to")
+set(CMAKE_CXX_STANDARD 17 CACHE STRING "C++ standard to conform to")
 set(CMAKE_CXX_STANDARD_REQUIRED YES)
 if (CYGWIN)
   # Cygwin is a bit stricter and lack things like 'strdup', 'stricmp', etc in
Index: lldb/CMakeLists.txt
===
--- lldb/CMakeLists.txt
+++ lldb/CMakeLists.txt
@@ -20,7 +20,7 @@
 if(LLDB_BUILT_STANDALONE)
   include(LLDBStandalone)
 
-  set(CMAKE_CXX_STANDARD 14 CACHE STRING "C++ standard to conform to")
+  set(CMAKE_CXX_STANDARD 17 CACHE STRING "C++ standard to c

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

[Tobias Hieta via Phabricator via cfe-commits]

thieta updated this revision to Diff 450364.
thieta added a comment.

Fixed spelling error and added links to C++ standard libraries


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

Files:
  bolt/runtime/CMakeLists.txt
  clang/CMakeLists.txt
  lld/CMakeLists.txt
  lldb/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/cmake/modules/CheckCompilerVersion.cmake
  llvm/docs/CodingStandards.rst
  llvm/docs/ReleaseNotes.rst

Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -47,6 +47,18 @@
 Update on required toolchains to build LLVM
 ---
 
+LLVM is now built with C++17 by default. This means C++17 can be used in
+the code base.
+
+The previous "soft" toolchain requirements have now been changed to "hard".
+This means that the the following versions are now required to build LLVM
+and there is no way to suppress this error.
+
+* GCC >= 7.1
+* Clang >= 5.0
+* Apple Clang >= 9.3
+* Visual Studio 2019 >= 16.7
+
 Changes to the LLVM IR
 --
 
Index: llvm/docs/CodingStandards.rst
===
--- llvm/docs/CodingStandards.rst
+++ llvm/docs/CodingStandards.rst
@@ -53,7 +53,7 @@
 C++ Standard Versions
 -
 
-Unless otherwise documented, LLVM subprojects are written using standard C++14
+Unless otherwise documented, LLVM subprojects are written using standard C++17
 code and avoid unnecessary vendor-specific extensions.
 
 Nevertheless, we restrict ourselves to features which are available in the
@@ -63,7 +63,13 @@
 Each toolchain provides a good reference for what it accepts:
 
 * Clang: https://clang.llvm.org/cxx_status.html
-* GCC: https://gcc.gnu.org/projects/cxx-status.html#cxx14
+
+  * libc++: https://libcxx.llvm.org/Status/Cxx17.html
+
+* GCC: https://gcc.gnu.org/projects/cxx-status.html#cxx17
+
+  * libstdc++: https://gcc.gnu.org/onlinedocs/libstdc++/manual/status.html#status.iso.2017
+
 * MSVC: https://msdn.microsoft.com/en-us/library/hh567368.aspx
 
 
Index: llvm/cmake/modules/CheckCompilerVersion.cmake
===
--- llvm/cmake/modules/CheckCompilerVersion.cmake
+++ llvm/cmake/modules/CheckCompilerVersion.cmake
@@ -4,21 +4,19 @@
 
 include(CheckCXXSourceCompiles)
 
-set(GCC_MIN 5.1)
+set(GCC_MIN 7.1)
 set(GCC_SOFT_ERROR 7.1)
-set(CLANG_MIN 3.5)
+set(CLANG_MIN 5.0)
 set(CLANG_SOFT_ERROR 5.0)
-set(APPLECLANG_MIN 6.0)
+set(APPLECLANG_MIN 9.3)
 set(APPLECLANG_SOFT_ERROR 9.3)
 
 # https://en.wikipedia.org/wiki/Microsoft_Visual_C#Internal_version_numbering
-# _MSC_VER == 1920 MSVC++ 14.20 Visual Studio 2019 Version 16.0
 # _MSC_VER == 1927 MSVC++ 14.27 Visual Studio 2019 Version 16.7
-set(MSVC_MIN 19.20)
+set(MSVC_MIN 19.27)
 set(MSVC_SOFT_ERROR 19.27)
 
-# Map the above GCC versions to dates: https://gcc.gnu.org/develop.html#timeline
-set(GCC_MIN_DATE 20150422)
+set(LIBSTDCXX_MIN 7)
 set(LIBSTDCXX_SOFT_ERROR 7)
 
 
@@ -76,22 +74,14 @@
 set(OLD_CMAKE_REQUIRED_FLAGS ${CMAKE_REQUIRED_FLAGS})
 set(OLD_CMAKE_REQUIRED_LIBRARIES ${CMAKE_REQUIRED_LIBRARIES})
 set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -std=c++0x")
-# Test for libstdc++ version of at least 4.8 by checking for _ZNKSt17bad_function_call4whatEv.
-# Note: We should check _GLIBCXX_RELEASE when possible (i.e., for GCC 7.1 and up).
 check_cxx_source_compiles("
 #include 
 #if defined(__GLIBCXX__)
-#if __GLIBCXX__ < ${GCC_MIN_DATE}
+#if !defined(_GLIBCXX_RELEASE) || _GLIBCXX_RELEASE < ${LIBSTDCXX_MIN}
 #error Unsupported libstdc++ version
 #endif
 #endif
-#if defined(__GLIBCXX__)
-extern const char _ZNKSt17bad_function_call4whatEv[];
-const char *chk = _ZNKSt17bad_function_call4whatEv;
-#else
-const char *chk = \"\";
-#endif
-int main() { ++chk; return 0; }
+int main() { return 0; }
 "
   LLVM_LIBSTDCXX_MIN)
 if(NOT LLVM_LIBSTDCXX_MIN)
Index: llvm/CMakeLists.txt
===
--- llvm/CMakeLists.txt
+++ llvm/CMakeLists.txt
@@ -58,7 +58,7 @@
 # Must go after project(..)
 include(GNUInstallDirs)
 
-set(CMAKE_CXX_STANDARD 14 CACHE STRING "C++ standard to conform to")
+set(CMAKE_CXX_STANDARD 17 CACHE STRING "C++ standard to conform to")
 set(CMAKE_CXX_STANDARD_REQUIRED YES)
 if (CYGWIN)
   # Cygwin is a bit stricter and lack things like 'strdup', 'stricmp', etc in
Index: lldb/CMakeLists.txt
===
--- lldb/CMakeLists.txt
+++ lldb/CMakeLists.txt
@@ -20,7 +20,7 @@
 if(LLDB_BUILT_STANDALONE)
   include(LLDBStandalone)
 
-  set(CMAKE_CXX_STANDARD 14 CACHE STRING "C++ standard to conform to")
+  set(CMAKE_CXX_STANDARD 17 CACHE STRING "C++ standard to conform to")
   set(CMAKE_CXX_STANDARD_REQUIRED YES)
   set(CMAKE_CXX_EXTENSIONS NO)
 

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

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

All right! Last call - I am going fix the spelling error and merge this 
tomorrow EU time unless someone objects. Thanks for all reviews so far!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

In D130689#3696186 , @thieta wrote:

> @nikic @thakis I fixed this issue in https://reviews.llvm.org/D131063 and it 
> can be built with CXX_STANDARD=17 and OSX_DEPLOYMENT_TARGET=10.11.

Thanks! We took this opportunity to bump our clang deployment target from 10.7 
to 10.12 (which makes a few other minor things easier too), so we don't need 
this change any more. But it's a good change anyways :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Fraser Cormack via Phabricator via cfe-commits]

frasercrmck added inline comments.



Comment at: llvm/docs/ReleaseNotes.rst:55
+This means that the the following versions are now required to build LLVM
+and there is no way to supress this error.
+

`suppress`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

@nikic @thakis I fixed this issue in https://reviews.llvm.org/D131063 and it 
can be built with CXX_STANDARD=17 and OSX_DEPLOYMENT_TARGET=10.11.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

In D130689#3690274 , @nikic wrote:

> 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?

Yeah - I just realized that when I checked this. Just building the rest of the 
tree now to confirm this is the only change we need and I will publish this a 
different diff first.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Nikita Popov via Phabricator via cfe-commits]

nikic added a comment.

In D130689#3690258 , @thieta wrote:

> In 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

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

In 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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

In D130689#3686716 , @thieta wrote:

> You can already test this with `-DCMAKE_CXX_STANDARD=17` afaik. I wonder how 
> many bot owners would actually test this if we made another flag available.

Thanks, that works.

Our linux and win bots are happy, but our mac bot fails with:

  
/opt/s/w/ir/cache/builder/src/third_party/llvm/llvm/include/llvm/Support/RWMutex.h:98:8:
 error: 'shared_mutex' is unavailable: introduced in macOS 10.12
std::shared_mutex impl;
 ^
  
/opt/s/w/ir/cache/osx_sdk/XCode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.3.sdk/usr/include/c++/v1/shared_mutex:179:58:
 note: 'shared_mutex' has been explicitly marked unavailable here
  class _LIBCPP_TYPE_VIS _LIBCPP_AVAILABILITY_SHARED_MUTEX shared_mutex
   ^

Is it expected and intentional that this increases the mac deployment target to 
10.12?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini added a comment.

In D130689#3686760 , @h-vetinari 
wrote:

> My point boils down to: "written using standard C++17
> code" does not sound at all like "core language, no stdlib", but very much 
> like "core+stdlib".

We're allowing C++17 library feature, this isn't covered by the "vendor 
extensions" part but by the following paragraph:

> Nevertheless, we restrict ourselves to features which are available in the 
> major toolchains supported as host compilers

This includes not only missing features in libstdc++ but also gcc and clang 
bugs/limitations that we'll have to work around.

> This is also the first time this split becomes relevant AFAIK

I don't : the migration to C++11 was done the same way, piecewise by making 
sure that when we start using a new feature (core or stdlib) it actually works 
on the stated minimum version of the toolchains we support.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[H. Vetinari via Phabricator via cfe-commits]

h-vetinari added a comment.

My point boils down to: "written using standard C++17
code" does not sound at all like "core language, no stdlib", but very much like 
"core+stdlib".

This is also the first time this split becomes relevant AFAIK, because for 
moving to C++14, the stdlib was ready basically around the same time / compiler 
versions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

In D130689#3686723 , @h-vetinari 
wrote:

> I don't think the standard library can be called a vendor-specific extension, 
> and so I think this still could/should be made clearer (even though the 
> status links below would show upon inspection that there's no full support 
> yet, but that's a bit too tucked away I feel).

For libstdc++ and libc++ there are also pages with status in a specific 
version. Maybe we should link those matrixes as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[H. Vetinari via Phabricator via cfe-commits]

h-vetinari added a comment.

From the text you quoted:

> LLVM subprojects are written using standard C++17

code and avoid unnecessary vendor-specific extensions.

I don't think the standard library can be called a vendor-specific extension, 
and so I think this still could/should be made clearer (even though the status 
links below would show upon inspection that there's no full support yet, but 
that's a bit too tucked away I feel).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

In D130689#3686718 , @thieta wrote:

> In D130689#3684399 , @h-vetinari 
> wrote:
>
>> It may be worth calling out that this is about C++17 core language and not 
>> the standard library?
>>
>> libstdcxx only finished C++17 support in GCC 12, and libcxx is still missing 
>> various pieces even today (much less for Clang 5).
>
> I will add a small line about this in the coding standards document.

Actually - never mind this is already well documented in the coding standards 
document:

  Unless otherwise documented, LLVM subprojects are written using standard C++17
  code and avoid unnecessary vendor-specific extensions.
  
  Nevertheless, we restrict ourselves to features which are available in the
  major toolchains supported as host compilers (see :doc:`GettingStarted` page,
  section `Software`).
  
  Each toolchain provides a good reference for what it accepts:
  
  * Clang: https://clang.llvm.org/cxx_status.html
  * GCC: https://gcc.gnu.org/projects/cxx-status.html#cxx17
  * MSVC: https://msdn.microsoft.com/en-us/library/hh567368.aspx

I feel that's good enough.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

In D130689#3684399 , @h-vetinari 
wrote:

> It may be worth calling out that this is about C++17 core language and not 
> the standard library?
>
> libstdcxx only finished C++17 support in GCC 12, and libcxx is still missing 
> various pieces even today (much less for Clang 5).

I will add a small line about this in the coding standards document.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

In D130689#3686397 , @thakis wrote:

> It'd be nice if this was landed opt-in behind some cmake variable at first, 
> so that folks could try it out on their bots and see how well things work.

You can already test this with `-DCMAKE_CXX_STANDARD=17` afaik. I wonder how 
many bot owners would actually test this if we made another flag available.

Since we can already test this with a current flag maybe we should just post to 
discourse that bot-owners can test it and a date when this will be merged. But 
I don't expect this to be a big problem, when we merged the soft error earlier 
this year - it only broke one bot which was out of date.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

It'd be nice if this was landed opt-in behind some cmake variable at first, so 
that folks could try it out on their bots and see how well things work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[H. Vetinari via Phabricator via cfe-commits]

h-vetinari added a comment.

It may be worth calling out that this is about C++17 core language and not the 
standard library?

libstdcxx only finished C++17 support in GCC 12, and libcxx is still missing 
various pieces even today (much less for Clang 5).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin accepted this revision.
cor3ntin added a comment.

Thanks a lot for working on this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Tobias Hieta via Phabricator via cfe-commits]

thieta updated this revision to Diff 448262.
thieta added a comment.

Fixed unintended indentation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

Files:
  bolt/runtime/CMakeLists.txt
  clang/CMakeLists.txt
  lld/CMakeLists.txt
  lldb/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/cmake/modules/CheckCompilerVersion.cmake
  llvm/docs/CodingStandards.rst
  llvm/docs/ReleaseNotes.rst

Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -47,6 +47,18 @@
 Update on required toolchains to build LLVM
 ---
 
+LLVM is now built with C++17 by default. This means C++17 can be used in
+the code base.
+
+The previous "soft" toolchain requirements have now been changed to "hard".
+This means that the the following versions are now required to build LLVM
+and there is no way to supress this error.
+
+* GCC >= 7.1
+* Clang >= 5.0
+* Apple Clang >= 9.3
+* Visual Studio 2019 >= 16.7
+
 Changes to the LLVM IR
 --
 
Index: llvm/docs/CodingStandards.rst
===
--- llvm/docs/CodingStandards.rst
+++ llvm/docs/CodingStandards.rst
@@ -53,7 +53,7 @@
 C++ Standard Versions
 -
 
-Unless otherwise documented, LLVM subprojects are written using standard C++14
+Unless otherwise documented, LLVM subprojects are written using standard C++17
 code and avoid unnecessary vendor-specific extensions.
 
 Nevertheless, we restrict ourselves to features which are available in the
@@ -63,7 +63,7 @@
 Each toolchain provides a good reference for what it accepts:
 
 * Clang: https://clang.llvm.org/cxx_status.html
-* GCC: https://gcc.gnu.org/projects/cxx-status.html#cxx14
+* GCC: https://gcc.gnu.org/projects/cxx-status.html#cxx17
 * MSVC: https://msdn.microsoft.com/en-us/library/hh567368.aspx
 
 
Index: llvm/cmake/modules/CheckCompilerVersion.cmake
===
--- llvm/cmake/modules/CheckCompilerVersion.cmake
+++ llvm/cmake/modules/CheckCompilerVersion.cmake
@@ -4,21 +4,19 @@
 
 include(CheckCXXSourceCompiles)
 
-set(GCC_MIN 5.1)
+set(GCC_MIN 7.1)
 set(GCC_SOFT_ERROR 7.1)
-set(CLANG_MIN 3.5)
+set(CLANG_MIN 5.0)
 set(CLANG_SOFT_ERROR 5.0)
-set(APPLECLANG_MIN 6.0)
+set(APPLECLANG_MIN 9.3)
 set(APPLECLANG_SOFT_ERROR 9.3)
 
 # https://en.wikipedia.org/wiki/Microsoft_Visual_C#Internal_version_numbering
-# _MSC_VER == 1920 MSVC++ 14.20 Visual Studio 2019 Version 16.0
 # _MSC_VER == 1927 MSVC++ 14.27 Visual Studio 2019 Version 16.7
-set(MSVC_MIN 19.20)
+set(MSVC_MIN 19.27)
 set(MSVC_SOFT_ERROR 19.27)
 
-# Map the above GCC versions to dates: https://gcc.gnu.org/develop.html#timeline
-set(GCC_MIN_DATE 20150422)
+set(LIBSTDCXX_MIN 7)
 set(LIBSTDCXX_SOFT_ERROR 7)
 
 
@@ -76,22 +74,14 @@
 set(OLD_CMAKE_REQUIRED_FLAGS ${CMAKE_REQUIRED_FLAGS})
 set(OLD_CMAKE_REQUIRED_LIBRARIES ${CMAKE_REQUIRED_LIBRARIES})
 set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -std=c++0x")
-# Test for libstdc++ version of at least 4.8 by checking for _ZNKSt17bad_function_call4whatEv.
-# Note: We should check _GLIBCXX_RELEASE when possible (i.e., for GCC 7.1 and up).
 check_cxx_source_compiles("
 #include 
 #if defined(__GLIBCXX__)
-#if __GLIBCXX__ < ${GCC_MIN_DATE}
+#if !defined(_GLIBCXX_RELEASE) || _GLIBCXX_RELEASE < ${LIBSTDCXX_MIN}
 #error Unsupported libstdc++ version
 #endif
 #endif
-#if defined(__GLIBCXX__)
-extern const char _ZNKSt17bad_function_call4whatEv[];
-const char *chk = _ZNKSt17bad_function_call4whatEv;
-#else
-const char *chk = \"\";
-#endif
-int main() { ++chk; return 0; }
+int main() { return 0; }
 "
   LLVM_LIBSTDCXX_MIN)
 if(NOT LLVM_LIBSTDCXX_MIN)
Index: llvm/CMakeLists.txt
===
--- llvm/CMakeLists.txt
+++ llvm/CMakeLists.txt
@@ -58,7 +58,7 @@
 # Must go after project(..)
 include(GNUInstallDirs)
 
-set(CMAKE_CXX_STANDARD 14 CACHE STRING "C++ standard to conform to")
+set(CMAKE_CXX_STANDARD 17 CACHE STRING "C++ standard to conform to")
 set(CMAKE_CXX_STANDARD_REQUIRED YES)
 if (CYGWIN)
   # Cygwin is a bit stricter and lack things like 'strdup', 'stricmp', etc in
Index: lldb/CMakeLists.txt
===
--- lldb/CMakeLists.txt
+++ lldb/CMakeLists.txt
@@ -20,7 +20,7 @@
 if(LLDB_BUILT_STANDALONE)
   include(LLDBStandalone)
 
-  set(CMAKE_CXX_STANDARD 14 CACHE STRING "C++ standard to conform to")
+  set(CMAKE_CXX_STANDARD 17 CACHE STRING "C++ standard to conform to")
   set(CMAKE_CXX_STANDARD_REQUIRED YES)
   set(CMAKE_CXX_EXTENSIONS NO)
 endif()
Index: lld/CMakeLists.txt
===
--- lld/CMakeLists.txt
+++ lld/CMakeLists.txt
@@ -11,7 +11,7 @@
 include(GNUInstallDirs)

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

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

In D130689#3684334 , @ChuanqiXu wrote:

> So it is free that developers want to use some C++17 features in a small 
> amount of code, right?

As soon as this land C++ 17 should be free to use everywhere yeah!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

In D130689#3684360 , @barannikov88 
wrote:

> There are a few places (primarily in ADT and Support) that check __cplusplus 
> > 201402. Do they need to be cleaned up?

Sounds good - but maybe that can be addressed in a separate diff unless they 
make the build fail after this change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Tobias Hieta via Phabricator via cfe-commits]

thieta updated this revision to Diff 448261.
thieta added a comment.

Address some old comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

Files:
  bolt/runtime/CMakeLists.txt
  clang/CMakeLists.txt
  lld/CMakeLists.txt
  lldb/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/cmake/modules/CheckCompilerVersion.cmake
  llvm/docs/CodingStandards.rst
  llvm/docs/ReleaseNotes.rst

Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -47,6 +47,18 @@
 Update on required toolchains to build LLVM
 ---
 
+LLVM is now built with C++17 by default. This means C++17 can be used in
+the code base.
+
+The previous "soft" toolchain requirements have now been changed to "hard".
+This means that the the following versions are now required to build LLVM
+and there is no way to supress this error.
+
+* GCC >= 7.1
+* Clang >= 5.0
+* Apple Clang >= 9.3
+* Visual Studio 2019 >= 16.7
+
 Changes to the LLVM IR
 --
 
Index: llvm/docs/CodingStandards.rst
===
--- llvm/docs/CodingStandards.rst
+++ llvm/docs/CodingStandards.rst
@@ -53,7 +53,7 @@
 C++ Standard Versions
 -
 
-Unless otherwise documented, LLVM subprojects are written using standard C++14
+Unless otherwise documented, LLVM subprojects are written using standard C++17
 code and avoid unnecessary vendor-specific extensions.
 
 Nevertheless, we restrict ourselves to features which are available in the
@@ -63,7 +63,7 @@
 Each toolchain provides a good reference for what it accepts:
 
 * Clang: https://clang.llvm.org/cxx_status.html
-* GCC: https://gcc.gnu.org/projects/cxx-status.html#cxx14
+* GCC: https://gcc.gnu.org/projects/cxx-status.html#cxx17
 * MSVC: https://msdn.microsoft.com/en-us/library/hh567368.aspx
 
 
Index: llvm/cmake/modules/CheckCompilerVersion.cmake
===
--- llvm/cmake/modules/CheckCompilerVersion.cmake
+++ llvm/cmake/modules/CheckCompilerVersion.cmake
@@ -4,21 +4,19 @@
 
 include(CheckCXXSourceCompiles)
 
-set(GCC_MIN 5.1)
+set(GCC_MIN 7.1)
 set(GCC_SOFT_ERROR 7.1)
-set(CLANG_MIN 3.5)
+set(CLANG_MIN 5.0)
 set(CLANG_SOFT_ERROR 5.0)
-set(APPLECLANG_MIN 6.0)
+set(APPLECLANG_MIN 9.3)
 set(APPLECLANG_SOFT_ERROR 9.3)
 
 # https://en.wikipedia.org/wiki/Microsoft_Visual_C#Internal_version_numbering
-# _MSC_VER == 1920 MSVC++ 14.20 Visual Studio 2019 Version 16.0
 # _MSC_VER == 1927 MSVC++ 14.27 Visual Studio 2019 Version 16.7
-set(MSVC_MIN 19.20)
+set(MSVC_MIN 19.27)
 set(MSVC_SOFT_ERROR 19.27)
 
-# Map the above GCC versions to dates: https://gcc.gnu.org/develop.html#timeline
-set(GCC_MIN_DATE 20150422)
+set(LIBSTDCXX_MIN 7)
 set(LIBSTDCXX_SOFT_ERROR 7)
 
 
@@ -76,23 +74,15 @@
 set(OLD_CMAKE_REQUIRED_FLAGS ${CMAKE_REQUIRED_FLAGS})
 set(OLD_CMAKE_REQUIRED_LIBRARIES ${CMAKE_REQUIRED_LIBRARIES})
 set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -std=c++0x")
-# Test for libstdc++ version of at least 4.8 by checking for _ZNKSt17bad_function_call4whatEv.
-# Note: We should check _GLIBCXX_RELEASE when possible (i.e., for GCC 7.1 and up).
 check_cxx_source_compiles("
-#include 
-#if defined(__GLIBCXX__)
-#if __GLIBCXX__ < ${GCC_MIN_DATE}
-#error Unsupported libstdc++ version
-#endif
-#endif
-#if defined(__GLIBCXX__)
-extern const char _ZNKSt17bad_function_call4whatEv[];
-const char *chk = _ZNKSt17bad_function_call4whatEv;
-#else
-const char *chk = \"\";
-#endif
-int main() { ++chk; return 0; }
-"
+  #include 
+  #if defined(__GLIBCXX__)
+  #if !defined(_GLIBCXX_RELEASE) || _GLIBCXX_RELEASE < ${LIBSTDCXX_MIN}
+  #error Unsupported libstdc++ version
+  #endif
+  #endif
+  int main() { return 0; }
+  "
   LLVM_LIBSTDCXX_MIN)
 if(NOT LLVM_LIBSTDCXX_MIN)
   message(FATAL_ERROR "libstdc++ version must be at least ${GCC_MIN}.")
Index: llvm/CMakeLists.txt
===
--- llvm/CMakeLists.txt
+++ llvm/CMakeLists.txt
@@ -58,7 +58,7 @@
 # Must go after project(..)
 include(GNUInstallDirs)
 
-set(CMAKE_CXX_STANDARD 14 CACHE STRING "C++ standard to conform to")
+set(CMAKE_CXX_STANDARD 17 CACHE STRING "C++ standard to conform to")
 set(CMAKE_CXX_STANDARD_REQUIRED YES)
 if (CYGWIN)
   # Cygwin is a bit stricter and lack things like 'strdup', 'stricmp', etc in
Index: lldb/CMakeLists.txt
===
--- lldb/CMakeLists.txt
+++ lldb/CMakeLists.txt
@@ -20,7 +20,7 @@
 if(LLDB_BUILT_STANDALONE)
   include(LLDBStandalone)
 
-  set(CMAKE_CXX_STANDARD 14 CACHE STRING "C++ standard to conform to")
+  set(CMAKE_CXX_STANDARD 17 CACHE STRING "C++ standard to conform to")
   set(CMAKE_CXX_STANDARD_REQUIRED YES)
   set(CMAKE_CXX_EXTENSIONS NO)
 endif()

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

[Sergei Barannikov via Phabricator via cfe-commits]

barannikov88 added a comment.

There are a few places (primarily in ADT and Support) that check __cplusplus > 
201402. Do they need to be cleaned up?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini added a comment.

In D130689#3684333 , @thieta wrote:

> In D130689#3684330 , @mehdi_amini 
> wrote:
>
>> What does it mean exactly? We can't use **anything** C++17 without writing 
>> it in the coding standards?
>> I'm not sure it'll be manageable: how do you see this playing out?
>
> Probably poorly worded - what I was trying to convey is that if we want to 
> use a C++17 feature that's really impactful on the syntax/readability we 
> should probably consider recommending ways to use it in the coding standards, 
> similar to our guidelines on using for() loops 
> (https://llvm.org/docs/CodingStandards.html#use-range-based-for-loops-wherever-possible)
>  or the auto keyword 
> (https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable).

Makes sense, thanks! Let's just update the wording to convey this and this all 
looks good to me :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Daniel Bertalan via Phabricator via cfe-commits]

BertalanD added inline comments.



Comment at: llvm/cmake/modules/CheckCompilerVersion.cmake:79-80
 set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -std=c++0x")
 # Test for libstdc++ version of at least 4.8 by checking for 
_ZNKSt17bad_function_call4whatEv.
 # Note: We should check _GLIBCXX_RELEASE when possible (i.e., for GCC 7.1 
and up).
 check_cxx_source_compiles("

This comment should be updated too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[James Henderson via Phabricator via cfe-commits]

jhenderson accepted this revision.
jhenderson added a comment.
This revision is now accepted and ready to land.

In D130689#3684330 , @mehdi_amini 
wrote:

> Nice, LGTM, thanks for driving this!
>
>> Remember that if we want to adopt some new feature in a bigger way it should 
>> be discussed and added to the CodingStandard document.
>
> What does it mean exactly? We can't use **anything** C++17 without writing it 
> in the coding standards?
> I'm not sure it'll be manageable: how do you see this playing out?

Based on the release note, I don't think that was what was intended, although I 
am curious to understand what //was// intended!

Looks good to me anyway (but get more buy-in, perhaps a new Discourse post too, 
not just an update on the existing thread, since new threads get emailed out to 
more people).




Comment at: llvm/cmake/modules/CheckCompilerVersion.cmake:20
 
 # Map the above GCC versions to dates: 
https://gcc.gnu.org/develop.html#timeline
+set(LIBSTDCXX_MIN 7)

This comment seems out-of-date now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

In D130689#3684333 , @thieta wrote:

> In D130689#3684330 , @mehdi_amini 
> wrote:
>
>> What does it mean exactly? We can't use **anything** C++17 without writing 
>> it in the coding standards?
>> I'm not sure it'll be manageable: how do you see this playing out?
>
> Probably poorly worded - what I was trying to convey is that if we want to 
> use a C++17 feature that's really impactful on the syntax/readability we 
> should probably consider recommending ways to use it in the coding standards, 
> similar to our guidelines on using for() loops 
> (https://llvm.org/docs/CodingStandards.html#use-range-based-for-loops-wherever-possible)
>  or the auto keyword 
> (https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable).

So it is free that developers want to use some C++17 features in a small amount 
of code, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

In D130689#3684330 , @mehdi_amini 
wrote:

> What does it mean exactly? We can't use **anything** C++17 without writing it 
> in the coding standards?
> I'm not sure it'll be manageable: how do you see this playing out?

Probably poorly worded - what I was trying to convey is that if we want to use 
a C++17 feature that's really impactful on the syntax/readability we should 
probably consider recommending ways to use it in the coding standards, similar 
to our guidelines on using for() loops 
(https://llvm.org/docs/CodingStandards.html#use-range-based-for-loops-wherever-possible)
 or the auto keyword 
(https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini added a comment.
Herald added a subscriber: JDevlieghere.

Nice, LGTM, thanks for driving this!

> Remember that if we want to adopt some new feature in a bigger way it should 
> be discussed and added to the CodingStandard document.

What does it mean exactly? We can't use **anything** C++17 without writing it 
in the coding standards?
I'm not sure it'll be manageable: how do you see this playing out?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Tobias Hieta via Phabricator via cfe-commits]

thieta created this revision.
thieta added reviewers: mehdi_amini, jyknight, jhenderson, hans, tstellar, 
cor3ntin, MaskRay.
Herald added subscribers: ayermolo, StephenFan, mgorny.
Herald added a reviewer: rafauler.
Herald added a reviewer: Amir.
Herald added a reviewer: maksfb.
Herald added a project: All.
thieta requested review of this revision.
Herald added subscribers: lldb-commits, cfe-commits, yota9.
Herald added projects: clang, LLDB, LLVM.

Also make the soft toolchain requirements hard. This allows
us to use C++17 features in LLVM now. Remember that if we want
to adopt some new feature in a bigger way it should be discussed
and added to the CodingStandard document.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130689

Files:
  bolt/runtime/CMakeLists.txt
  clang/CMakeLists.txt
  lld/CMakeLists.txt
  lldb/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/cmake/modules/CheckCompilerVersion.cmake
  llvm/docs/CodingStandards.rst
  llvm/docs/ReleaseNotes.rst

Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -47,6 +47,18 @@
 Update on required toolchains to build LLVM
 ---
 
+LLVM is now built with C++17 by default. This means C++17 can be used in
+the code base.
+
+The previous "soft" toolchain requirements have now been changed to "hard".
+This means that the the following versions are now required to build LLVM
+and there is no way to supress this error.
+
+* GCC >= 7.1
+* Clang >= 5.0
+* Apple Clang >= 9.3
+* Visual Studio 2019 >= 16.7
+
 Changes to the LLVM IR
 --
 
Index: llvm/docs/CodingStandards.rst
===
--- llvm/docs/CodingStandards.rst
+++ llvm/docs/CodingStandards.rst
@@ -53,7 +53,7 @@
 C++ Standard Versions
 -
 
-Unless otherwise documented, LLVM subprojects are written using standard C++14
+Unless otherwise documented, LLVM subprojects are written using standard C++17
 code and avoid unnecessary vendor-specific extensions.
 
 Nevertheless, we restrict ourselves to features which are available in the
@@ -63,7 +63,7 @@
 Each toolchain provides a good reference for what it accepts:
 
 * Clang: https://clang.llvm.org/cxx_status.html
-* GCC: https://gcc.gnu.org/projects/cxx-status.html#cxx14
+* GCC: https://gcc.gnu.org/projects/cxx-status.html#cxx17
 * MSVC: https://msdn.microsoft.com/en-us/library/hh567368.aspx
 
 
Index: llvm/cmake/modules/CheckCompilerVersion.cmake
===
--- llvm/cmake/modules/CheckCompilerVersion.cmake
+++ llvm/cmake/modules/CheckCompilerVersion.cmake
@@ -4,21 +4,21 @@
 
 include(CheckCXXSourceCompiles)
 
-set(GCC_MIN 5.1)
+set(GCC_MIN 7.1)
 set(GCC_SOFT_ERROR 7.1)
-set(CLANG_MIN 3.5)
+set(CLANG_MIN 5.0)
 set(CLANG_SOFT_ERROR 5.0)
-set(APPLECLANG_MIN 6.0)
+set(APPLECLANG_MIN 9.3)
 set(APPLECLANG_SOFT_ERROR 9.3)
 
 # https://en.wikipedia.org/wiki/Microsoft_Visual_C#Internal_version_numbering
 # _MSC_VER == 1920 MSVC++ 14.20 Visual Studio 2019 Version 16.0
 # _MSC_VER == 1927 MSVC++ 14.27 Visual Studio 2019 Version 16.7
-set(MSVC_MIN 19.20)
+set(MSVC_MIN 19.27)
 set(MSVC_SOFT_ERROR 19.27)
 
 # Map the above GCC versions to dates: https://gcc.gnu.org/develop.html#timeline
-set(GCC_MIN_DATE 20150422)
+set(LIBSTDCXX_MIN 7)
 set(LIBSTDCXX_SOFT_ERROR 7)
 
 
@@ -79,20 +79,14 @@
 # Test for libstdc++ version of at least 4.8 by checking for _ZNKSt17bad_function_call4whatEv.
 # Note: We should check _GLIBCXX_RELEASE when possible (i.e., for GCC 7.1 and up).
 check_cxx_source_compiles("
-#include 
-#if defined(__GLIBCXX__)
-#if __GLIBCXX__ < ${GCC_MIN_DATE}
-#error Unsupported libstdc++ version
-#endif
-#endif
-#if defined(__GLIBCXX__)
-extern const char _ZNKSt17bad_function_call4whatEv[];
-const char *chk = _ZNKSt17bad_function_call4whatEv;
-#else
-const char *chk = \"\";
-#endif
-int main() { ++chk; return 0; }
-"
+  #include 
+  #if defined(__GLIBCXX__)
+  #if !defined(_GLIBCXX_RELEASE) || _GLIBCXX_RELEASE < ${LIBSTDCXX_MIN}
+  #error Unsupported libstdc++ version
+  #endif
+  #endif
+  int main() { return 0; }
+  "
   LLVM_LIBSTDCXX_MIN)
 if(NOT LLVM_LIBSTDCXX_MIN)
   message(FATAL_ERROR "libstdc++ version must be at least ${GCC_MIN}.")
Index: llvm/CMakeLists.txt
===
--- llvm/CMakeLists.txt
+++ llvm/CMakeLists.txt
@@ -58,7 +58,7 @@
 # Must go after project(..)
 include(GNUInstallDirs)
 
-set(CMAKE_CXX_STANDARD 14 CACHE STRING "C++ standard to conform to")
+set(CMAKE_CXX_STANDARD 17 CACHE STRING "C++ standard to conform to")
 set(CMAKE_CXX_STANDARD_REQUIRED YES)
 if (CYGWIN)
   # Cygwin is a bit stricter and lack things like 'strdup', 'stricmp', etc in
Index: lldb/CMakeLists.txt
===
--- lldb/CMakeLists