public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Fix __has_{cpp_}attribute with -traditional-cpp (PR preprocessor/65238)
@ 2015-03-11 19:11 Jakub Jelinek
  2015-03-19 16:54 ` Dodji Seketeli
  2015-03-20 16:30 ` Jason Merrill
  0 siblings, 2 replies; 8+ messages in thread
From: Jakub Jelinek @ 2015-03-11 19:11 UTC (permalink / raw)
  To: Joseph S. Myers, Marek Polacek, Jason Merrill, Dodji Seketeli; +Cc: gcc-patches

Hi!

__has_{cpp_,}attribute builtin macros are effectively function-like macros
taking one argument (and the ISO preprocessor expands macros in the argument
which is IMHO desirable), but the traditional preprocessor has been crashing
on them or reporting errors.
As the hook uses cpp_get_token and thus the ISO preprocessor, we need to set
up things such that the argument and ()s around it are already preprocessed
and ready to be reparsed by the ISO preprocessor (this is similar to how
e.g. #if/#elif and various other directives are handled).

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

2015-03-11  Jakub Jelinek  <jakub@redhat.com>

	PR preprocessor/65238
	* internal.h (_cpp_scan_out_logical_line): Add third argument.
	* directives.c (prepare_directive_trad): Pass false to it.
	* traditional.c (_cpp_read_logical_line_trad,
	_cpp_create_trad_definition): Likewise.
	(struct fun_macro): Add paramc field.
	(fun_like_macro): New function.
	(maybe_start_funlike): Handle NODE_BUILTIN macros.  Initialize
	macro->paramc field.
	(save_argument): Use macro->paramc instead of
	macro->node->value.macro->paramc.
	(push_replacement_text): Formatting fix.
	(recursive_macro): Use fun_like_macro helper.
	(_cpp_scan_out_logical_line): Likewise.  Add BUILTIN_MACRO_ARG
	argument.  Initialize fmacro.paramc field.  Handle builtin
	function-like macros.

	* c-c++-common/cpp/pr65238-1.c: New test.
	* gcc.dg/cpp/pr65238-2.c: New test.
	* gcc.dg/cpp/trad/pr65238-3.c: New test.
	* gcc.dg/cpp/trad/pr65238-4.c: New test.

--- libcpp/internal.h.jj	2015-02-03 10:33:59.000000000 +0100
+++ libcpp/internal.h	2015-03-11 14:11:13.410854083 +0100
@@ -708,7 +708,7 @@ extern void _cpp_preprocess_dir_only (cp
 				      const struct _cpp_dir_only_callbacks *);
 
 /* In traditional.c.  */
-extern bool _cpp_scan_out_logical_line (cpp_reader *, cpp_macro *);
+extern bool _cpp_scan_out_logical_line (cpp_reader *, cpp_macro *, bool);
 extern bool _cpp_read_logical_line_trad (cpp_reader *);
 extern void _cpp_overlay_buffer (cpp_reader *pfile, const unsigned char *,
 				 size_t);
--- libcpp/directives.c.jj	2015-01-23 19:18:20.000000000 +0100
+++ libcpp/directives.c	2015-03-11 14:11:34.742511193 +0100
@@ -346,7 +346,7 @@ prepare_directive_trad (cpp_reader *pfil
 
       if (no_expand)
 	pfile->state.prevent_expansion++;
-      _cpp_scan_out_logical_line (pfile, NULL);
+      _cpp_scan_out_logical_line (pfile, NULL, false);
       if (no_expand)
 	pfile->state.prevent_expansion--;
 
--- libcpp/traditional.c.jj	2015-03-10 16:37:11.418949471 +0100
+++ libcpp/traditional.c	2015-03-11 16:19:05.598475497 +0100
@@ -62,6 +62,9 @@ struct fun_macro
   /* The line the macro name appeared on.  */
   source_location line;
 
+  /* Number of parameters.  */
+  unsigned int paramc;
+
   /* Zero-based index of argument being currently lexed.  */
   unsigned int argc;
 };
@@ -304,24 +307,41 @@ _cpp_read_logical_line_trad (cpp_reader
       if (pfile->buffer->need_line && !_cpp_get_fresh_line (pfile))
 	return false;
     }
-  while (!_cpp_scan_out_logical_line (pfile, NULL) || pfile->state.skipping);
+  while (!_cpp_scan_out_logical_line (pfile, NULL, false)
+	 || pfile->state.skipping);
 
   return pfile->buffer != NULL;
 }
 
