libcpp: Fix up handling of deferred pragmas [PR102432]

Message ID 20211203102727.GO2646553@tucnak
State Committed
Headers
Series libcpp: Fix up handling of deferred pragmas [PR102432] |

Commit Message

Jakub Jelinek Dec. 3, 2021, 10:27 a.m. UTC
  Hi!

The https://gcc.gnu.org/pipermail/gcc-patches/2020-November/557903.html
change broke the following testcases.  The problem is when a pragma
namespace allows expansion (i.e. p->is_nspace && p->allow_expansion),
e.g. the omp or acc namespaces do, then when parsing the second pragma
token we do it with pfile->state.in_directive set,
pfile->state.prevent_expansion clear and pfile->state.in_deferred_pragma
clear (the last one because we don't know yet if it will be a deferred
pragma or not).  If the pragma line only contains a single name
and newline after it, and there exists a function-like macro with the
same name, the preprocessor needs to peek in funlike_invocation_p
the next token whether it isn't ( but in this case it will see a newline.
As pfile->state.in_directive is set, we don't read anything after the
newline, pfile->buffer->need_line is set and CPP_EOF is lexed, which
funlike_invocation_p doesn't push back.  Because name is a function-like
macro and on the pragma line there is no ( after the name, it isn't
expanded, and control flow returns to do_pragma.  If name is valid
deferred pragma, we set pfile->state.in_deferred_pragma (and really
need it set so that e.g. end_directive later on doesn't eat all the
tokens from the pragma line).

Before Nathan's change (which unfortunately didn't contain rationale
on why it is better to do it like that), this wasn't a problem,
next _cpp_lex_direct called when we want next token would return
CPP_PRAGMA_EOF when it saw buffer->need_line, which would turn off
pfile->state.in_deferred_pragma and following get token would already
read the next line.  But Nathan's patch replaced it with an assertion
failure that now triggers and CPP_PRAGMA_EOL is done only when lexing
the '\n'.  Except for this special case that works fine, but in
this case it doesn't because when peeking the token we still didn't know
that it will be a deferred pragma.
I've tried to fix that up in do_pragma by detecting this and pushing
CPP_PRAGMA_EOL as lookahead, but that doesn't work because end_directive
still needs to see pfile->state.in_deferred_pragma set.

So, this patch affectively reverts part of Nathan's change, CPP_PRAGMA_EOL
addition isn't done only when parsing the '\n', but is now done in both
places, in the first one instead of the assertion failure.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2021-12-03  Jakub Jelinek  <jakub@redhat.com>

	PR preprocessor/102432
	* lex.c (_cpp_lex_direct): If buffer->need_line while
	pfile->state.in_deferred_pragma, return CPP_PRAGMA_EOL token instead
	of assertion failure.

	* c-c++-common/gomp/pr102432.c: New test.
	* c-c++-common/goacc/pr102432.c: New test.


	Jakub
  

Comments

