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

Mon Aug 8 06:29:52 PDT 2022

aaron.ballman added a comment.

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

> In D130689#3706336 <https://reviews.llvm.org/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