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]

cpplib: Tweak conditional block skipping


Currently cpplib doesn't "accept" the newline at the end of a
directive until the directive has finished being processed.  This is
to catch various issues like unterminated macro args at EOL, and to
avoid having to keep a read-ahead token lying around.

I'm starting to work towards describing the location in a _translation
unit_ as a 32-bit bitfield, with 20 bits for the translation unit (not
file) line and 10 for the column.  This will make various things
simpler, and allow us to cram a complete unambiguous source location
into an int.  In turn, this should allow me to improve the macro
expansion algorithm, and have cpplib return pointers to tokens rather
than taking a pointer to a token structure to fill in.

However, being unable to read the newline at EOL before finishing the
directive makes this change difficult, because line numbers are kind
of in limbo.  It is also the cause of some hacks.  For example, we
don't finish a #include line until the included file is finished
(which causes issues, see the PR where two one-line -included files on
the command line can end up on the same line of output).

Fixing that in turn is made difficult because whether we are
conditional block skipping or not is turned off when lexing a
directive, and only updated when a directive has finished.

So, I've removed that limitation with this patch, which actually makes
things a little more readable too, IMO.  I also took the opportunity
to move the skipping boolean to the lexer state structure.

I also fixed this (pedantic) standards compliance issue:

#if 0
#if <anything>
#else bar    /* Should not warn.  */
#endif bar   /* Should not warn.  */
#endif bar   /* Should warn.  */

The standard explicity states that the only thing we care about in
skipped conditional blocks is the names of directives.

Bootstrapped and made check x86 Linux.

Neil.

	* cpphash.h (struct_lexer_state): Delete was_skipping.
	Move skipping here from struct cpp_reader.
	* cpplex.c (parse_identifier): Update.
	(_cpp_lex_token): Don't skip tokens in a directive.
	* cpplib.c (struct if_stack): Update.
	(start_directive, end_directive): Don't change skipping state.
	(_cpp_handle_directive): Update.
	(do_ifdef, do_ifndef, do_if, do_elif): Similarly.
	(do_else, do_endif): Update; only check for excess tokens if not
	in a skipped conditional block.
	(push_conditional): Update for new struct if_stack.

	* gcc.dg/cpp/extratokens.c: Fix.
	* gcc.dg/cpp/skipping2.c: New tests.

Index: cpphash.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cpphash.h,v
retrieving revision 1.105
diff -u -p -r1.105 cpphash.h
--- cpphash.h	2001/05/26 01:31:33	1.105
+++ cpphash.h	2001/07/25 21:32:41
@@ -126,6 +126,9 @@ struct lexer_state
   /* Nonzero if first token on line is CPP_HASH.  */
   unsigned char in_directive;
 
+  /* True if we are skipping a failed conditional group.  */
+  unsigned char skipping;
+
   /* Nonzero if in a directive that takes angle-bracketed headers.  */
   unsigned char angled_headers;
 
@@ -216,9 +219,6 @@ struct cpp_buffer
      buffers.  */
   unsigned char from_stage3;
 
-  /* Temporary storage for pfile->skipping whilst in a directive.  */
-  unsigned char was_skipping;
-
   /* 1 = system header file, 2 = C system header file used for C++.  */
   unsigned char sysp;
 
@@ -341,9 +341,6 @@ struct cpp_reader
 
   /* We're printed a warning recommending against using #import.  */
   unsigned char import_warning;
-
-  /* True if we are skipping a failed conditional group.  */
-  unsigned char skipping;
 
   /* Whether to print our version number.  Done this way so
      we don't get it twice for -v -version.  */
Index: cpplex.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cpplex.c,v
retrieving revision 1.145
diff -u -p -r1.145 cpplex.c
--- cpplex.c	2001/05/30 21:22:00	1.145
+++ cpplex.c	2001/07/25 21:32:46
@@ -509,7 +509,7 @@ parse_identifier (pfile, c)
   /* $ is not a identifier character in the standard, but is commonly
      accepted as an extension.  Don't warn about it in skipped
      conditional blocks.  */
-  if (saw_dollar && CPP_PEDANTIC (pfile) && ! pfile->skipping)
+  if (saw_dollar && CPP_PEDANTIC (pfile) && ! pfile->state.skipping)
     cpp_pedwarn (pfile, "'$' character(s) in identifier");
 
   /* Identifiers are null-terminated.  */
