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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
