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

Tue Aug 9 10:28:58 PDT 2022

aaron.ballman added a comment.

In D130689#3710340 <https://reviews.llvm.org/D130689#3710340>, @MaskRay wrote:

> In D130689#3710291 <https://reviews.llvm.org/D130689#3710291>, @aaron.ballman wrote:
>
>> In D130689#3710281 <https://reviews.llvm.org/D130689#3710281>, @royjacobson wrote:
>>
>>> In D130689#3709834 <https://reviews.llvm.org/D130689#3709834>, @thieta wrote:
>>>
>>>> In D130689#3709742 <https://reviews.llvm.org/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