@@ -521,7 +521,7 @@ parse_identifier (pfile, c)
     ht_lookup (pfile->hash_table, obstack_finish (stack), len, HT_ALLOCED);
 
   /* Some identifiers require diagnostics when lexed.  */
-  if (result->flags & NODE_DIAGNOSTIC && !pfile->skipping)
+  if (result->flags & NODE_DIAGNOSTIC && !pfile->state.skipping)
     {
       /* It is allowed to poison the same identifier twice.  */
       if ((result->flags & NODE_POISONED) && !pfile->state.poisoned_ok)
@@ -888,7 +888,7 @@ _cpp_lex_token (pfile, result)
       if (pfile->lexer_pos.col != 0 && !bol && !buffer->from_stage3)
 	cpp_pedwarn (pfile, "no newline at end of file");
       pfile->state.next_bol = 1;
-      pfile->skipping = 0;	/* In case missing #endif.  */
+      pfile->state.skipping = 0;	/* In case missing #endif.  */
       result->type = CPP_EOF;
       /* Don't do MI optimisation.  */
       return;
@@ -915,7 +915,7 @@ _cpp_lex_token (pfile, result)
       buffer->read_ahead = c;
       pfile->state.next_bol = 1;
       result->type = CPP_EOF;
-      /* Don't break; pfile->skipping might be true.  */
+      /* Don't break; pfile->state.skipping might be true.  */
       return;
 
     case '?':
@@ -1261,7 +1261,7 @@ _cpp_lex_token (pfile, result)
       break;
     }
 
-  if (pfile->skipping)
+  if (!pfile->state.in_directive && pfile->state.skipping)
     goto skip;
 
   /* If not in a directive, this token invalidates controlling macros.  */
Index: cpplib.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cpplib.c,v
retrieving revision 1.254
diff -u -p -r1.254 cpplib.c
--- cpplib.c	2001/07/01 18:48:13	1.254
+++ cpplib.c	2001/07/25 21:32:50
@@ -43,8 +43,9 @@ struct if_stack
   struct if_stack *next;
   cpp_lexer_pos pos;		/* line and column where condition started */
   const cpp_hashnode *mi_cmacro;/* macro name for #ifndef around entire file */
-  unsigned char was_skipping;	/* Value of pfile->skipping before this if.  */
-  int type;			/* type of last directive seen in this group */
+  bool skip_elses;		/* Can future #else / #elif be skipped?  */
+  bool was_skipping;		/* If were skipping on entry.  */
+  int type;			/* Most recent conditional, for diagnostics.  */
 };
 
 /* Values for the origin field of struct directive.  KANDR directives
@@ -220,8 +221,6 @@ static void
 start_directive (pfile)
      cpp_reader *pfile;
 {
-  cpp_buffer *buffer = pfile->buffer;
-
   /* Setup in-directive state.  */
   pfile->state.in_directive = 1;
   pfile->state.save_comments = 0;
@@ -232,10 +231,6 @@ start_directive (pfile)
   /* Don't save directive tokens for external clients.  */
   pfile->la_saved = pfile->la_write;
   pfile->la_write = 0;
-
-  /* Turn off skipping.  */
-  buffer->was_skipping = pfile->skipping;
-  pfile->skipping = 0;
 }
 
 /* Called when leaving a directive, _Pragma or command-line directive.  */