Marek Polacek Dec. 3, 2021, 11:04 p.m. UTC | #1
On Fri, Dec 03, 2021 at 11:27:27AM +0100, Jakub Jelinek wrote:
> Hi!
> 
> The https://gcc.gnu.org/pipermail/gcc-patches/2020-November/557903.html
> change broke the following testcases.  The problem is when a pragma
> namespace allows expansion (i.e. p->is_nspace && p->allow_expansion),
> e.g. the omp or acc namespaces do, then when parsing the second pragma
> token we do it with pfile->state.in_directive set,
> pfile->state.prevent_expansion clear and pfile->state.in_deferred_pragma
> clear (the last one because we don't know yet if it will be a deferred
> pragma or not).  If the pragma line only contains a single name
> and newline after it, and there exists a function-like macro with the
> same name, the preprocessor needs to peek in funlike_invocation_p
> the next token whether it isn't ( but in this case it will see a newline.
> As pfile->state.in_directive is set, we don't read anything after the
> newline, pfile->buffer->need_line is set and CPP_EOF is lexed, which
> funlike_invocation_p doesn't push back.  Because name is a function-like
> macro and on the pragma line there is no ( after the name, it isn't
> expanded, and control flow returns to do_pragma.  If name is valid
> deferred pragma, we set pfile->state.in_deferred_pragma (and really
> need it set so that e.g. end_directive later on doesn't eat all the
> tokens from the pragma line).
> 
> Before Nathan's change (which unfortunately didn't contain rationale
> on why it is better to do it like that), this wasn't a problem,
> next _cpp_lex_direct called when we want next token would return
> CPP_PRAGMA_EOF when it saw buffer->need_line, which would turn off
> pfile->state.in_deferred_pragma and following get token would already
> read the next line.  But Nathan's patch replaced it with an assertion
> failure that now triggers and CPP_PRAGMA_EOL is done only when lexing
> the '\n'.  Except for this special case that works fine, but in
> this case it doesn't because when peeking the token we still didn't know
> that it will be a deferred pragma.
> I've tried to fix that up in do_pragma by detecting this and pushing
> CPP_PRAGMA_EOL as lookahead, but that doesn't work because end_directive
> still needs to see pfile->state.in_deferred_pragma set.
> 
> So, this patch affectively reverts part of Nathan's change, CPP_PRAGMA_EOL
> addition isn't done only when parsing the '\n', but is now done in both
> places, in the first one instead of the assertion failure.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK, thanks.

Marek
  

Patch

--- libcpp/lex.c.jj	2021-12-01 10:21:16.808869062 +0100
+++ libcpp/lex.c	2021-12-02 10:38:23.010621589 +0100
@@ -3530,7 +3530,21 @@  _cpp_lex_direct (cpp_reader *pfile)
   buffer = pfile->buffer;
   if (buffer->need_line)
     {
-      gcc_assert (!pfile->state.in_deferred_pragma);
+      if (pfile->state.in_deferred_pragma)
+	{
+	  /* This can happen in cases like:
+	     #define loop(x) whatever
+	     #pragma omp loop
+	     where when trying to expand loop we need to peek
+	     next token after loop, but aren't still in_deferred_pragma
+	     mode but are in in_directive mode, so buffer->need_line
+	     is set, a CPP_EOF is peeked.  */
+	  result->type = CPP_PRAGMA_EOL;
+	  pfile->state.in_deferred_pragma = false;
+	  if (!pfile->state.pragma_allow_expansion)
+	    pfile->state.prevent_expansion--;
+	  return result;
+	}
       if (!_cpp_get_fresh_line (pfile))
 	{
 	  result->type = CPP_EOF;
--- gcc/testsuite/c-c++-common/gomp/pr102432.c.jj	2021-12-02 10:42:23.627138897 +0100
+++ gcc/testsuite/c-c++-common/gomp/pr102432.c	2021-12-02 10:41:54.163565353 +0100
@@ -0,0 +1,23 @@ 
+/* PR preprocessor/102432 */
+
+#define loop(x)
+
+void
+foo (void)
+{
+  int i;
+#pragma omp parallel
+#pragma omp loop
+  for (i = 0; i < 64; i++)
+    ;
+}
+
+void
+bar (void)
+{
+  int i;
+  _Pragma ("omp parallel")
+  _Pragma ("omp loop")
+  for (i = 0; i < 64; i++)
+    ;
+}
--- gcc/testsuite/c-c++-common/goacc/pr102432.c.jj	2021-12-02 10:43:10.663458092 +0100
+++ gcc/testsuite/c-c++-common/goacc/pr102432.c	2021-12-02 10:43:24.791253606 +0100
@@ -0,0 +1,23 @@ 
+/* PR preprocessor/102432 */
+
+#define loop(x)
+
+void
+foo (void)
+{
+  int i;
+#pragma acc parallel
+#pragma acc loop
+  for (i = 0; i < 64; i++)
+    ;
+}
+
+void
+bar (void)
+{
+  int i;
+  _Pragma ("acc parallel")
+  _Pragma ("acc loop")
+  for (i = 0; i < 64; i++)
+    ;
+}