patch/rfc: A better way to do variadic functions

Zack Weinberg zackw@stanford.edu
Mon May 7 10:18:00 GMT 2001


Presently, all functions which take variable arguments have to have a
pair of #ifdefs at the top to deal with the fixed arguments:

void
error VPARAMS ((const char *msgid, ...))
{
#ifndef ANSI_PROTOTYPES
  const char *msgid;
#endif
  va_list ap;
  diagnostic_context dc;

  VA_START (ap, msgid);

#ifndef ANSI_PROTOTYPES
  msgid = va_arg (ap, const char *);
#endif

  /* body of function */

  va_end (ap);
}

This is because the pre-C89 style of varargs didn't provide for any
fixed arguments at all.  They have to be retrieved from the va_list
just like the variable arguments.  We can't move the declaration of
msgid down with its initialization, because C up to C99 doesn't let
you mix declarations and statements, and VA_START is a statement.

In the shower this morning, it dawned on me how to get around that.
This patch creates some new ansidecl.h macros which let us write

void
error VPARAMS ((const char *msgid, ...))
{
  diagnostic_context dc;
  VA_OPEN (ap, msgid);
  VA_FIXEDARG (ap, const char *, msgid);

  /* body of function */

  VA_CLOSE (ap);
}

which is a third the size, easier to read, and clearer as to what's
going on.  You can put the declaration of diagnostic_context after the
VA_FIXEDARG call(s) if you like, too.

We are stuck with duplication of information between the VA_FIXEDARG
sequence and the VPARAMS list.  I don't see any way around that short
of re-introducing DEFUN, which is too much IMO.

The trick: VA_OPEN declares ap, calls va_start on it, then opens a new
block, which will be closed by VA_CLOSE.  That gets us back into a
context where we can legitimately make variable declarations.
VA_FIXEDARG then declares and initializes each fixed argument in K+R
mode, or just makes a dummy declaration in ANSI mode, so that it
remains safe to declare variables after the VA_FIXEDARG sequence
either way.

I converted cpperror.c by way of demonstration.  I've compiled cpp0 on
i686-linux with gcc, and - with a moderate amount of coaxing; some
files touched in stage1 are not K+R clean - make stage1_build
completes successfully on hppa2.0n-hp-hpux11.00 using the bundled K+R
compiler.  I don't have enough disk available to do a full HP/UX bootstrap.

-- 
zw  I am a believer in free will.  If my dog chooses to hate the whole human
    race except myself, it must be free to do so.
    	-- Diana Wynne Jones, _Castle in the Air_

	* ansidecl.h (VA_OPEN, VA_CLOSE, VA_FIXEDARG): New macros for
	more convenient variable argument functions.

	* cpperror.c: Revise all variadic functions in terms of
	VA_OPEN, VA_CLOSE, VA_FIXEDARG.

===================================================================
Index: include/ansidecl.h
--- include/ansidecl.h	2001/04/04 00:46:27	1.9
+++ include/ansidecl.h	2001/05/07 17:07:26
@@ -117,6 +117,13 @@ Foundation, Inc., 59 Temple Place - Suit
 #define VPARAMS(ARGS)			ARGS
 #define VA_START(va_list,var)		va_start(va_list,var)
 
+/* "struct Qdmy" swallows the semicolon after VA_OPEN / VA_FIXEDARG's
+   use without inhibiting further decls and without declaring an
+   actual variable.  */
+#define VA_OPEN(AP,VAR)		va_list AP; VA_START(AP,VAR); { struct Qdmy
+#define VA_CLOSE(AP)		} va_end(AP)
+#define VA_FIXEDARG(AP, TYPE, NAME)	struct Qdmy
+
 /* These are obsolete.  Do not use.  */
 #ifndef IN_GCC
 #define CONST				const
@@ -148,6 +155,10 @@ Foundation, Inc., 59 Temple Place - Suit
 
 #define VPARAMS(ARGS)			(va_alist) va_dcl
 #define VA_START(va_list,var)		va_start(va_list)