@@ -244,12 +239,6 @@ end_directive (pfile, skip_line)
      cpp_reader *pfile;
      int skip_line;
 {
-  cpp_buffer *buffer = pfile->buffer;
-
-  /* Restore pfile->skipping before skip_rest_of_line, so that e.g.
-     __VA_ARGS__ in the rest of the directive doesn't warn.  */
-  pfile->skipping = buffer->was_skipping;
-
   /* We don't skip for an assembler #.  */
   if (skip_line)
     skip_rest_of_line (pfile);
@@ -270,7 +259,6 @@ _cpp_handle_directive (pfile, indented)
      cpp_reader *pfile;
      int indented;
 {
-  cpp_buffer *buffer = pfile->buffer;
   const directive *dir = 0;
   cpp_token dname;
   int skip = 1;
@@ -293,7 +281,7 @@ _cpp_handle_directive (pfile, indented)
 	 skipped conditional groups.  Complain about this form if
 	 we're being pedantic, but not if this is regurgitated input
 	 (preprocessed or fed back in by the C++ frontend).  */
-      if (! buffer->was_skipping && CPP_OPTION (pfile, lang) != CLK_ASM)
+      if (! pfile->state.skipping && CPP_OPTION (pfile, lang) != CLK_ASM)
 	{
 	  dir = &dtable[T_LINE];
 	  pfile->state.line_extension = 1;
@@ -361,7 +349,7 @@ _cpp_handle_directive (pfile, indented)
 
 	  /* If we are skipping a failed conditional group, all
 	     non-conditional directives are ignored.  */
-	  if (! buffer->was_skipping || (dir->flags & COND))
+	  if (! pfile->state.skipping || (dir->flags & COND))
 	    {
 	      /* Issue -pedantic warnings for extensions.   */
 	      if (CPP_PEDANTIC (pfile) && dir->origin == EXTENSION)
@@ -376,7 +364,7 @@ _cpp_handle_directive (pfile, indented)
 	    }
 	}
     }
-  else if (dname.type != CPP_EOF && ! buffer->was_skipping)
+  else if (dname.type != CPP_EOF && ! pfile->state.skipping)
     {
       /* An unknown directive.  Don't complain about it in assembly
 	 source: we don't know where the comments are, and # may
@@ -1256,7 +1244,7 @@ do_ifdef (pfile)
 {
   int skip = 1;
 
-  if (! pfile->buffer->was_skipping)
+  if (! pfile->state.skipping)
     {
       const cpp_hashnode *node = lex_macro_node (pfile);
 
@@ -1277,7 +1265,7 @@ do_ifndef (pfile)
   int skip = 1;
   const cpp_hashnode *node = 0;
 
-  if (! pfile->buffer->was_skipping)
+  if (! pfile->state.skipping)
     {
       node = lex_macro_node (pfile);
       if (node)
@@ -1302,7 +1290,7 @@ do_if (pfile)
   int skip = 1;
   const cpp_hashnode *cmacro = 0;
 
-  if (! pfile->buffer->was_skipping)
+  if (! pfile->state.skipping)
     {
       /* Controlling macro of #if ! defined ()  */
       pfile->mi_ind_cmacro = 0;
@@ -1336,16 +1324,17 @@ do_else (pfile)
 	}
       ifs->type = T_ELSE;
 
-      /* Buffer->was_skipping is 1 if all conditionals in this chain
-	 have been false, 2 if a conditional has been true.  */
-      if (! ifs->was_skipping && buffer->was_skipping != 2)
-	buffer->was_skipping = ! buffer->was_skipping;
+      /* Skip any future (erroneous) #elses or #elifs.  */
+      pfile->state.skipping = ifs->skip_elses;
+      ifs->skip_elses = true;
 
       /* Invalidate any controlling macro.  */
       ifs->mi_cmacro = 0;
-    }
 
-  check_eol (pfile);
+      /* Only check EOL if was not originally skipping.  */
+      if (!ifs->was_skipping)
+	check_eol (pfile);
+    }
 }
 
 /* handle a #elif directive by not changing if_stack either.  see the
@@ -1370,23 +1359,23 @@ do_elif (pfile)
 	}
       ifs->type = T_ELIF;
 
-      /* Don't evaluate #elif if our higher level is skipping.  */
-      if (! ifs->was_skipping)
+      /* Only evaluate this if we aren't skipping elses.  During
+	 evaluation, set skipping to false to get lexer warnings.  */
+      if (ifs->skip_elses)
+	pfile->state.skipping = 1;
+      else
 	{
-	  /* Buffer->was_skipping is 1 if all conditionals in this
-	     chain have been false, 2 if a conditional has been true.  */
-	  if (buffer->was_skipping == 1)
-	    buffer->was_skipping = ! _cpp_parse_expr (pfile);
-	  else
-	    buffer->was_skipping = 2;
-
-	  /* Invalidate any controlling macro.  */
-	  ifs->mi_cmacro = 0;
+	  pfile->state.skipping = 0;
+	  pfile->state.skipping = ! _cpp_parse_expr (pfile);
+	  ifs->skip_elses = ! pfile->state.skipping;
 	}
+
+      /* Invalidate any controlling macro.  */
+      ifs->mi_cmacro = 0;
     }
 }
 
-/* #endif pops the if stack and resets pfile->skipping.  */
+/* #endif pops the if stack and resets pfile->state.skipping.  */
 
 static void
 do_endif (pfile)
