public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
From: "jakub at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug c++/101488] Implement p1042r1 __VA_OPT__ placemarker changes
Date: Mon, 19 Jul 2021 12:51:40 +0000	[thread overview]
Message-ID: <bug-101488-4-PUgdB76BDZ@http.gcc.gnu.org/bugzilla/> (raw)
In-Reply-To: <bug-101488-4@http.gcc.gnu.org/bugzilla/>

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101488

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Different patch:
--- libcpp/macro.c.jj   2021-07-16 11:10:08.512925510 +0200
+++ libcpp/macro.c      2021-07-19 14:19:48.217122675 +0200
@@ -236,10 +236,20 @@ class vaopt_state {
     return m_state == 0;
   }

+  /* Return true for # __VA_OPT__.  */
   bool stringify () const
   {
     return m_stringify;
-  }  
+  }
+
+  /* Return true if inside of __VA_OPT__ args but not inside of extra ()s
+     nested in it, i.e. when if next token is ), it will be END.
+     Next token is guaranteed to appear, as otherwise it would be ERROR
+     earlier.  */
+  bool maybe_before_end () const
+  {
+    return m_state == 3;
+  }

  private:

@@ -2025,6 +2035,7 @@ replace_args (cpp_reader *pfile, cpp_has
   i = 0;
   vaopt_state vaopt_tracker (pfile, macro->variadic, &args[macro->paramc -
1]);
   const cpp_token **vaopt_start = NULL;
+  bool vaopt_paste_left = false;
   for (src = macro->exp.tokens; src < limit; src++)
     {
       unsigned int arg_tokens_count;
@@ -2050,6 +2061,8 @@ replace_args (cpp_reader *pfile, cpp_has
                                         t->src_loc, t->src_loc,
                                         map, index);
                }
+              vaopt_paste_left = (src != macro->exp.tokens
+                                 && (src[-1].flags & PASTE_LEFT));
              vaopt_start = tokens_buff_last_token_ptr (buff);
            }
          else if (vostate == vaopt_state::END)
@@ -2059,7 +2072,7 @@ replace_args (cpp_reader *pfile, cpp_has

              /* Remove any tail padding from inside the __VA_OPT__.  */
              paste_flag = tokens_buff_last_token_ptr (buff);
-             while (paste_flag && paste_flag != start
+             while (0 && paste_flag && paste_flag != start
                     && (*paste_flag)->type == CPP_PADDING)
                {
                  tokens_buff_remove_last_token (buff);
@@ -2184,7 +2197,7 @@ replace_args (cpp_reader *pfile, cpp_has
                 previous emitted token is at the beginning of __VA_OPT__;
                 placemarkers within __VA_OPT__ are ignored in that case.  */
              else if (arg_tokens_count == 0
-                      && tmp_token_ptr != vaopt_start)
+/*                    && tmp_token_ptr != vaopt_start */)
                paste_flag = tmp_token_ptr;
            }
        }
@@ -2214,8 +2227,9 @@ replace_args (cpp_reader *pfile, cpp_has

       /* Padding on the left of an argument (unless RHS of ##).  */
       if ((!pfile->state.in_directive || pfile->state.directive_wants_padding)
-         && src != macro->exp.tokens && !(src[-1].flags & PASTE_LEFT)
-         && !last_token_is (buff, vaopt_start))
+         && src != macro->exp.tokens
+         && !(src[-1].flags & PASTE_LEFT)
+         && (!last_token_is (buff, vaopt_start) || !vaopt_paste_left))
        {
          const cpp_token *t = padding_token (pfile, src);
          unsigned index = expanded_token_index (pfile, macro, src, i);
@@ -2300,8 +2314,12 @@ replace_args (cpp_reader *pfile, cpp_has
                     NODE_NAME (node), src->val.macro_arg.arg_no);

       /* Avoid paste on RHS (even case count == 0).  */
-      if (!pfile->state.in_directive && !(src->flags & PASTE_LEFT)
-         && !last_token_is (buff, vaopt_start))
+      if (!pfile->state.in_directive
+         && !(src->flags & PASTE_LEFT)
+         && (!vaopt_tracker.maybe_before_end ()
+             || src[1].type != CPP_CLOSE_PAREN
+             || (src[1].flags & PASTE_LEFT) == 0)
+         /*&& !last_token_is (buff, vaopt_start)*/)
        {
          const cpp_token *t = &pfile->avoid_paste;
          tokens_buff_add_token (buff, virt_locs,
With this one, clang's macro_vaopt_expand.cpp and macro_vaopt_p1042r1.cpp and
the #c0 testcase preprocess the same between g++ and clang++, but
c-c++-common/cpp/va-opt-3.c has one difference:
t26 f20 (f21 (), 2);
/* { dg-final { scan-file va-opt-3.i "t26 f17 h;" } } */
which yields with the patched cc1plus into
t26 f17 f17 m4();

I guess I understand why for the cases of x ## __VA_OPT__ (y z) and __VA_OPT__
(a b) ## c we want
for y and b macros use MACRO_ARG_TOKEN_EXPANDED instead of
MACRO_ARG_TOKEN_NORMAL when it would be
x ## y z or a b ## c, but already in the above patch I had to uncomment again
the
          if (last_token_is (buff, vaopt_start))
            {
              /* We're expanding an arg at the beginning of __VA_OPT__.
                 Skip padding. */
              while (arg_tokens_count)
                {
                  const cpp_token *t = macro_arg_token_iter_get_token (&from);
                  if (t->type != CPP_PADDING)
                    break;
                  macro_arg_token_iter_forward (&from);
                  --arg_tokens_count;
                }
            }
padding token removal because otherwise e.g. the #c1 testcase (and
macro_vaopt_expand.cpp) misbehaves - MACRO_ARG_TOKEN_EXPANDED
tokens for the x macro start with CPP_PADDING and at least if clang++ is
correct we don't want it there.
The question is if we should optimize away a single CPP_PADDING tokens or
unlimited number of them, whether it is possible
that somehow the macro expanded definition would start with multiple
CPP_PADDING tokens and whether some of them shouldn't be kept.
For the t26 case, we need to remove some trailing CPP_PADDING tokens too, but
as various testcases show, not all of them.
So, perhaps remember in the loop in some integer variable how many CPP_PADDING
tokens should be removed if next token would be vaopt_state::END
with PASTE_LEFT on it?

  parent reply	other threads:[~2021-07-19 12:51 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-17 14:21 [Bug c++/101488] New: " jakub at gcc dot gnu.org
2021-07-17 14:29 ` [Bug c++/101488] " jakub at gcc dot gnu.org
2021-07-19 12:51 ` jakub at gcc dot gnu.org [this message]
2021-07-19 14:38 ` jakub at gcc dot gnu.org
2021-09-01 19:34 ` cvs-commit at gcc dot gnu.org
2021-09-01 19:35 ` jakub at gcc dot gnu.org

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bug-101488-4-PUgdB76BDZ@http.gcc.gnu.org/bugzilla/ \
    --to=gcc-bugzilla@gcc.gnu.org \
    --cc=gcc-bugs@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).