Codesbyusman added a comment. In D129048#3671665 <https://reviews.llvm.org/D129048#3671665>, @erichkeane wrote:
> In D129048#3671657 <https://reviews.llvm.org/D129048#3671657>, @ldionne wrote: > >> In D129048#3671568 <https://reviews.llvm.org/D129048#3671568>, >> @aaron.ballman wrote: >> >>> FWIW, I've convinced myself that I agree with you here that the burden >>> probably should have been on libc++ maintainers in this case. libc++ almost >>> feels more like a downstream consumer of Clang in terms of testing needs, >>> so I think when tests break because Clang diagnostic wording or behavior >>> changes in a standards conforming way, libc++ folks should address it, but >>> if Clang changes in a way that's not standards conforming, then Clang folks >>> should address it. (Roughly speaking.) >> >> Yes, exactly. It's always possible to write tests that depend on >> implementation details of another component in subtle ways, and the fact >> that such brittle tests exist doesn't create a responsibility on that >> component to avoid breaking those downstream users. Here, it's about Clang >> making a valid change and libc++ containing brittle tests, but it happens >> quite often where libc++ changes something valid and a downstream consumer >> is broken in some way. The LLVM revert culture has some benefits to keep >> things working in a post-commit CI world, however I've noticed that it also >> creates a lot of friction between projects and sometimes makes important >> work much more difficult to land than it should. For example, we're >> currently in the middle of D128146 <https://reviews.llvm.org/D128146> where >> LLDB reverted an important patch for LLVM 15 because one of their tests >> broke. This is not something that comes up super often, but it's extremely >> disruptive and frustrating when it does, and I think it would be worth >> trying to address. Anyway, I'm digressing now, but I'll try to talk to a few >> people at the LLVM Dev Meeting to see if this is a shared concern and to >> think about potential solutions to address this. >> >>> That said, I absolutely think we all need to continue to collaborate >>> closely with one another as best we can when issues arise, and I really >>> appreciate the discussion on how we can do that! >> >> Agreed. >> >>> For this particular issue, I'd like @Codesbyusman to continue to try to fix >>> the libc++ testing issues (it's good experience), but if that takes >>> significantly longer (say, more than 8 hours of his effort), perhaps >>> @ldionne or someone else from libc++ will have a moment to step in to help? >> >> Replacing `static_assert` by `(static_assert|static assertion)` should do >> the trick. See the patch attached to this comment, I think it should satisfy >> the CI @Codesbyusman. >> F23872372: static_assert.diff <https://reviews.llvm.org/F23872372> > > I just want to say: I really appreciate the insight you gave into the libc++ > testing in this thread. From the CFE's perspective, our view of precommit is > "it is basically always misconfigured or broken in some way", so knowing that > you guys keep yours running well is nice to hear! We'll be more cognizant in > the future. > > I DO think there IS a sizable difference between patches like this one, and > patches like my deferred-concepts (which, btw, the libc++ tests have been > great for coming up with new tests... though it was a pain to figure out how > to run them in the first place!). My patches DID deserve the reverts it > received so far, so don't see Aaron's mention as an issue with that. > > I appreciate your suggestion/patch file, that IS a much better regex than > we'd come up with, and looks like it should work. So thank you! We'll > commit that once @Codesbyusman can get it integrated into his patch (and > we'll make sure CI passes too!). yes will update soon > Thanks again Louis! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129048/new/ https://reviews.llvm.org/D129048 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