@@ -1399,6 +1388,10 @@ do_endif (pfile)
     cpp_error (pfile, "#endif without #if");
   else
     {
+      /* Only check EOL if was not originally skipping.  */
+      if (!ifs->was_skipping)
+	check_eol (pfile);
+
       /* If potential control macro, we go back outside again.  */
       if (ifs->next == 0 && ifs->mi_cmacro)
 	{
@@ -1407,14 +1400,12 @@ do_endif (pfile)
 	}
 
       buffer->if_stack = ifs->next;
-      buffer->was_skipping = ifs->was_skipping;
+      pfile->state.skipping = ifs->was_skipping;
       obstack_free (&pfile->buffer_ob, ifs);
     }
-
-  check_eol (pfile);
 }
 
-/* Push an if_stack entry and set pfile->skipping accordingly.
+/* Push an if_stack entry and set pfile->state.skipping accordingly.
    If this is a #ifndef starting at the beginning of a file,
    CMACRO is the macro name tested by the #ifndef.  */
 
@@ -1431,14 +1422,15 @@ push_conditional (pfile, skip, type, cma
   ifs = xobnew (&pfile->buffer_ob, struct if_stack);
   ifs->pos = pfile->directive_pos;
   ifs->next = buffer->if_stack;
-  ifs->was_skipping = buffer->was_skipping;
+  ifs->skip_elses = pfile->state.skipping || !skip;
+  ifs->was_skipping = pfile->state.skipping;
   ifs->type = type;
   if (pfile->mi_state == MI_OUTSIDE && pfile->mi_cmacro == 0)
     ifs->mi_cmacro = cmacro;
   else
     ifs->mi_cmacro = 0;
 
-  buffer->was_skipping = skip;
+  pfile->state.skipping = skip;
   buffer->if_stack = ifs;
 }
 
Index: gcc/testsuite/gcc.dg/cpp/skipping2.c
===================================================================
RCS file: skipping2.c
diff -N skipping2.c
--- /dev/null	Tue May  5 13:32:27 1998
+++ skipping2.c	Wed Jul 25 15:21:38 2001
@@ -0,0 +1,14 @@
+/* Copyright (C) 2001 Free Software Foundation, Inc.  */
+
+/* { dg-do preprocess } */
+
+/* Tests that excess tokens in skipped conditional blocks don't warn.  */
+
+/* Source: Neil Booth, 25 Jul 2001.  */
+
+#if 0
+#if foo
+#else foo   /* { dg-bogus "extra tokens" "extra tokens in skipped block" } */
+#endif foo  /* { dg-bogus "extra tokens" "extra tokens in skipped block" } */
+#endif bar  /* { dg-warning "extra tokens" "tokens after #endif" } */
+
Index: gcc/testsuite/gcc.dg/cpp/extratokens.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/testsuite/gcc.dg/cpp/extratokens.c,v
retrieving revision 1.2
diff -u -p -r1.2 extratokens.c
--- extratokens.c	2000/12/07 23:21:09	1.2
+++ extratokens.c	2001/07/25 23:10:27
@@ -15,7 +15,7 @@
 #ifndef foo bar  /* { dg-error "extra tokens" "tokens after #ifndef" } */
 #endif
 
-#if 0
+#if 1
 #if 0
 #else foo	/* { dg-warning "extra tokens" "tokens after #else" } */
 #endif /	/* { dg-warning "extra tokens" "tokens after #endif" } */
Index: gcc/testsuite/gcc.dg/cpp/skipping2.c
===================================================================
RCS file: skipping2.c
diff -N skipping2.c
--- /dev/null	Tue May  5 13:32:27 1998
+++ skipping2.c	Wed Jul 25 16:10:27 2001
@@ -0,0 +1,14 @@
+/* Copyright (C) 2001 Free Software Foundation, Inc.  */
+
+/* { dg-do preprocess } */
+
+/* Tests that excess tokens in skipped conditional blocks don't warn.  */
+
+/* Source: Neil Booth, 25 Jul 2001.  */
+
+#if 0
+#if foo
+#else foo   /* { dg-bogus "extra tokens" "extra tokens in skipped block" } */
+#endif foo  /* { dg-bogus "extra tokens" "extra tokens in skipped block" } */
+#endif bar  /* { dg-warning "extra tokens" "tokens after #endif" } */
+


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