+/* Return true if NODE is a fun_like macro.  */
+static inline bool
+fun_like_macro (cpp_hashnode *node)
+{
+  if (node->flags & NODE_BUILTIN)
+    return node->value.builtin == BT_HAS_ATTRIBUTE;
+  else
+    return node->value.macro->fun_like;
+}
+
 /* Set up state for finding the opening '(' of a function-like
    macro.  */
 static void
-maybe_start_funlike (cpp_reader *pfile, cpp_hashnode *node, const uchar *start, struct fun_macro *macro)
+maybe_start_funlike (cpp_reader *pfile, cpp_hashnode *node, const uchar *start,
+		     struct fun_macro *macro)
 {
-  unsigned int n = node->value.macro->paramc + 1;
+  unsigned int n;
+  if (node->flags & NODE_BUILTIN)
+    n = 1;
+  else
+    n = node->value.macro->paramc;
 
   if (macro->buff)
     _cpp_release_buff (pfile, macro->buff);
-  macro->buff = _cpp_get_buff (pfile, n * sizeof (size_t));
+  macro->buff = _cpp_get_buff (pfile, (n + 1) * sizeof (size_t));
   macro->args = (size_t *) BUFF_FRONT (macro->buff);
   macro->node = node;
   macro->offset = start - pfile->out.base;
+  macro->paramc = n;
   macro->argc = 0;
 }
 
@@ -330,7 +350,7 @@ static void
 save_argument (struct fun_macro *macro, size_t offset)
 {
   macro->argc++;
-  if (macro->argc <= macro->node->value.macro->paramc)
+  if (macro->argc <= macro->paramc)
     macro->args[macro->argc] = offset;
 }
 
