Varargs macros subtly broken
Neil Booth
NeilB@earthling.net
Mon Sep 25 16:32:00 GMT 2000
Neil Booth wrote:-
> > Observe: gnu_count expansions differ.
>
> Yes, this is a bug with identifying empty varargs (the VOID_REST
> flag). I'm bootstrapping a patch to fix this at the moment; I've got
> rid of the VOID_REST stuff as it was too ugly to fix, and am trying
> something else instead.
Here's a fix. It's doesn't fix the deeply nested varargs mess, but
plugs the counting bug. Macro expansion needs to be simplified, but
really needs token lookahead first.
Bootstraps and passes CPP tests.
Thanks,
Neil.
P.S. I've just noticed that the system header diagnostic patch that
just went in broke cpplex.c. I think I've fixed it, but I may have
missed something.
* cpplex.c (parse_args): Don't set VOID_REST flag.
(fix diagnostic patch breakage)
(CONTEXT_VARARGS): New flag.
(maybe_paste_with_next): Set context earlier in loop. Use
it. Do varargs test with CONTEXT_VARARGS flag.
(push_arg_context): Set CONTEXT_VARARGS flag if we're
pushing an argument context for a varargs argument.
* cpplib.h (VOID_REST): Delete.
* gcc.dg/cpp/vararg1.c: Add test case.
Index: cpplex.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/cpplex.c,v
retrieving revision 1.103
diff -c -p -r1.103 cpplex.c
*** cpplex.c 2000/09/25 22:39:51 1.103
--- cpplex.c 2000/09/25 23:06:58
*************** static const cpp_token eof_token = {0, 0
*** 52,57 ****
--- 52,58 ----
#define CONTEXT_PASTER (1 << 1) /* An argument context on RHS of ##. */
#define CONTEXT_RAW (1 << 2) /* If argument tokens already expanded. */
#define CONTEXT_ARG (1 << 3) /* If an argument context. */
+ #define CONTEXT_VARARGS (1 << 4) /* If a varargs argument context. */
typedef struct cpp_context cpp_context;
struct cpp_context
*************** parse_args (pfile, hp, args)
*** 1990,1998 ****
{
/* Duplicate the placemarker. Then we can set its flags and
position and safely be using more than one. */
! cpp_token *pm = duplicate_token (pfile, &placemarker_token);
! pm->flags = VOID_REST;
! save_token (args, pm);
args->ends[argc] = total + 1;
if (CPP_OPTION (pfile, c99) && CPP_PEDANTIC (pfile))
--- 1991,1997 ----
{
/* Duplicate the placemarker. Then we can set its flags and
position and safely be using more than one. */
! save_token (args, duplicate_token (pfile, &placemarker_token));
args->ends[argc] = total + 1;
if (CPP_OPTION (pfile, c99) && CPP_PEDANTIC (pfile))
*************** maybe_paste_with_next (pfile, token)
*** 2302,2307 ****
--- 2301,2307 ----
pfile->paste_level = pfile->cur_context;
second = _cpp_get_token (pfile);
pfile->paste_level = 0;
+ context = CURRENT_CONTEXT (pfile);
/* Ignore placemarker argument tokens (cannot be from an empty
macro since macros are not expanded). */
*************** maybe_paste_with_next (pfile, token)
*** 2313,2319 ****
a varargs parameter: the comma disappears if b was given
no actual arguments (not merely if b is an empty
argument). */
! if (token->type == CPP_COMMA && second->flags & VOID_REST)
pasted = duplicate_token (pfile, second);
else
pasted = duplicate_token (pfile, token);
--- 2313,2319 ----
a varargs parameter: the comma disappears if b was given
no actual arguments (not merely if b is an empty
argument). */
! if (token->type == CPP_COMMA && (context->flags & CONTEXT_VARARGS))
pasted = duplicate_token (pfile, second);
else
pasted = duplicate_token (pfile, token);
*************** maybe_paste_with_next (pfile, token)
*** 2331,2339 ****
<whatever> came from a variable argument, because
the author probably intended the ## to trigger
the special extended semantics (see above). */
! if (token->type == CPP_COMMA
! && IS_ARG_CONTEXT (CURRENT_CONTEXT (pfile))
! && ON_REST_ARG (CURRENT_CONTEXT (pfile) - 1))
/* no warning */;
else
cpp_warning (pfile,
--- 2331,2338 ----
<whatever> came from a variable argument, because
the author probably intended the ## to trigger
the special extended semantics (see above). */
! if (token->type == CPP_COMMA && IS_ARG_CONTEXT (context)
! && ON_REST_ARG (context - 1))
/* no warning */;
else
cpp_warning (pfile,
*************** maybe_paste_with_next (pfile, token)
*** 2397,2403 ****
/* See if there is another token to be pasted onto the one we just
constructed. */
token = pasted;
- context = CURRENT_CONTEXT (pfile);
/* and loop */
}
return token;
--- 2396,2401 ----
*************** push_arg_context (pfile, token)
*** 2578,2583 ****
--- 2576,2584 ----
context->posn = 0;
context->level = args->level;
context->flags = CONTEXT_ARG | CONTEXT_RAW;
+ if ((context[-1].u.list->flags & VAR_ARGS)
+ && token->val.aux + 1 == (unsigned) context[-1].u.list->paramc)
+ context->flags |= CONTEXT_VARARGS;
context->pushed_token = 0;
/* Set the flags of the first token. There is one. */
Index: cpplib.h
===================================================================
RCS file: /cvs/gcc/egcs/gcc/cpplib.h,v
retrieving revision 1.124
diff -c -p -r1.124 cpplib.h
*** cpplib.h 2000/09/24 10:42:09 1.124
--- cpplib.h 2000/09/25 23:07:11
*************** struct cpp_string
*** 160,166 ****
#define PASTE_LEFT (1 << 4) /* If on LHS of a ## operator. */
#define PASTED (1 << 5) /* The result of a ## operator. */
#define NAMED_OP (1 << 6) /* C++ named operators, also "defined". */
- #define VOID_REST (1 << 7) /* When a rest arg gets zero actual args. */
/* A preprocessing token. This has been carefully packed and should
occupy 16 bytes on 32-bit hosts and 24 bytes on 64-bit hosts. */
--- 160,165 ----
Index: testsuite/gcc.dg/cpp/vararg1.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/testsuite/gcc.dg/cpp/vararg1.c,v
retrieving revision 1.1
diff -c -p -r1.1 vararg1.c
*** vararg1.c 2000/07/18 23:25:06 1.1
--- vararg1.c 2000/09/25 23:07:20
***************
*** 1,11 ****
/* Test for changed behavior of the GNU varargs extension.
##args, where args is a rest argument which received zero tokens,
used to delete the previous sequence of nonwhitespace characters.
Now it deletes the previous token. */
- /* { dg-do run } */
- /* { dg-options -w } */
-
#include <string.h>
#define S(str, args...) " " str "\n", ##args
--- 1,19 ----
+ /* { dg-do run } */
+ /* { dg-options -w } */
+
+ /* count() used to give 1 owing to a buggy test for varargs. */
+ #define count(y...) count1 ( , ##y)
+ #define count1(y...) count2 (y,1,0)
+ #define count2(_,x0,n,y...) n
+ #if count() != 0 || count(A) != 1
+ #error Incorrect vararg argument counts
+ #endif
+
/* Test for changed behavior of the GNU varargs extension.
##args, where args is a rest argument which received zero tokens,
used to delete the previous sequence of nonwhitespace characters.
Now it deletes the previous token. */
#include <string.h>
#define S(str, args...) " " str "\n", ##args
*************** main()
*** 16,19 ****
const char *s = S("foo");
return strchr (s, '\n') == NULL;
}
-
--- 24,26 ----
----- End forwarded message -----
More information about the Gcc-patches
mailing list