thieta added a comment.

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.

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

Reply via email to