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