Re: [RFA] Implement __VA_OPT__
On Sun, Nov 12, 2017 at 2:33 AM, Tom Tromey wrote: > I'm still [not] too happy with the error message, so if you have any > suggestions there, please let me know. I removed the "C99" branch from > the earlier error message as well, since this isn't a C feature at all. > Again, please check the wording. I'm ambivalent about mentioning the standard level. And I find the use of "expansion" in this and the existing __VA_ARGS__ error odd, as expansion is what you *do* with a macro; I'd prefer "definition". But I suppose we might as well be consistent with the existing error. The patch is OK, thanks. Jason
Re: [RFA] Implement __VA_OPT__
> "Jason" == Jason Merrill writes:
Jason> Do we want 1s for vaopt in the GNU rows, then? It seems to only be
Jason> used for controlling the pedwarn about needing at least one argument
Jason> for the variadic parameter.
It seems reasonable to me, but I wasn't sure. I've made the change.
>> +maybe_va_opt_error (cpp_reader *pfile, cpp_hashnode *node)
Jason> Do we also want to look at the va_opt option in this function, to
Jason> complain if pedantic and it isn't set?
Thanks for noticing that. I also made it suppress the error for system
headers.
I'm still too happy with the error message, so if you have any
suggestions there, please let me know. I removed the "C99" branch from
the earlier error message as well, since this isn't a C feature at all.
Again, please check the wording.
thanks,
Tom
commit f163032782a1b5ed05ff3a3bab5a2e76c203d124
Author: Tom Tromey
Date: Thu Sep 14 12:41:51 2017 -0600
Implement __VA_OPT__
This implements __VA_OPT__, a new preprocessor feature added in C++2A.
The paper can be found here:
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2017/p0306r4.html
I am not completely sure that I have handled the error reporting
correctly. I chose to allow __VA_OPT__ generally, on the theory that,
because it is a reserved identifier, programs should not be referring
to it anyway. Also, this approach makes it possible for __VA_OPT__ to
be used in system headers even when the -std/-pedantic settings may
otherwise disallow it.
Bootstrapped and regression tested on x86-64 Fedora 25.
gcc/ChangeLog
2017-09-16 Tom Tromey
* doc/cpp.texi (Variadic Macros): Document __VA_OPT__.
gcc/testsuite/ChangeLog
2017-11-11 Tom Tromey
* c-c++-common/cpp/va-opt-pedantic.c: New file.
* c-c++-common/cpp/va-opt.c: New file.
* c-c++-common/cpp/va-opt-error.c: New file.
libcpp/ChangeLog
2017-09-16 Tom Tromey
* pch.c (cpp_read_state): Set n__VA_OPT__.
* macro.c (vaopt_state): New class.
(_cpp_arguments_ok): Check va_opt flag.
(replace_args, create_iso_definition): Use vaopt_state.
* lex.c (lex_identifier_intern): Possibly issue errors for
__VA_OPT__.
(lex_identifier): Likewise.
(maybe_va_opt_error): New function.
* internal.h (struct lexer_state) : Update comment.
(struct spec_nodes) : New field.
* init.c (struct lang_flags) : New field.
(lang_defaults): Add entries for C++2A. Update all entries for
va_opt.
(cpp_set_lang): Initialize va_opt.
* include/cpplib.h (struct cpp_options) : New field.
* identifiers.c (_cpp_init_hashtable): Initialize n__VA_OPT__.
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index e213db6e2a0..362f50ecb98 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,7 @@
+2017-09-16 Tom Tromey
+
+ * doc/cpp.texi (Variadic Macros): Document __VA_OPT__.
+
2017-09-16 Richard Sandiford
PR tree-optimization/82228
diff --git a/gcc/doc/cpp.texi b/gcc/doc/cpp.texi
index 52f2606eadc..33e72c63f59 100644
--- a/gcc/doc/cpp.texi
+++ b/gcc/doc/cpp.texi
@@ -1675,20 +1675,27 @@ macro. We could define @code{eprintf} like this,
instead:
@end smallexample
@noindent
-This formulation looks more descriptive, but unfortunately it is less
-flexible: you must now supply at least one argument after the format
-string. In standard C, you cannot omit the comma separating the named
-argument from the variable arguments. Furthermore, if you leave the
-variable argument empty, you will get a syntax error, because
-there will be an extra comma after the format string.
+This formulation looks more descriptive, but historically it was less
+flexible: you had to supply at least one argument after the format
+string. In standard C, you could not omit the comma separating the
+named argument from the variable arguments. (Note that this
+restriction has been lifted in C++2a, and never existed in GNU C; see
+below.)
+
+Furthermore, if you left the variable argument empty, you would have
+gotten a syntax error, because there would have been an extra comma
+after the format string.
@smallexample
eprintf("success!\n", );
@expansion{} fprintf(stderr, "success!\n", );
@end smallexample
-GNU CPP has a pair of extensions which deal with this problem. First,
-you are allowed to leave the variable argument out entirely:
+This has been fixed in C++2a, and GNU CPP also has a pair of
+extensions which deal with this problem.
+
+First, in GNU CPP, and in C++ beginning in C++2a, you are allowed to
+leave the variable argument out entirely:
@smallexample
eprintf ("success!\n")
@@ -1696,8 +1703,24 @@ eprintf ("success!\n")
@end smallexample
@noindent
-Second, the @samp{##} token paste operator has a special meaning wh
Re: [RFA] Implement __VA_OPT__
On 09/17/2017 11:44 AM, Tom Tromey wrote:
+@code{@w{__VA_OPT__}} is also available in GNU C and GNU C++.
+{ /* c99 c++ xnum xid c11 std digr ulit rlit udlit bincst digsep
trig u8chlit vaopt */
+ /* GNUC89 */ { 0, 0, 1, 0, 0, 0, 1, 0, 0, 0,0, 0,
0, 0, 0 },
+ /* GNUC99 */ { 1, 0, 1, 1, 0, 0, 1, 1, 1, 0,0, 0,
0, 0, 0 },
+ /* GNUC11 */ { 1, 0, 1, 1, 1, 0, 1, 1, 1, 0,0, 0,
0, 0, 0 },
[...]
Do we want 1s for vaopt in the GNU rows, then? It seems to only be used
for controlling the pedwarn about needing at least one argument for the
variadic parameter.
+maybe_va_opt_error (cpp_reader *pfile, cpp_hashnode *node)
Do we also want to look at the va_opt option in this function, to
complain if pedantic and it isn't set?
Jason
Re: [RFA] Implement __VA_OPT__
Tom> [ __VA_OPT__ ] Tom> Here's v3. Tom> Ping. Tom> Ping #2. Ping #3. Tom
Re: [RFA] Implement __VA_OPT__
> "Tom" == Tom Tromey writes: Tom> [ __VA_OPT__ ] Tom> Here's v3. Tom> Ping. Ping #2. Tom
Re: [RFA] Implement __VA_OPT__
> "Tom" == Tom Tromey writes: [ __VA_OPT__ ] Tom> Here's v3. Ping. Tom
Re: [RFA] Implement __VA_OPT__
And, darn it, I forgot to save cpp.texi, leaving out a couple of tweaks
there.
Here's v3. Sorry about the noise.
Tom
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index e213db6..362f50e 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,7 @@
+2017-09-16 Tom Tromey
+
+ * doc/cpp.texi (Variadic Macros): Document __VA_OPT__.
+
2017-09-16 Richard Sandiford
PR tree-optimization/82228
diff --git a/gcc/doc/cpp.texi b/gcc/doc/cpp.texi
index 52f2606..33e72c6 100644
--- a/gcc/doc/cpp.texi
+++ b/gcc/doc/cpp.texi
@@ -1675,20 +1675,27 @@ macro. We could define @code{eprintf} like this,
instead:
@end smallexample
@noindent
-This formulation looks more descriptive, but unfortunately it is less
-flexible: you must now supply at least one argument after the format
-string. In standard C, you cannot omit the comma separating the named
-argument from the variable arguments. Furthermore, if you leave the
-variable argument empty, you will get a syntax error, because
-there will be an extra comma after the format string.
+This formulation looks more descriptive, but historically it was less
+flexible: you had to supply at least one argument after the format
+string. In standard C, you could not omit the comma separating the
+named argument from the variable arguments. (Note that this
+restriction has been lifted in C++2a, and never existed in GNU C; see
+below.)
+
+Furthermore, if you left the variable argument empty, you would have
+gotten a syntax error, because there would have been an extra comma
+after the format string.
@smallexample
eprintf("success!\n", );
@expansion{} fprintf(stderr, "success!\n", );
@end smallexample
-GNU CPP has a pair of extensions which deal with this problem. First,
-you are allowed to leave the variable argument out entirely:
+This has been fixed in C++2a, and GNU CPP also has a pair of
+extensions which deal with this problem.
+
+First, in GNU CPP, and in C++ beginning in C++2a, you are allowed to
+leave the variable argument out entirely:
@smallexample
eprintf ("success!\n")
@@ -1696,8 +1703,24 @@ eprintf ("success!\n")
@end smallexample
@noindent
-Second, the @samp{##} token paste operator has a special meaning when
-placed between a comma and a variable argument. If you write
+Second, C++2a introduces the @code{@w{__VA_OPT__}} function macro.
+This macro may only appear in the definition of a variadic macro. If
+the variable argument has any tokens, then a @code{@w{__VA_OPT__}}
+invocation expands to its argument; but if the variable argument does
+not have any tokens, the @code{@w{__VA_OPT__}} expands to nothing:
+
+@smallexample
+#define eprintf(format, @dots{}) \\
+ fprintf (stderr, format __VA_OPT__(,) __VA_ARGS__)
+@end smallexample
+
+@code{@w{__VA_OPT__}} is also available in GNU C and GNU C++.
+
+Historically, GNU CPP has also had another extension to handle the
+trailing comma: the @samp{##} token paste operator has a special
+meaning when placed between a comma and a variable argument. Despite
+the introduction of @code{@w{__VA_OPT__}}, this extension remains
+supported in GNU CPP, for backward compatibility. If you write
@smallexample
#define eprintf(format, @dots{}) fprintf (stderr, format, ##__VA_ARGS__)
@@ -1730,6 +1753,9 @@ of macro. It may also be forbidden in open text; the
standard is
ambiguous. We recommend you avoid using it except for its defined
purpose.
+Likewise, C++ forbids @code{@w{__VA_OPT__}} anywhere outside the
+replacement list of a variadic macro.
+
Variadic macros became a standard part of the C language with C99.
GNU CPP previously supported them
with a named variable argument
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 1a94535..2538c4b 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2017-09-16 Tom Tromey
+
+ * c-c++-common/cpp/va-opt.c: New file.
+ * c-c++-common/cpp/va-opt-error.c: New file.
+
2017-09-15 Andrew Sutton
Jakub Jelinek
diff --git a/gcc/testsuite/c-c++-common/cpp/va-opt-error.c
b/gcc/testsuite/c-c++-common/cpp/va-opt-error.c
new file mode 100644
index 000..7718916
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/cpp/va-opt-error.c
@@ -0,0 +1,28 @@
+/* { dg-do preprocess }*/
+/* { dg-options "-std=gnu99" { target c } } */
+/* { dg-options "-std=c++2a" { target c++ } } */
+
+#define ERR1(x) __VA_OPT__ /* { dg-error "__VA_OPT__ can only appear" } */
+#define ERR2(x) __VA_OPT__( /* { dg-error "can only appear" } */
+#define ERR3(x) __VA_OPT__() /* { dg-error "can only appear" } */
+
+#define ERR4(x,...) __VA_OPT__ /* { dg-error "unterminated __VA_OPT__" } */
+#define ERR5(x,...) __VA_OPT__( /* { dg-error "unterminated" } */
+#define ERR6(x,...) __VA_OPT__(() /* { dg-error "unterminated" } */
+
+#define ERR7(x,...) __VA_OPT__(__VA_OPT__) /* { dg-error "may not appear" } */
+#define ERR7(x,...) __VA_OPT__(__VA_OPT__()) /* { dg-error "may not appear" }
*/
+
+#define ERR8
Re: [RFA] Implement __VA_OPT__
Tom> I don't really understand the rationale for why the errors are phrased
Tom> the way they are, but I notice the C errors generally mention C99 and
Tom> the C++ errors generally mention C++11. So, since I didn't have a
Tom> rationale, I copied what is already there. I thought maybe GCC is just
Tom> emitting the default standard choice for each language.
Here's an updated version of the patch. I've tried to address your
comments (except the one about the text of the error message, see above)
and also some comments I got off-list.
thanks,
Tom
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index e213db6..362f50e 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,7 @@
+2017-09-16 Tom Tromey
+
+ * doc/cpp.texi (Variadic Macros): Document __VA_OPT__.
+
2017-09-16 Richard Sandiford
PR tree-optimization/82228
diff --git a/gcc/doc/cpp.texi b/gcc/doc/cpp.texi
index 52f2606..5647d26f 100644
--- a/gcc/doc/cpp.texi
+++ b/gcc/doc/cpp.texi
@@ -1675,20 +1675,27 @@ macro. We could define @code{eprintf} like this,
instead:
@end smallexample
@noindent
-This formulation looks more descriptive, but unfortunately it is less
-flexible: you must now supply at least one argument after the format
-string. In standard C, you cannot omit the comma separating the named
-argument from the variable arguments. Furthermore, if you leave the
-variable argument empty, you will get a syntax error, because
-there will be an extra comma after the format string.
+This formulation looks more descriptive, but historically it was less
+flexible: you had to supply at least one argument after the format
+string. In standard C, you could not omit the comma separating the
+named argument from the variable arguments. (Note that this
+restriction has been lifted in C++2a, and never existed in GNU C; see
+below.)
+
+Furthermore, if you left the variable argument empty, you would have
+gotten a syntax error, because there would have been an extra comma
+after the format string.
@smallexample
eprintf("success!\n", );
@expansion{} fprintf(stderr, "success!\n", );
@end smallexample
-GNU CPP has a pair of extensions which deal with this problem. First,
-you are allowed to leave the variable argument out entirely:
+This has been fixed in C++2a, and GNU CPP also has a pair of
+extensions which deal with this problem.
+
+First, in GNU CPP, and in C++ beginning in C++2a, you are allowed to
+leave the variable argument out entirely:
@smallexample
eprintf ("success!\n")
@@ -1696,8 +1703,24 @@ eprintf ("success!\n")
@end smallexample
@noindent
-Second, the @samp{##} token paste operator has a special meaning when
-placed between a comma and a variable argument. If you write
+Second, C++2a introduces the @code{@w{__VA_OPT__}} function macro.
+This macro may only appear in the definition of a variadic macro. If
+the variable argument has any tokens, then a @code{@w{__VA_OPT__}}
+invocation expands to its argument; but if the variable argument does
+not have any tokens, the @code{@w{__VA_OPT__}} expands to nothing:
+
+@smallexample
+#define eprintf(format, @dots{}) \\
+ fprintf (stderr, format __VA_OPT__(,) __VA_ARGS__)
+@end smallexample
+
+@code{@w{__VA_OPT__}} is also available in GNU C and GNU C++.
+
+Historically, GNU has also had another extension to handle the
+trailing comma: the @samp{##} token paste operator has a special
+meaning when placed between a comma and a variable argument. Despite
+the introduction of @code{@w{__VA_OPT__}}, this extension remains
+supported in GNU CPP, for backward compatibility. If you write
@smallexample
#define eprintf(format, @dots{}) fprintf (stderr, format, ##__VA_ARGS__)
@@ -1730,6 +1753,9 @@ of macro. It may also be forbidden in open text; the
standard is
ambiguous. We recommend you avoid using it except for its defined
purpose.
+Likewise, C++ forbids @code{@w{__VA_OPT__}} anywhere outside the
+replacement list of a variadic macro.
+
Variadic macros became a standard part of the C language with C99.
GNU CPP previously supported them
with a named variable argument
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 1a94535..2538c4b 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2017-09-16 Tom Tromey
+
+ * c-c++-common/cpp/va-opt.c: New file.
+ * c-c++-common/cpp/va-opt-error.c: New file.
+
2017-09-15 Andrew Sutton
Jakub Jelinek
diff --git a/gcc/testsuite/c-c++-common/cpp/va-opt-error.c
b/gcc/testsuite/c-c++-common/cpp/va-opt-error.c
new file mode 100644
index 000..7718916
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/cpp/va-opt-error.c
@@ -0,0 +1,28 @@
+/* { dg-do preprocess }*/
+/* { dg-options "-std=gnu99" { target c } } */
+/* { dg-options "-std=c++2a" { target c++ } } */
+
+#define ERR1(x) __VA_OPT__ /* { dg-error "__VA_OPT__ can only appear" } */
+#define ERR2(x) __VA_OPT__( /* { dg-error "can only appear" } */
+#define ERR3(x) __VA_OPT__()
Re: [RFA] Implement __VA_OPT__
> "Alexander" == Alexander Monakov writes: Alexander> This hunk reverts CXX17 back to CXX1Z. Thanks for noticing, I'd written this before Jakub's patch and so the error came in during the rebase. Alexander> These two hunks add more duplication in already-duplicated 'if' statement Alexander> bodies. Is it possible to move the whole body to a separate function? I suppose, but this is just copying what pre-existing code does. Alexander> I guess the references to C++11 and C99 are duplicated from the preceding 'if', Alexander> but is that correct here? And why is it useful to distinguish between C and C++ Alexander> for this error in the first place, why not simply say 'cpp_error (..., Alexander> "__VA_OPT__ can only appear in the expansion of a variadic macro");'? I don't really understand the rationale for why the errors are phrased the way they are, but I notice the C errors generally mention C99 and the C++ errors generally mention C++11. So, since I didn't have a rationale, I copied what is already there. I thought maybe GCC is just emitting the default standard choice for each language. Tom
Re: [RFA] Implement __VA_OPT__
On Sat, 16 Sep 2017, Tom Tromey wrote:
> --- a/gcc/doc/cpp.texi
> +++ b/gcc/doc/cpp.texi
> @@ -1675,20 +1675,27 @@ macro. We could define @code{eprintf} like this,
> instead:
[snip]
> +This formulation looks more descriptive, but historically it was less
> +flexible: you had to supply at least one argument after the format
> +string. In standard C, you could not omit the comma separating the
> +named argument from the variable arguments. (Note that this
> +restriction has been lifted in C++20, and never existed in GNU C; see
> +below.)
Shouldn't this say 'C++2a' (more instances follow)?
> +Historically, GNU has also had another extension to handle the
'GNU CPP'?
> --- a/libcpp/init.c
> +++ b/libcpp/init.c
> @@ -91,28 +91,29 @@ struct lang_flags
> static const struct lang_flags lang_defaults[] =
> -{ /* c99 c++ xnum xid c11 std digr ulit rlit udlit bincst
> digsep trig u8chlit */
[snip]
> - /* GNUCXX17 */ { 1, 1, 1, 1, 1, 0, 1, 1, 1, 1,1, 1,
> 0, 1 },
> - /* CXX17*/ { 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,1, 1,
> 0, 1 },
> - /* GNUCXX2A */ { 1, 1, 1, 1, 1, 0, 1, 1, 1, 1,1, 1,
> 0, 1 },
> - /* CXX2A*/ { 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,1, 1,
> 0, 1 },
> - /* ASM */ { 0, 0, 1, 0, 0, 0, 0, 0, 0, 0,0, 0,
> 0, 0 }
> +{ /* c99 c++ xnum xid c11 std digr ulit rlit udlit bincst
> digsep trig u8chlit vaopt */
[snip]
> + /* GNUCXX1Z */ { 1, 1, 1, 1, 1, 0, 1, 1, 1, 1,1, 1,
> 0, 1, 0 },
> + /* CXX1Z*/ { 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,1, 1,
> 0, 1, 0 },
> + /* GNUCXX2A */ { 1, 1, 1, 1, 1, 0, 1, 1, 1, 1,1, 1,
> 0, 1, 1 },
> + /* CXX2A*/ { 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,1, 1,
> 0, 1, 1 },
> + /* ASM */ { 0, 0, 1, 0, 0, 0, 0, 0, 0, 0,0, 0,
> 0, 0, 0 }
> };
This hunk reverts CXX17 back to CXX1Z.
> --- a/libcpp/lex.c
> +++ b/libcpp/lex.c
> @@ -1396,6 +1396,21 @@ lex_identifier_intern (cpp_reader *pfile, const uchar
> *base)
> " of a C99 variadic macro");
> }
>
> + /* __VA_OPT__ should only appear in the replacement list of a
> + variadic macro. */
> + if (result == pfile->spec_nodes.n__VA_OPT__
> + && !pfile->state.va_args_ok)
> + {
> + if (CPP_OPTION (pfile, cplusplus))
> + cpp_error (pfile, CPP_DL_ERROR,
> +"__VA_OPT__ can only appear in the expansion"
> +" of a C++11 variadic macro");
> + else
> + cpp_error (pfile, CPP_DL_ERROR,
> +"__VA_OPT__ can only appear in the expansion"
> +" of a C99 variadic macro");
> + }
> +
>/* For -Wc++-compat, warn about use of C++ named operators. */
>if (result->flags & NODE_WARN_OPERATOR)
> cpp_warning (pfile, CPP_W_CXX_OPERATOR_NAMES,
> @@ -1485,6 +1500,21 @@ lex_identifier (cpp_reader *pfile, const uchar *base,
> bool starts_ucn,
> " of a C99 variadic macro");
> }
>
> + /* __VA_OPT__ should only appear in the replacement list of a
> + variadic macro. */
> + if (result == pfile->spec_nodes.n__VA_OPT__
> + && !pfile->state.va_args_ok)
> + {
> + if (CPP_OPTION (pfile, cplusplus))
> + cpp_error (pfile, CPP_DL_ERROR,
> +"__VA_OPT__ can only appear in the expansion"
> +" of a C++11 variadic macro");
> + else
> + cpp_error (pfile, CPP_DL_ERROR,
> +"__VA_OPT__ can only appear in the expansion"
> +" of a C99 variadic macro");
> + }
> +
>/* For -Wc++-compat, warn about use of C++ named operators. */
>if (result->flags & NODE_WARN_OPERATOR)
> cpp_warning (pfile, CPP_W_CXX_OPERATOR_NAMES,
These two hunks add more duplication in already-duplicated 'if' statement
bodies. Is it possible to move the whole body to a separate function?
I guess the references to C++11 and C99 are duplicated from the preceding 'if',
but is that correct here? And why is it useful to distinguish between C and C++
for this error in the first place, why not simply say 'cpp_error (...,
"__VA_OPT__ can only appear in the expansion of a variadic macro");'?
> --- a/libcpp/macro.c
> +++ b/libcpp/macro.c
> @@ -1670,6 +1821,8 @@ replace_args (cpp_reader *pfile, cpp_hashnode *node,
> cpp_macro *macro,
>num_macro_tokens);
> }
>i = 0;
> + vaopt_state state_tracker (pfile, macro->variadic,
> + args[macro->paramc - 1].count > 0);
The name 'state_tracker' seems too general for what it does, I think
'vaopt_st' or such would have been a better fit (sorry for the bikeshed).
Thanks.
Alexander
