rnk added a comment. In D91913#2525993 <https://reviews.llvm.org/D91913#2525993>, @hvdijk wrote:
> @rnk Taking it upon yourself to revert this without approval and without > communication on all branches, especially given the earlier suggestion by > @rsmith to only revert this on the LLVM 12 branch, is an abuse of your commit > privileges as far as I am concerned. This is not a Google project. Google > does not get to unilaterally dictate its direction. I do not intend to get > into a revert-the-revert war and will not revert your revert myself. I do > expect you to do so. I'm sorry you feel that way, but I do believe that I have communicated the issue clearly already. I don't mean to say that Google code is more important than any other code. We can update our code as long as there is a portable conforming way to do it. I think what I did is reasonable, and falls under the situations outlined here: https://llvm.org/docs/CodeReview.html#can-code-be-reviewed-after-it-is-committed This `, ## __VA_ARG__` extension serves a valid use case. The C++20 standard `__VA_OPT__` feature shows that the committee recognizes the validity of this use case. As things stood before the revert, there was no way for a user of this feature to fix their code so that it would compile under -std=c++17, 14, 11, etc. My commit message attempted to outline what was necessary to reland. In D91913#2526171 <https://reviews.llvm.org/D91913#2526171>, @rsmith wrote: > rG0436ec2128c9775ba13b0308937238fc79673fdd > <https://reviews.llvm.org/rG0436ec2128c9775ba13b0308937238fc79673fdd> enables > `__VA_OPT__` across language modes and allows support for it to be detected > by `#ifdef`. Thanks. Do we expect that GCC will want to follow this behavior? I'm concerned that I might have a hard time selling the patch to use `__VA_OPT__` to V8 folks, who support a wide variety of compilers. The patch will not be pretty, since I cannot put an ifdef into the middle of a variadic macro. As an alternative, have you considered allowing the old GNU extension unless C++20 features are enabled? This would make it so that projects encounter this portability issue when they upgrade to C++20 instead of when they upgrade compilers. Maybe this has been considered, I don't have all the context. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91913/new/ https://reviews.llvm.org/D91913 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
