[PATCH] D91913: Suppress non-conforming GNU paste extension in all standard-conforming modes

Reid Kleckner via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jan 27 13:16:15 PST 2021


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



More information about the cfe-commits mailing list