+
+#define VA_OPEN(AP,VAR)		va_list AP; VA_START(AP, VAR); { struct Qdmy
+#define VA_CLOSE(AP)		} va_end(AP)
+#define VA_FIXEDARG(AP, TYPE, NAME)	TYPE NAME = va_arg(AP, TYPE)
 
 /* These are obsolete.  Do not use.  */
 #ifndef IN_GCC
===================================================================
Index: gcc/cpperror.c
--- gcc/cpperror.c	2001/04/22 22:33:45	1.47
+++ gcc/cpperror.c	2001/05/07 17:07:26
@@ -34,8 +34,8 @@ static void print_location		PARAMS ((cpp
 						 const char *,
 						 const cpp_lexer_pos *));
 
-/* Don't remove the blank before do, as otherwise the exgettext
-   script will mistake this as a function definition */
+/* Don't remove the blank before do, or the exgettext script will
+   mistake this for a function definition */
 #define v_message(msgid, ap) \
  do { vfprintf (stderr, _(msgid), ap); putc ('\n', stderr); } while (0)
 
@@ -207,23 +207,15 @@ _cpp_begin_message (pfile, code, file, p
 
 void
 cpp_ice VPARAMS ((cpp_reader *pfile, const char *msgid, ...))
-{  
-#ifndef ANSI_PROTOTYPES
-  cpp_reader *pfile;
-  const char *msgid;
-#endif
-  va_list ap;
-  
-  VA_START (ap, msgid);
+{
+  VA_OPEN (ap, msgid);
+  VA_FIXEDARG (ap, cpp_reader *, pfile);
+  VA_FIXEDARG (ap, const char *, msgid);
   
-#ifndef ANSI_PROTOTYPES
-  pfile = va_arg (ap, cpp_reader *);
-  msgid = va_arg (ap, const char *);
-#endif
-
   if (_cpp_begin_message (pfile, ICE, NULL, 0))
     v_message (msgid, ap);
-  va_end(ap);
+
+  VA_CLOSE (ap);
 }
 
 /* Same as cpp_error, except we consider the error to be "fatal",
@@ -235,72 +227,45 @@ cpp_ice VPARAMS ((cpp_reader *pfile, con
 void
 cpp_fatal VPARAMS ((cpp_reader *pfile, const char *msgid, ...))
 {  
-#ifndef ANSI_PROTOTYPES
-  cpp_reader *pfile;
-  const char *msgid;
-#endif
-  va_list ap;
-  
-  VA_START (ap, msgid);
-  
-#ifndef ANSI_PROTOTYPES
-  pfile = va_arg (ap, cpp_reader *);
-  msgid = va_arg (ap, const char *);
-#endif
+  VA_OPEN (ap, msgid);
+  VA_FIXEDARG (ap, cpp_reader *, pfile);
+  VA_FIXEDARG (ap, const char *, msgid);
 
   if (_cpp_begin_message (pfile, FATAL, NULL, 0))
     v_message (msgid, ap);
-  va_end(ap);
+
+  VA_CLOSE (ap);
 }
 
 void
 cpp_error VPARAMS ((cpp_reader * pfile, const char *msgid, ...))
 {
-#ifndef ANSI_PROTOTYPES
-  cpp_reader *pfile;
-  const char *msgid;
-#endif
-  va_list ap;
-
-  VA_START(ap, msgid);
-  
-#ifndef ANSI_PROTOTYPES
-  pfile = va_arg (ap, cpp_reader *);
-  msgid = va_arg (ap, const char *);
-#endif
+  VA_OPEN (ap, msgid);
+  VA_FIXEDARG (ap, cpp_reader *, pfile);
+  VA_FIXEDARG (ap, const char *, msgid);
 
   if (_cpp_begin_message (pfile, ERROR, NULL, 0))
     v_message (msgid, ap);
-  va_end(ap);
+  VA_CLOSE (ap);
 }
 
 void
 cpp_error_with_line VPARAMS ((cpp_reader *pfile, int line, int column,
 			     const char *msgid, ...))
 {
-#ifndef ANSI_PROTOTYPES
-  cpp_reader *pfile;
-  int line;
-  int column;
-  const char *msgid;
-#endif
-  va_list ap;
   cpp_lexer_pos pos;
-  
-  VA_START (ap, msgid);
-  
-#ifndef ANSI_PROTOTYPES
-  pfile = va_arg (ap, cpp_reader *);
-  line = va_arg (ap, int);
-  column = va_arg (ap, int);
-  msgid = va_arg (ap, const char *);
-#endif
+
+  VA_OPEN (ap, msgid);
+  VA_FIXEDARG (ap, cpp_reader *, pfile);
+  VA_FIXEDARG (ap, int, line);
+  VA_FIXEDARG (ap, int, column);
+  VA_FIXEDARG (ap, const char *, msgid);
 
   pos.line = line;
   pos.col = column;
   if (_cpp_begin_message (pfile, ERROR, NULL, &pos))
     v_message (msgid, ap);
-  va_end(ap);
+  VA_CLOSE (ap);
 }
 
 /* Error including a message from `errno'.  */
@@ -315,101 +280,62 @@ cpp_error_from_errno (pfile, name)
 void
 cpp_warning VPARAMS ((cpp_reader * pfile, const char *msgid, ...))
 {
-#ifndef ANSI_PROTOTYPES
-  cpp_reader *pfile;
-  const char *msgid;
-#endif
-  va_list ap;
-  
-  VA_START (ap, msgid);
-  
-#ifndef ANSI_PROTOTYPES
-  pfile = va_arg (ap, cpp_reader *);
-  msgid = va_arg (ap, const char *);
-#endif
+  VA_OPEN (ap, msgid);
+  VA_FIXEDARG (ap, cpp_reader *, pfile);
+  VA_FIXEDARG (ap, const char *, msgid);
 
   if (_cpp_begin_message (pfile, WARNING, NULL, 0))
     v_message (msgid, ap);
-  va_end(ap);
+  VA_CLOSE (ap);
 }
 
 void
 cpp_warning_with_line VPARAMS ((cpp_reader * pfile, int line, int column,
 			       const char *msgid, ...))
 {
-#ifndef ANSI_PROTOTYPES
-  cpp_reader *pfile;
-  int line;
-  int column;
-  const char *msgid;
-#endif
-  va_list ap;
   cpp_lexer_pos pos;
-  
-  VA_START (ap, msgid);
-  
-#ifndef ANSI_PROTOTYPES
-  pfile = va_arg (ap, cpp_reader *);
-  line = va_arg (ap, int);
-  column = va_arg (ap, int);
-  msgid = va_arg (ap, const char *);
-#endif
+  VA_OPEN (ap, msgid);
+  VA_FIXEDARG (ap, cpp_reader *, pfile);
+  VA_FIXEDARG (ap, int, line);
+  VA_FIXEDARG (ap, int, column);
+  VA_FIXEDARG (ap, const char *, msgid);
 
   pos.line = line;
   pos.col = column;
   if (_cpp_begin_message (pfile, WARNING, NULL, &pos))
     v_message (msgid, ap);
-  va_end(ap);
+  VA_CLOSE (ap);
 }
 
 void
 cpp_pedwarn VPARAMS ((cpp_reader * pfile, const char *msgid, ...))
 {
-#ifndef ANSI_PROTOTYPES
-  cpp_reader *pfile;
-  const char *msgid;
-#endif
-  va_list ap;
-  
-  VA_START (ap, msgid);
-  
-#ifndef ANSI_PROTOTYPES
-  pfile = va_arg (ap, cpp_reader *);
-  msgid = va_arg (ap, const char *);
-#endif
+  VA_OPEN (ap, msgid);
+  VA_FIXEDARG (ap, cpp_reader *, pfile);
+  VA_FIXEDARG (ap, const char *, msgid);
 
   if (_cpp_begin_message (pfile, PEDWARN, NULL, 0))
     v_message (msgid, ap);
-  va_end(ap);
+  VA_CLOSE (ap);
 }
 
 void
 cpp_pedwarn_with_line VPARAMS ((cpp_reader * pfile, int line, int column,
 			       const char *msgid, ...))
 {
-#ifndef ANSI_PROTOTYPES
-  cpp_reader *pfile;
-  int line;
-  int column;
-  const char *msgid;
-#endif
-  va_list ap;
   cpp_lexer_pos pos;
-  
-  VA_START (ap, msgid);
-  
-#ifndef ANSI_PROTOTYPES
-  pfile = va_arg (ap, cpp_reader *);
-  line = va_arg (ap, int);
-  column = va_arg (ap, int);
-  msgid = va_arg (ap, const char *);
-#endif
+
+  VA_OPEN (ap, msgid);
+  VA_FIXEDARG (ap, cpp_reader *, pfile);
+  VA_FIXEDARG (ap, int, line);
+  VA_FIXEDARG (ap, int, column);
+  VA_FIXEDARG (ap, const char *, msgid);
 
   pos.line = line;
   pos.col = column;
   if (_cpp_begin_message (pfile, PEDWARN, NULL, &pos))
     v_message (msgid, ap);
-  va_end(ap);
+  VA_CLOSE (ap);
 }
 
 /* Report a warning (or an error if pedantic_errors)
@@ -420,49 +346,29 @@ cpp_pedwarn_with_file_and_line VPARAMS (
 					 const char *file, int line, int col,
 					 const char *msgid, ...))
 {
-#ifndef ANSI_PROTOTYPES
-  cpp_reader *pfile;
-  const char *file;
-  int line;
-  int col;
-  const char *msgid;
-#endif
-  va_list ap;
   cpp_lexer_pos pos;
-  
-  VA_START (ap, msgid);
 
-#ifndef ANSI_PROTOTYPES
-  pfile = va_arg (ap, cpp_reader *);
-  file = va_arg (ap, const char *);
-  line = va_arg (ap, int);
-  col = va_arg (ap, int);
-  msgid = va_arg (ap, const char *);
-#endif
+  VA_OPEN (ap, msgid);
+  VA_FIXEDARG (ap, cpp_reader *, pfile);
+  VA_FIXEDARG (ap, const char *, file);
+  VA_FIXEDARG (ap, int, line);
+  VA_FIXEDARG (ap, int, col);
+  VA_FIXEDARG (ap, const char *, msgid);
 
   pos.line = line;
   pos.col = col;
   if (_cpp_begin_message (pfile, PEDWARN, file, &pos))
     v_message (msgid, ap);
-  va_end(ap);
+  VA_CLOSE (ap);
 }
 
 /* Print an error message not associated with a file.  */
 void
 cpp_notice VPARAMS ((cpp_reader *pfile, const char *msgid, ...))
 {
-#ifndef ANSI_PROTOTYPES
-  cpp_reader *pfile;
-  const char *msgid;
-#endif
-  va_list ap;
-  
-  VA_START (ap, msgid);
-  
-#ifndef ANSI_PROTOTYPES
-  pfile = va_arg (ap, cpp_reader *);
-  msgid = va_arg (ap, const char *);
-#endif
+  VA_OPEN (ap, msgid);
+  VA_FIXEDARG (ap, cpp_reader *, pfile);
+  VA_FIXEDARG (ap, const char *, msgid);
 
   if (pfile->errors < CPP_FATAL_LIMIT)
     pfile->errors++;
@@ -470,7 +376,7 @@ cpp_notice VPARAMS ((cpp_reader *pfile, 
   vfprintf (stderr, _(msgid), ap);
   putc('\n', stderr);
 
-  va_end(ap);
+  VA_CLOSE (ap);
 }
 
 void



More information about the Gcc-patches mailing list