[RFA] Add support for __VA_OPT__
Commit Message
C++2a adds a "__VA_OPT__" feature that can be used to control the
pesky "," emission when the final (variable) argument of a variadic
macro is empty. This patch implements this feature for gdb. (A patch
to implement it for gcc is pending.)
gdb/ChangeLog
2017-09-17 Tom Tromey <tom@tromey.com>
* macroexp.c (get_next_token_for_substitution): New function.
(substitute_args): Call it. Check for __VA_OPT__.
gdb/testsuite/ChangeLog
2017-09-17 Tom Tromey <tom@tromey.com>
* gdb.base/macscp.exp: Add __VA_OPT__ tests.
---
gdb/ChangeLog | 5 +++
gdb/macroexp.c | 73 +++++++++++++++++++++++++++++++++++----
gdb/testsuite/ChangeLog | 4 +++
gdb/testsuite/gdb.base/macscp.exp | 12 +++++++
4 files changed, 87 insertions(+), 7 deletions(-)
Comments
W00t, thanks for doing this (and the gcc side as well!)
On 09/18/2017 03:42 AM, Tom Tromey wrote:
> diff --git a/gdb/testsuite/gdb.base/macscp.exp b/gdb/testsuite/gdb.base/macscp.exp
> index 54b5ab2..a08ec46 100644
> --- a/gdb/testsuite/gdb.base/macscp.exp
> +++ b/gdb/testsuite/gdb.base/macscp.exp
> @@ -620,6 +620,10 @@ gdb_test_no_output "macro define va_gnu(args...) varfunc (fixedarg, args)" \
> gdb_test_no_output "macro define va2_gnu(args...) varfunc (fixedarg, ## args)" \
> "define fourth varargs helper"
>
> +gdb_test_no_output \
> + "macro define va3_cxx2a(x, ...) varfunc (x __VA_OPT__(,) __VA_ARGS__)" \
> + "define fifth varargs helper"
> +
> gdb_test "macro expand va_c99(one, two, three)" \
> "expands to: *varfunc \\(fixedarg, *one, two, three\\)" \
> "c99 varargs expansion"
> @@ -644,6 +648,14 @@ gdb_test "macro expand va2_gnu()" \
> "expands to: *varfunc \\(fixedarg\\)" \
> "gnu varargs expansion special splicing without an argument"
>
> +gdb_test "macro expand va3_cxx2a(23)" \
> + "expands to: *varfunc \\(23 \\)" \
> + "C++2a __VA_OPT__ handling without variable argument"
> +
> +gdb_test "macro expand va3_cxx2a(23, 24, 25)" \
> + "expands to: *varfunc \\(23, 24, 25\\)" \
> + "C++2a __VA_OPT__ handling with variable argument"
> +
The patch looks good to me, though I think it'd be nice to see
tests that make sure that ill-formed input doesn't send us
to the weeds. Like:
- does the state machine handle "__VA_OPT__)" gracefully?
I.e., ')' before '('.
- similarly: "__VA_OPT__)(,)"
- does the state machine handle multiple occurrences of
__VA_OPT__ in the same macro expansion? It looks like
it, but....
Also, does this handle:
__VA_OPT__(__VA_ARGS__)
correctly? I think so, but...
Thanks,
Pedro Alves
@@ -946,6 +946,22 @@ find_parameter (const struct macro_buffer *tok,
return -1;
}
+/* Helper function for substitute_args that gets the next token and
+ updates the passed-in state variables. */
+
+static int
+get_next_token_for_substitution (struct macro_buffer *replacement_list,
+ struct macro_buffer *token,
+ char **start,
+ struct macro_buffer *lookahead,
+ char **lookahead_start)
+{
+ *token = *lookahead;
+ *start = *lookahead_start;
+ *lookahead_start = replacement_list->text;
+ return get_token (lookahead, replacement_list);
+}
+
/* Given the macro definition DEF, being invoked with the actual
arguments given by ARGC and ARGV, substitute the arguments into the
replacement list, and store the result in DEST.
@@ -996,8 +1012,57 @@ substitute_args (struct macro_buffer *dest,
lookahead_rl_start = replacement_list.text;
lookahead_valid = get_token (&lookahead, &replacement_list);
- for (;;)
+ /* __VA_OPT__ state variable. The states are:
+ 0 - nothing happening
+ 1 - saw __VA_OPT__
+ >= 2 in __VA_OPT__, the value encodes the parenthesis depth. */
+ unsigned vaopt_state = 0;
+
+ for (;;
+ lookahead_valid = get_next_token_for_substitution (&replacement_list,
+ &tok,
+ &original_rl_start,
+ &lookahead,
+ &lookahead_rl_start))
{
+ if (vaopt_state > 0)
+ {
+ if (tok.len == 1)
+ {
+ if (tok.text[0] == '(')
+ {
+ ++vaopt_state;
+ /* We just entered __VA_OPT__, so don't emit this
+ token. */
+ continue;
+ }
+ else if (tok.text[0] == ')')
+ {
+ --vaopt_state;
+ if (vaopt_state == 1)
+ {
+ /* Done with __VA_OPT__. */
+ vaopt_state = 0;
+ /* Don't emit. */
+ continue;
+ }
+ }
+ }
+
+ /* If __VA_ARGS__ is empty, then drop the contents of
+ __VA_OPT__. */
+ if (argv[argc - 1].len == 0)
+ continue;
+ }
+ else if (is_varargs
+ && tok.len == 10
+ && strncmp (tok.text, "__VA_OPT__", 10) == 0)
+ {
+ vaopt_state = 1;
+ /* Don't emit this token. */
+ continue;
+ }
+
/* Just for aesthetics. If we skipped some whitespace, copy
that to DEST. */
if (tok.text > original_rl_start)
@@ -1160,12 +1225,6 @@ substitute_args (struct macro_buffer *dest,
if (! lookahead_valid)
break;
-
- tok = lookahead;
- original_rl_start = lookahead_rl_start;
-
- lookahead_rl_start = replacement_list.text;
- lookahead_valid = get_token (&lookahead, &replacement_list);
}
}
@@ -620,6 +620,10 @@ gdb_test_no_output "macro define va_gnu(args...) varfunc (fixedarg, args)" \
gdb_test_no_output "macro define va2_gnu(args...) varfunc (fixedarg, ## args)" \
"define fourth varargs helper"
+gdb_test_no_output \
+ "macro define va3_cxx2a(x, ...) varfunc (x __VA_OPT__(,) __VA_ARGS__)" \
+ "define fifth varargs helper"
+
gdb_test "macro expand va_c99(one, two, three)" \
"expands to: *varfunc \\(fixedarg, *one, two, three\\)" \
"c99 varargs expansion"
@@ -644,6 +648,14 @@ gdb_test "macro expand va2_gnu()" \
"expands to: *varfunc \\(fixedarg\\)" \
"gnu varargs expansion special splicing without an argument"
+gdb_test "macro expand va3_cxx2a(23)" \
+ "expands to: *varfunc \\(23 \\)" \
+ "C++2a __VA_OPT__ handling without variable argument"
+
+gdb_test "macro expand va3_cxx2a(23, 24, 25)" \
+ "expands to: *varfunc \\(23, 24, 25\\)" \
+ "C++2a __VA_OPT__ handling with variable argument"
+
# Stringification tests.
gdb_test_no_output "macro define str(x) #x" \