@@ -340,9 +360,13 @@ save_argument (struct fun_macro *macro,
 
    If MACRO is non-NULL, then we are scanning the replacement list of
    MACRO, and we call save_replacement_text() every time we meet an
-   argument.  */
+   argument.
+
+   If BUILTIN_MACRO_ARG is true, this is called to macro expand
+   arguments of builtin function-like macros.  */
 bool
-_cpp_scan_out_logical_line (cpp_reader *pfile, cpp_macro *macro)
+_cpp_scan_out_logical_line (cpp_reader *pfile, cpp_macro *macro,
+			    bool builtin_macro_arg)
 {
   bool result = true;
   cpp_context *context;
@@ -359,14 +383,18 @@ _cpp_scan_out_logical_line (cpp_reader *
   fmacro.node = NULL;
   fmacro.offset = 0;
   fmacro.line = 0;
+  fmacro.paramc = 0;
   fmacro.argc = 0;
 
   quote = 0;
   header_ok = pfile->state.angled_headers;
   CUR (pfile->context) = pfile->buffer->cur;
   RLIMIT (pfile->context) = pfile->buffer->rlimit;
-  pfile->out.cur = pfile->out.base;
-  pfile->out.first_line = pfile->line_table->highest_line;
+  if (!builtin_macro_arg)
+    {
+      pfile->out.cur = pfile->out.base;
+      pfile->out.first_line = pfile->line_table->highest_line;
+    }
   /* start_of_input_line is needed to make sure that directives really,
      really start at the first character of the line.  */
   start_of_input_line = pfile->buffer->cur;
@@ -379,6 +407,7 @@ _cpp_scan_out_logical_line (cpp_reader *
   for (;;)
     {
       if (!context->prev
+	  && !builtin_macro_arg
 	  && cur >= pfile->buffer->notes[pfile->buffer->cur_note].pos)
 	{
 	  pfile->buffer->cur = cur;
@@ -410,6 +439,8 @@ _cpp_scan_out_logical_line (cpp_reader *
 	  /* Omit the newline from the output buffer.  */
 	  pfile->out.cur = out - 1;
 	  pfile->buffer->cur = cur;
+	  if (builtin_macro_arg)
+	    goto done;
 	  pfile->buffer->need_line = true;
 	  CPP_INCREMENT_LINE (pfile, 0);
 
@@ -489,8 +520,7 @@ _cpp_scan_out_logical_line (cpp_reader *
 		{
 		  /* Macros invalidate MI optimization.  */
 		  pfile->mi_valid = false;
-		  if (! (node->flags & NODE_BUILTIN)
-		      && node->value.macro->fun_like)
+		  if (fun_like_macro (node))
 		    {
 		      maybe_start_funlike (pfile, node, out_start, &fmacro);
 		      lex_state = ls_fun_open;
@@ -572,6 +602,91 @@ _cpp_scan_out_logical_line (cpp_reader *
 	      paren_depth--;
 	      if (lex_state == ls_fun_close && paren_depth == 0)
 		{
+		  if (fmacro.node->flags & NODE_BUILTIN)
+		    {
+		      /* Handle builtin function-like macros like
+			 __has_attribute.  The already parsed arguments
+			 are put into a buffer, which is then preprocessed
+			 and the result is fed to _cpp_push_text_context
+			 with disabled expansion, where the ISO preprocessor
+			 parses it.  */
+		      lex_state = ls_none;
+		      save_argument (&fmacro, out - pfile->out.base);
+		      cpp_macro m;
+		      memset (&m, '\0', sizeof (m));
+		      m.paramc = fmacro.paramc;
+		      if (_cpp_arguments_ok (pfile, &m, fmacro.node,
+					     fmacro.argc))
+			{
+			  size_t len = fmacro.args[1] - fmacro.args[0];
+			  uchar *buf;
+
+			  /* Remove the macro's invocation from the
+			     output, and push its replacement text.  */
+			  pfile->out.cur = pfile->out.base + fmacro.offset;
+			  CUR (context) = cur;
+			  buf = _cpp_unaligned_alloc (pfile, len + 2);
+			  buf[0] = '(';
+			  memcpy (buf + 1, pfile->out.base + fmacro.args[0],
+				  len);
+			  buf[len + 1] = '\n';
+
+			  const unsigned char *ctx_rlimit = RLIMIT (context);
+			  const unsigned char *saved_cur = pfile->buffer->cur;
+			  const unsigned char *saved_rlimit
+			    = pfile->buffer->rlimit;
+			  const unsigned char *saved_line_base
+			    = pfile->buffer->line_base;
+			  bool saved_need_line = pfile->buffer->need_line;
+			  cpp_buffer *saved_overlaid_buffer
+			    = pfile->overlaid_buffer;
+			  pfile->buffer->cur = buf;
+			  pfile->buffer->line_base = buf;
+			  pfile->buffer->rlimit = buf + len + 1;
+			  pfile->buffer->need_line = false;
+			  pfile->overlaid_buffer = pfile->buffer;
+			  bool saved_in_directive = pfile->state.in_directive;
+			  pfile->state.in_directive = true;
+			  cpp_context *saved_prev_context = context->prev;
+			  context->prev = NULL;
+
+			  _cpp_scan_out_logical_line (pfile, NULL, true);
+
+			  pfile->state.in_directive = saved_in_directive;
+			  check_output_buffer (pfile, 1);
+			  *pfile->out.cur = '\n';
+			  pfile->buffer->cur = pfile->out.base + fmacro.offset;
+			  pfile->buffer->line_base = pfile->buffer->cur;
+			  pfile->buffer->rlimit = pfile->out.cur;
+			  CUR (context) = pfile->buffer->cur;
+			  RLIMIT (context) = pfile->buffer->rlimit;
+
+			  pfile->state.prevent_expansion++;
+			  const uchar *text
+			    = _cpp_builtin_macro_text (pfile, fmacro.node);
+			  pfile->state.prevent_expansion--;
+
+			  context->prev = saved_prev_context;
+			  pfile->buffer->cur = saved_cur;
+			  pfile->buffer->rlimit = saved_rlimit;
+			  pfile->buffer->line_base = saved_line_base;
+			  pfile->buffer->need_line = saved_need_line;
+			  pfile->overlaid_buffer = saved_overlaid_buffer;
+			  pfile->out.cur = pfile->out.base + fmacro.offset;
+			  CUR (context) = cur;
+			  RLIMIT (context) = ctx_rlimit;
+			  len = ustrlen (text);
+			  buf = _cpp_unaligned_alloc (pfile, len + 1);
+			  memcpy (buf, text, len);
+			  buf[len] = '\n';
+			  text = buf;
+			  _cpp_push_text_context (pfile, fmacro.node,
+						  text, len);
+			  goto new_context;
+			}
+		      break;
+		    }
+
 		  cpp_macro *m = fmacro.node->value.macro;
 
 		  m->used = 1;
@@ -588,8 +703,7 @@ _cpp_scan_out_logical_line (cpp_reader *
 		    {
 		      /* Remove the macro's invocation from the
 			 output, and push its replacement text.  */
-		      pfile->out.cur = (pfile->out.base
-					     + fmacro.offset);
+		      pfile->out.cur = pfile->out.base + fmacro.offset;
 		      CUR (context) = cur;
 		      replace_args_and_push (pfile, &fmacro);
 		      goto new_context;
@@ -711,7 +825,7 @@ push_replacement_text (cpp_reader *pfile
       len = ustrlen (text);
       buf = _cpp_unaligned_alloc (pfile, len + 1);
       memcpy (buf, text, len);
-      buf[len]='\n';
+      buf[len] = '\n';
       text = buf;
     }
   else
@@ -742,7 +856,7 @@ recursive_macro (cpp_reader *pfile, cpp_
      detect true recursion; instead we assume any expansion more than
      20 deep since the first invocation of this macro must be
      recursing.  */
-  if (recursing && node->value.macro->fun_like)
+  if (recursing && fun_like_macro (node))
     {
       size_t depth = 0;
       cpp_context *context = pfile->context;
@@ -1080,7 +1194,7 @@ _cpp_create_trad_definition (cpp_reader
 		       CPP_OPTION (pfile, discard_comments_in_macro_exp));
 
   pfile->state.prevent_expansion++;
-  _cpp_scan_out_logical_line (pfile, macro);
+  _cpp_scan_out_logical_line (pfile, macro, false);
   pfile->state.prevent_expansion--;
 
   if (!macro)
--- gcc/testsuite/c-c++-common/cpp/pr65238-1.c.jj	2015-03-11 17:12:26.909996912 +0100
+++ gcc/testsuite/c-c++-common/cpp/pr65238-1.c	2015-03-11 17:12:26.909996912 +0100
@@ -0,0 +1,53 @@
+/* PR preprocessor/65238 */
+/* { dg-do run } */
+
+#define A unused
+#define B A
+#define C B
+#define D __has_attribute(unused)
+#define E __has_attribute(C)
+#define F(X) __has_attribute(X)
+#if !__has_attribute(unused)
+#error unused attribute not supported - 1
+#endif
+#if !__has_attribute(C)
+#error unused attribute not supported - 2
+#endif
+#if !D
+#error unused attribute not supported - 3
+#endif
+#if !E
+#error unused attribute not supported - 4
+#endif
+#if !F(unused)
+#error unused attribute not supported - 5
+#endif
+#if !F(C)
+#error unused attribute not supported - 6
+#endif
+int a = __has_attribute (unused) + __has_attribute (C) + D + E + F (unused) + F (C);
+int b = __has_attribute (unused);
+int c = __has_attribute (C);
+int d = D;
+int e = E;
+int f = F (unused);
+int g = F (C);
+int h = __has_attribute (
+  unused
+) + __has_attribute  (
+
+C) + F (
+unused
+
+) + F
+(
+C
+);
+
+int
+main ()
+{
+  if (a != 6 || b != 1 || c != 1 || d != 1 || e != 1 || f != 1 || g != 1 || h != 4)
+    __builtin_abort ();
+  return 0;
+}
--- gcc/testsuite/gcc.dg/cpp/pr65238-2.c.jj	2015-03-11 17:12:26.909996912 +0100
+++ gcc/testsuite/gcc.dg/cpp/pr65238-2.c	2015-03-11 20:00:23.235269177 +0100
@@ -0,0 +1,18 @@
+/* PR preprocessor/65238 */
+/* { dg-do preprocess } */
+
+#if __has_attribute(
+#endif
+#if __has_attribute(unused
+#endif
+#if __has_attribute(unused, unused)
+#endif
+#if __has_attribute(__has_attribute(unused))
+#endif
+
+/* { dg-error "macro .__has_attribute. requires an identifier" "" {target "*-*-*"} 4 } */
+/* { dg-error "missing ... after .__has_attribute." "" {target "*-*-*"} 6 } */
+/* { dg-error "missing ... after .__has_attribute." "" {target "*-*-*"} 8 } */
+/* { dg-error "missing binary operator before token .unused." "" {target "*-*-*"} 8 } */
+/* { dg-error "macro .__has_attribute. requires an identifier" "" {target "*-*-*"} 10 } */
+/* { dg-error "missing ... in expression" "" {target "*-*-*"} 10 } */
--- gcc/testsuite/gcc.dg/cpp/trad/pr65238-3.c.jj	2015-03-11 17:12:26.909996912 +0100
+++ gcc/testsuite/gcc.dg/cpp/trad/pr65238-3.c	2015-03-11 17:12:26.909996912 +0100
@@ -0,0 +1,5 @@
+/* PR preprocessor/65238 */
+/* { dg-do run } */
+/* { dg-options "-traditional-cpp" } */
+
+#include "../../../c-c++-common/cpp/pr65238-1.c"
--- gcc/testsuite/gcc.dg/cpp/trad/pr65238-4.c.jj	2015-03-11 17:12:26.909996912 +0100
+++ gcc/testsuite/gcc.dg/cpp/trad/pr65238-4.c	2015-03-11 17:12:26.909996912 +0100
@@ -0,0 +1,19 @@
+/* PR preprocessor/65238 */
+/* { dg-do preprocess } */
+/* { dg-options "-traditional-cpp" } */
+
+#if __has_attribute(
+#endif
+#if __has_attribute(unused
+#endif
+#if __has_attribute(unused, unused)
+#endif
+#if __has_attribute(__has_attribute(unused))
+#endif
+
+/* { dg-error "unterminated argument list invoking macro .__has_attribute." "" {target "*-*-*"} 5 } */
+/* { dg-error "#if with no expression" "" {target "*-*-*"} 5 } */
+/* { dg-error "unterminated argument list invoking macro .__has_attribute." "" {target "*-*-*"} 7 } */
+/* { dg-error "macro .__has_attribute. passed 2 arguments, but takes just 1" "" {target "*-*-*"} 9 } */
+/* { dg-error "missing ... in expression" "" {target "*-*-*"} 9 } */
+/* { dg-error "macro .__has_attribute. requires an identifier" "" {target "*-*-*"} 11 } */

	Jakub

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Fix __has_{cpp_}attribute with -traditional-cpp (PR preprocessor/65238)
  2015-03-11 19:11 [PATCH] Fix __has_{cpp_}attribute with -traditional-cpp (PR preprocessor/65238) Jakub Jelinek
@ 2015-03-19 16:54 ` Dodji Seketeli
  2015-03-20 16:30 ` Jason Merrill
  1 sibling, 0 replies; 8+ messages in thread
From: Dodji Seketeli @ 2015-03-19 16:54 UTC (permalink / raw)
  To: Jakub Jelinek
  Cc: Joseph S. Myers, Marek Polacek, Jason Merrill, Dodji Seketeli,
	gcc-patches

Hello Jakub,

Jakub Jelinek <jakub@redhat.com> writes:

> __has_{cpp_,}attribute builtin macros are effectively function-like macros
> taking one argument (and the ISO preprocessor expands macros in the argument
> which is IMHO desirable), but the traditional preprocessor has been crashing
> on them or reporting errors.
> As the hook uses cpp_get_token and thus the ISO preprocessor, we need to set
> up things such that the argument and ()s around it are already preprocessed
> and ready to be reparsed by the ISO preprocessor (this is similar to how
> e.g. #if/#elif and various other directives are handled).
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2015-03-11  Jakub Jelinek  <jakub@redhat.com>
>
> 	PR preprocessor/65238
> 	* internal.h (_cpp_scan_out_logical_line): Add third argument.
> 	* directives.c (prepare_directive_trad): Pass false to it.
> 	* traditional.c (_cpp_read_logical_line_trad,
> 	_cpp_create_trad_definition): Likewise.
> 	(struct fun_macro): Add paramc field.
> 	(fun_like_macro): New function.
> 	(maybe_start_funlike): Handle NODE_BUILTIN macros.  Initialize
> 	macro->paramc field.
> 	(save_argument): Use macro->paramc instead of
> 	macro->node->value.macro->paramc.
> 	(push_replacement_text): Formatting fix.
> 	(recursive_macro): Use fun_like_macro helper.
> 	(_cpp_scan_out_logical_line): Likewise.  Add BUILTIN_MACRO_ARG
> 	argument.  Initialize fmacro.paramc field.  Handle builtin
> 	function-like macros.
>
> 	* c-c++-common/cpp/pr65238-1.c: New test.
> 	* gcc.dg/cpp/pr65238-2.c: New test.
> 	* gcc.dg/cpp/trad/pr65238-3.c: New test.
> 	* gcc.dg/cpp/trad/pr65238-4.c: New test.

I do not have the rights to ACK this but FWIW it looks OK to me.  Sorry
for the delay in reviewing this.

Thanks!

Cheers,

-- 
		Dodji

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Fix __has_{cpp_}attribute with -traditional-cpp (PR preprocessor/65238)
  2015-03-11 19:11 [PATCH] Fix __has_{cpp_}attribute with -traditional-cpp (PR preprocessor/65238) Jakub Jelinek
  2015-03-19 16:54 ` Dodji Seketeli
@ 2015-03-20 16:30 ` Jason Merrill
  2015-03-20 16:49   ` Jakub Jelinek
  1 sibling, 1 reply; 8+ messages in thread
From: Jason Merrill @ 2015-03-20 16:30 UTC (permalink / raw)
  To: Jakub Jelinek, Joseph S. Myers, Marek Polacek, Dodji Seketeli; +Cc: gcc-patches

On 03/11/2015 03:10 PM, Jakub Jelinek wrote:
> __has_{cpp_,}attribute builtin macros are effectively function-like macros
> taking one argument (and the ISO preprocessor expands macros in the argument
> which is IMHO desirable), but the traditional preprocessor has been crashing
> on them or reporting errors.

Why do we want ISO preprocessor behavior in this specific situation?

Jason

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Fix __has_{cpp_}attribute with -traditional-cpp (PR preprocessor/65238)
  2015-03-20 16:30 ` Jason Merrill
@ 2015-03-20 16:49   ` Jakub Jelinek
  2015-03-20 17:16     ` Jason Merrill
  0 siblings, 1 reply; 8+ messages in thread
From: Jakub Jelinek @ 2015-03-20 16:49 UTC (permalink / raw)
  To: Jason Merrill; +Cc: Joseph S. Myers, Marek Polacek, Dodji Seketeli, gcc-patches

On Fri, Mar 20, 2015 at 12:30:44PM -0400, Jason Merrill wrote:
> On 03/11/2015 03:10 PM, Jakub Jelinek wrote:
> >__has_{cpp_,}attribute builtin macros are effectively function-like macros
> >taking one argument (and the ISO preprocessor expands macros in the argument
> >which is IMHO desirable), but the traditional preprocessor has been crashing
> >on them or reporting errors.
> 
> Why do we want ISO preprocessor behavior in this specific situation?

You mean that we would handle
#define U unused
#if __has_attribute(U)
int u __attribute__((unused));
#endif
differently between ISO and traditional preprocessing?
That would be surprising to users.  IMHO either we want to expand
the arguments in both cases (what the patch does), or in none
(that would be then consistent with clang++, guess would mean adding
pfile->state.prevent_expansion++; / pfile->state.prevent_expansion--;
pair around something in the ISO case, and would slightly but not too much
simplify the traditional __has_attribute handling; still we'd need to build
the buffer with the argument and feed it to the langhook, which parses it
with ISO preprocessor with disabled expansion).

	Jakub

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Fix __has_{cpp_}attribute with -traditional-cpp (PR preprocessor/65238)
  2015-03-20 16:49   ` Jakub Jelinek
@ 2015-03-20 17:16     ` Jason Merrill
  2015-03-20 17:24       ` Jakub Jelinek
  2015-03-20 17:34       ` Dodji Seketeli
  0 siblings, 2 replies; 8+ messages in thread
From: Jason Merrill @ 2015-03-20 17:16 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Joseph S. Myers, Marek Polacek, Dodji Seketeli, gcc-patches

On 03/20/2015 12:48 PM, Jakub Jelinek wrote:
> On Fri, Mar 20, 2015 at 12:30:44PM -0400, Jason Merrill wrote:
>> On 03/11/2015 03:10 PM, Jakub Jelinek wrote:
>>> __has_{cpp_,}attribute builtin macros are effectively function-like macros
>>> taking one argument (and the ISO preprocessor expands macros in the argument
>>> which is IMHO desirable), but the traditional preprocessor has been crashing
>>> on them or reporting errors.
>>
>> Why do we want ISO preprocessor behavior in this specific situation?
>
> You mean that we would handle
> #define U unused
> #if __has_attribute(U)
> int u __attribute__((unused));
> #endif
> differently between ISO and traditional preprocessing?

> That would be surprising to users.

Why surprising?  Don't users of the traditional preprocessor expect 
traditional preprocessor behavior?

Jason

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Fix __has_{cpp_}attribute with -traditional-cpp (PR preprocessor/65238)
  2015-03-20 17:16     ` Jason Merrill
@ 2015-03-20 17:24       ` Jakub Jelinek
  2015-03-20 21:07         ` Jason Merrill
  2015-03-20 17:34       ` Dodji Seketeli
  1 sibling, 1 reply; 8+ messages in thread
From: Jakub Jelinek @ 2015-03-20 17:24 UTC (permalink / raw)
  To: Jason Merrill; +Cc: Joseph S. Myers, Marek Polacek, Dodji Seketeli, gcc-patches

On Fri, Mar 20, 2015 at 01:15:52PM -0400, Jason Merrill wrote:
> On 03/20/2015 12:48 PM, Jakub Jelinek wrote:
> >On Fri, Mar 20, 2015 at 12:30:44PM -0400, Jason Merrill wrote:
> >>On 03/11/2015 03:10 PM, Jakub Jelinek wrote:
> >>>__has_{cpp_,}attribute builtin macros are effectively function-like macros
> >>>taking one argument (and the ISO preprocessor expands macros in the argument
> >>>which is IMHO desirable), but the traditional preprocessor has been crashing
> >>>on them or reporting errors.
> >>
> >>Why do we want ISO preprocessor behavior in this specific situation?
> >
> >You mean that we would handle
> >#define U unused
> >#if __has_attribute(U)
> >int u __attribute__((unused));
> >#endif
> >differently between ISO and traditional preprocessing?
> 
> >That would be surprising to users.
> 
> Why surprising?  Don't users of the traditional preprocessor expect
> traditional preprocessor behavior?

Well, but the traditional preprocessor behavior in the end is not in not
expanding macro arguments, normally they actually are expanded, but not
immediately, but because of trying to preprocess the result of preprocessing
again and again.
When traditionally preprocessing #if/#elif etc. directives, the directive
line is preprocessed until no macros are present and only then fed to the
ISO preprocessor to evaluate and handle the directive.
The problem with the builtin function-like macros is that it needs the
argument immediately.

	Jakub

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Fix __has_{cpp_}attribute with -traditional-cpp (PR preprocessor/65238)
  2015-03-20 17:16     ` Jason Merrill
  2015-03-20 17:24       ` Jakub Jelinek
@ 2015-03-20 17:34       ` Dodji Seketeli
  1 sibling, 0 replies; 8+ messages in thread
From: Dodji Seketeli @ 2015-03-20 17:34 UTC (permalink / raw)
  To: Jason Merrill
  Cc: Jakub Jelinek, Joseph S. Myers, Marek Polacek, Dodji Seketeli,
	gcc-patches

Jason Merrill <jason@redhat.com> writes:

> On 03/20/2015 12:48 PM, Jakub Jelinek wrote:
>> On Fri, Mar 20, 2015 at 12:30:44PM -0400, Jason Merrill wrote:
>>> On 03/11/2015 03:10 PM, Jakub Jelinek wrote:
>>>> __has_{cpp_,}attribute builtin macros are effectively function-like macros
>>>> taking one argument (and the ISO preprocessor expands macros in the argument
>>>> which is IMHO desirable), but the traditional preprocessor has been crashing
>>>> on them or reporting errors.
>>>
>>> Why do we want ISO preprocessor behavior in this specific situation?
>>
>> You mean that we would handle
>> #define U unused
>> #if __has_attribute(U)
>> int u __attribute__((unused));
>> #endif
>> differently between ISO and traditional preprocessing?
>
>> That would be surprising to users.
>
> Why surprising?  Don't users of the traditional preprocessor expect
> traditional preprocessor behavior?

One of the reasons why I thought it'd be "nice" to have the traditionnal
mode support the macro-expansion of the arguments here is that there
already are cases where the traditionnal mode supports ISO behaviour.
For instance, the documentation of cpp says:

    10.3 Traditional miscellany
    ===========================

    Here are some things to be aware of when using the traditional
    preprocessor.

    [...]

       * A true traditional C preprocessor does not recognize '#error' or
         '#pragma', and may not recognize '#elif'.  CPP supports all the
         directives in traditional mode that it supports in ISO mode,
         including extensions, with the exception that the effects of
         '#pragma GCC poison' are undefined.

So I thought this useful particular use case of __has_attribute(U) might
well be another of such case even if it's not a directive.

Just my 2 cents.

-- 
		Dodji

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Fix __has_{cpp_}attribute with -traditional-cpp (PR preprocessor/65238)
  2015-03-20 17:24       ` Jakub Jelinek
@ 2015-03-20 21:07         ` Jason Merrill
  0 siblings, 0 replies; 8+ messages in thread
From: Jason Merrill @ 2015-03-20 21:07 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Joseph S. Myers, Marek Polacek, Dodji Seketeli, gcc-patches

On 03/20/2015 01:23 PM, Jakub Jelinek wrote:
> Well, but the traditional preprocessor behavior in the end is not in not
> expanding macro arguments, normally they actually are expanded, but not
> immediately, but because of trying to preprocess the result of preprocessing
> again and again.
> When traditionally preprocessing #if/#elif etc. directives, the directive
> line is preprocessed until no macros are present and only then fed to the
> ISO preprocessor to evaluate and handle the directive.
> The problem with the builtin function-like macros is that it needs the
> argument immediately.

Ah, I see, thanks.  Please add some of this rationale to the patch.  OK 
with that.

Jason


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2015-03-20 21:07 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-11 19:11 [PATCH] Fix __has_{cpp_}attribute with -traditional-cpp (PR preprocessor/65238) Jakub Jelinek
2015-03-19 16:54 ` Dodji Seketeli
2015-03-20 16:30 ` Jason Merrill
2015-03-20 16:49   ` Jakub Jelinek
2015-03-20 17:16     ` Jason Merrill
2015-03-20 17:24       ` Jakub Jelinek
2015-03-20 21:07         ` Jason Merrill
2015-03-20 17:34       ` Dodji Seketeli

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).