This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: Varargs macros subtly broken


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 -----

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]