Mordante marked 6 inline comments as done. Mordante added a comment. Thanks for all feedback!
================ Comment at: clang/www/hacking.html:311-312 + <p>Unlike Clang, libc++ supports multiple versions of Clang. Therefore when a + patch changes the diagnostics it might be required to use a regex in the + "expected" tests to make it pass the CI.</p> + ---------------- aaron.ballman wrote: > ldionne wrote: > > Mordante wrote: > > > Mordante wrote: > > > > philnik wrote: > > > > > aaron.ballman wrote: > > > > > > Should we document the expectation that breaking libc++ due to > > > > > > conforming changes in Clang (in terms of diagnostics and bug fixes, > > > > > > not so much in terms of introducing new language extensions) are > > > > > > generally the responsibility of libc++ maintainers to address, but > > > > > > Clang contributors should attempt to reduce disruptions as much as > > > > > > possible by collaborating with libc++ maintainers when this > > > > > > situation comes up? > > > > > That's definitely a good idea. Maybe mention that clang contributors > > > > > should ping the #libc group to get the attention of libc++ > > > > > contributors so we can prepare a follow-up patch or, if the author is > > > > > comfortable with it, also fix libc++ in the same patch (and hopefully > > > > > get fast approval). Most breakages should be relatively simple to fix. > > > > I like the idea to collaborate more. > > > > > > > > I want to give this some thought and especially how to word it. I want > > > > to avoid that it's seen as a carte blanche to commit Clang changes > > > > which break libc++. This would mean HEAD is broken and that's a huge > > > > issue for downstream users like Google who tend to follow HEAD closely. > > > > > > > > I would prefer to use commandeering and instead of two patches where > > > > one leave HEAD in a broken state. Even if it's short it would make > > > > bi-secting harder. Personally I really dislike the idea of having > > > > commits that knowingly leave a repository in a broken state. It also > > > > doesn't align with the LLVM developer policy > > > > https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy > > > > ``` > > > > As a community, we strongly value having the tip of tree in a good > > > > state while allowing rapid iterative development. > > > > ``` > > > > > > > > Should we document the expectation that breaking libc++ due to > > > > conforming changes in Clang (in terms of diagnostics and bug fixes, not > > > > so much in terms of introducing new language extensions) are generally > > > > the responsibility of libc++ maintainers to address, but Clang > > > > contributors should attempt to reduce disruptions as much as possible > > > > by collaborating with libc++ maintainers when this situation comes up? > > > > > > @aaron.ballman I have given this some thought. I think it would be best > > > to discuss this with some libc++ and Clang contributors/code owners. For > > > libc++ at least @ldionne should be present since he's the libc++ code > > > owner. I would like to be present too. After we have agreement we can > > > update this documentation accordingly. > > I remember discussing this with @aaron.ballman. > > > > I am 100% on-board with the notion that if Clang makes a legitimate > > conforming change and it breaks libc++ (most likely because we are doing > > something wrong, UB, or relying on Clang details), then it is *primarily* > > libc++'s responsibility to fix this ASAP. I also remember that we agreed it > > should be a collaborative effort, i.e. the Clang folks should try to help > > us understand the failure, and should be understanding if the fix on our > > side is really non-trivial. But, the bottom line is that if they change > > something and it breaks us because we're doing something wrong, it's our > > responsibility to fix our stuff ASAP to get the CI green again. > > > > This is the same situation that we sometimes have with the LLDB data > > formatters, and I think the dynamics need to be the same there as well. If > > we break that because of a 100% legitimate change in libc++, our > > expectation is that they'll fix it ASAP, and their expectation is that > > we'll provide support as needed. > > > > I think it would be useful to document that, since it technically departs > > from LLVM's usual policy of "revert when it breaks anything at all". And > > more generally speaking, I strongly think that this policy needs to be > > revisited, however that's beyond the scope of this documentation > > improvement. > > I am 100% on-board with the notion that if Clang makes a legitimate > > conforming change and it breaks libc++ (most likely because we are doing > > something wrong, UB, or relying on Clang details), then it is *primarily* > > libc++'s responsibility to fix this ASAP. I also remember that we agreed it > > should be a collaborative effort, i.e. the Clang folks should try to help > > us understand the failure, and should be understanding if the fix on our > > side is really non-trivial. But, the bottom line is that if they change > > something and it breaks us because we're doing something wrong, it's our > > responsibility to fix our stuff ASAP to get the CI green again. > > Thanks! I think a succinct way to put it is: I think of libc++ as a > downstream consumer of Clang. If we break a downstream, we want to fix it > ASAP, but if the break is because of a conforming change, we don't want to > revert the changes in Clang. Communication between Clang and libc++ > developers will help us get to the bottom of any such problem to find a good > solution. > > > I think it would be useful to document that, since it technically departs > > from LLVM's usual policy of "revert when it breaks anything at all". And > > more generally speaking, I strongly think that this policy needs to be > > revisited, however that's beyond the scope of this documentation > > improvement. > > +1 to all of this. It seems this patch slipped under the radar, sorry about that. I wouldn't mind to change the policy but it would be good discuss with Clang and libc++ developers what the expectations are. I really prefer to *not* break the CI, instead I would like to collaborate on patches so we have one patch that makes the Clang *and* libc++ changes. Ideally I would like to do the same for LLDB. I actually like the LLVM revert policy, especially since the reverter has obligations too. But that would be better to discuss on Discourse. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133249/new/ https://reviews.llvm.org/D133249 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
