[clang] [llvm] [libcxx] "Reapply "[Sema] Fix crash on invalid code with parenthesized aggrega… (PR #76833)

Ilya Biryukov via cfe-commits cfe-commits at lists.llvm.org
Fri Jan 5 02:16:56 PST 2024


ilya-biryukov wrote:

> It's not ideal, but I can't think of a better solution. Typically nightly apt builds are built nightly so then we can update the Docker CI quickly. If the nightly builds are red it might take a bit longer.
> The patch is in draft since I wanted to test with the bootstrap build which required temporary disabling other builds (the second commit). When the CI is green I'll do a final review and publish the patch.

It's been 2 days, maybe we should just submit this and see if anything goes wrong?
Besides, the CI seems green now, so hopefully everything has already been checked.

> > In the long-term, how should people land changes like this? Compiler diagnostics are expected to change sometimes, so we should figure out some way that allows landing these without reverts. Any thoughts?
> 
> Typically we work together with whom changes the tests. When there are libc++ changes it is visible in our review queue. For changes in diagnostics it's often possible to use a regex matcher. For example, the messages of `static_asserts` changed a while back. In general we prefer to minimize the tests depending on the compiler diagnostics.

This use-case is different, we need a way to change the set of reported diagnostics in Clang even if the diagnostic is produced in libc++ tests. Blocking changes to Clang because libc++ tests against nightly and head builds looks too harsh.

I think we need to find some alternative solution that works here. The obvious solution that I see is to run the nightly Clang as post-submits and allow them to be broken for this reasons (as it will fix itself after nightly Clang gets rebuilt).

@AaronBallman, @cor3ntin what are your thoughts as Clang maintainers? TLDR; is that libc++ runs tests in CI with both head Clang and nightly Clang builds. Therefore, if we happen to change the set of reported diagnostics in Clang (in this case it was a spurious error in initialization of invalid classes that got silenced with a Clang change), we cannot land this change without breaking the libc++ CI.

https://github.com/llvm/llvm-project/pull/76833


More information about the cfe-commits mailing list