On 10/10/22 15:18, Marek Polacek wrote:
On Mon, Oct 10, 2022 at 10:49:34AM -0400, Jason Merrill wrote:
On 10/7/22 18:16, Marek Polacek wrote:
On Fri, Oct 07, 2022 at 05:56:18PM -0400, Jason Merrill wrote:
On 10/7/22 17:08, Marek Polacek wrote:
On Fri, Oct 07, 2022 at 12:17:34PM -0400, Jason Merrill wrote:
On 10/6/22 22:12, Marek Polacek wrote:
On Thu, Oct 06, 2022 at 05:42:41PM -0400, Jason Merrill wrote:
On 10/4/22 19:06, Marek Polacek wrote:
On Fri, Sep 30, 2022 at 09:12:24AM -0400, Jason Merrill wrote:
On 9/29/22 18:49, Marek Polacek wrote:
When getting the name of an attribute, we ought to use
get_attribute_name, which handles both [[ ]] and __attribute__(())
forms.  Failure to do so may result in an ICE, like here.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

How do we print the attributes with this patch?  Don't we also want to print
the namespace, and use [[]] in the output?

Good point, however: while the testcase indeed has an attribute
in the [[]] form in the typedef, here we're printing its "aka":

warning: initialization of 'FuncPointerWithNoCfCheck' {aka 'void 
(__attribute__((nocf_check)) *)(void)'} from incompatible pointer type 
'FuncPointer' {aka 'void (*)(void)'}

c-pretty-print.cc doesn't seem to know how to print an [[]] attribute.
I could do that, but then we'd print

       aka 'void ([[nocf_check]] *)(void)'

in the above, but that's invalid syntax!

Indeed, it should be

void (* [[gnu::nocf_check]])(void)

Ok, let me fix that too, then.  I've updated the description to show
what we print with the patch, and what was printed before.

Oops, apparently I was wrongly assuming that the attribute appertained to
the pointer.

So was I :/.

Since it actually appertains to the function type, it should
be

        void (*)(void) [[gnu::nocf_check]].

But for GCC attribute syntax, I think we want to leave the __attribute__
where it was before, as in the manual's example

        void (__attribute__((noreturn)) ****f) (void);

Thanks, done here:

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

-- >8 --
When getting the name of an attribute, we ought to use
get_attribute_name, which handles both [[]] and __attribute__(())
forms.  Failure to do so may result in an ICE, like here.

pp_c_attributes_display wasn't able to print the [[]] form of
attributes, so this patch teaches it to.

When printing a pointer to function with a standard attribute, the attribute
should be printed after the parameter-list.  With this patch we print:

     aka 'void (*)(int) [[gnu::nocf_check]]'

pp_c_attributes has been unused since its introduction in r56273 so
this patch removes it.

        PR c++/106937

gcc/c-family/ChangeLog:

        * c-pretty-print.cc (pp_c_specifier_qualifier_list): Only print
        GNU attributes here.
        (c_pretty_printer::abstract_declarator): For a pointer to function,
        print the standard [[]] attributes here.
        (pp_c_attributes): Remove.
        (pp_c_attributes_display): Print the [[]] form if appropriate.  Use
        get_attribute_name.  Don't print a trailing space when printing the
        [[]] form.
        * c-pretty-print.h (pp_c_attributes): Remove.

gcc/cp/ChangeLog:

        * error.cc: Include "attribs.h".
        (dump_type_prefix): Only print GNU attributes here.
        (dump_type_suffix): Print standard attributes here.

gcc/testsuite/ChangeLog:

        * c-c++-common/pointer-to-fn1.c: New test.
---
    gcc/c-family/c-pretty-print.cc              | 102 +++++++++++---------
    gcc/c-family/c-pretty-print.h               |   1 -
    gcc/cp/error.cc                             |  17 +++-
    gcc/testsuite/c-c++-common/pointer-to-fn1.c |  18 ++++
    4 files changed, 92 insertions(+), 46 deletions(-)
    create mode 100644 gcc/testsuite/c-c++-common/pointer-to-fn1.c

diff --git a/gcc/c-family/c-pretty-print.cc b/gcc/c-family/c-pretty-print.cc
index efa1768f4d6..349f0a07d3e 100644
--- a/gcc/c-family/c-pretty-print.cc
+++ b/gcc/c-family/c-pretty-print.cc
@@ -466,7 +466,12 @@ pp_c_specifier_qualifier_list (c_pretty_printer *pp, tree 
t)
          {
            pp_c_whitespace (pp);
            pp_c_left_paren (pp);
-           pp_c_attributes_display (pp, TYPE_ATTRIBUTES (pointee));
+           /* If we're dealing with the GNU form of attributes, print this:
+                void (__attribute__((noreturn)) *f) ();
+              If it is the standard [[]] attribute, we'll print the attribute
+              in c_pretty_printer::abstract_declarator.  */
+           if (!cxx11_attribute_p (TYPE_ATTRIBUTES (pointee)))
+             pp_c_attributes_display (pp, TYPE_ATTRIBUTES (pointee));
          }
        else if (!c_dialect_cxx ())
          pp_c_whitespace (pp);
@@ -564,15 +569,26 @@ pp_c_parameter_type_list (c_pretty_printer *pp, tree t)
    void
    c_pretty_printer::abstract_declarator (tree t)
    {
+  bool fn_ptr_p = false;
      if (TREE_CODE (t) == POINTER_TYPE)
        {
          if (TREE_CODE (TREE_TYPE (t)) == ARRAY_TYPE
          || TREE_CODE (TREE_TYPE (t)) == FUNCTION_TYPE)
-       pp_c_right_paren (this);
+       {
+         pp_c_right_paren (this);
+         fn_ptr_p = true;
+       }
          t = TREE_TYPE (t);
        }
      direct_abstract_declarator (t);
+  /* If this is the standard [[]] attribute, print
+       void (*)() [[noreturn]];  */
+  if (fn_ptr_p && cxx11_attribute_p (TYPE_ATTRIBUTES (t)))

Do you need to check fn_ptr_p?  The attributes on a function type go here
whether or not we were dealing with a pointer.

I probably don't need it!  Here's a version without that:

So far all dg.exp tests passed.

-- >8 --
When getting the name of an attribute, we ought to use
get_attribute_name, which handles both [[]] and __attribute__(())
forms.  Failure to do so may result in an ICE, like here.

pp_c_attributes_display wasn't able to print the [[]] form of
attributes, so this patch teaches it to.

When printing a pointer to function with a standard attribute, the attribute
should be printed after the parameter-list.  With this patch we print:

    aka 'void (*)(int) [[gnu::nocf_check]]'

pp_c_attributes has been unused since its introduction in r56273 so
this patch removes it.

        PR c++/106937

gcc/c-family/ChangeLog:

        * c-pretty-print.cc (pp_c_specifier_qualifier_list): Only print
        GNU attributes here.
        (c_pretty_printer::abstract_declarator): For a pointer to function,
        print the standard [[]] attributes here.

This change now applies to all types, whereas the dump_type_suffix change
only applies to functions.  Maybe move this change into
direct_abstract_declarator?
That's better, done.

        (pp_c_attributes): Remove.
        (pp_c_attributes_display): Print the [[]] form if appropriate.  Use
        get_attribute_name.  Don't print a trailing space when printing the
        [[]] form.

This is no longer accurate.

What specifically?  I think it's still accurate.

You're right, I was confused.

        * c-pretty-print.h (pp_c_attributes): Remove.

gcc/cp/ChangeLog:

        * error.cc: Include "attribs.h".
        (dump_type_prefix): Only print GNU attributes here.
        (dump_type_suffix): Print standard attributes here.

See below.
@@ -1027,6 +1033,14 @@ dump_type_suffix (cxx_pretty_printer *pp, tree t, int 
flags)
                              TREE_CODE (t) == FUNCTION_TYPE
                              && (flags & TFF_POINTER));
        dump_ref_qualifier (pp, t, flags);
+       /* If this is the standard [[]] attribute, print
+            void (*)() [[noreturn]];  */
+       if (cxx11_attribute_p (TYPE_ATTRIBUTES (t)))
+         {
+           pp_space (pp);
+           pp_c_attributes_display (pp, TYPE_ATTRIBUTES (t));
+           pp->padding = pp_before;
+         }

This is too soon, attributes come after the exception-specifier.

Ah, fixed then.

Hopefully this is the correct patch.  Thanks.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

OK, thanks.

-- >8 --
When getting the name of an attribute, we ought to use
get_attribute_name, which handles both [[]] and __attribute__(())
forms.  Failure to do so may result in an ICE, like here.

pp_c_attributes_display wasn't able to print the [[]] form of
attributes, so this patch teaches it to.

When printing a pointer to function with a standard attribute, the attribute
should be printed after the parameter-list.  With this patch we print:

   aka 'void (*)(int) [[gnu::nocf_check]]'

or, in C++ with noexcept:

   aka 'void (*)(int) noexcept [[gnu::nocf_check]]'

pp_c_attributes has been unused since its introduction in r56273 so
this patch removes it.

        PR c++/106937

gcc/c-family/ChangeLog:

        * c-pretty-print.cc (pp_c_specifier_qualifier_list): Print only GNU
        attributes here.
        (c_pretty_printer::direct_abstract_declarator): Print the standard [[]]
        attributes here.
        (pp_c_attributes): Remove.
        (pp_c_attributes_display): Print the [[]] form if appropriate.  Use
        get_attribute_name.  Don't print a trailing space when printing the
        [[]] form.
        * c-pretty-print.h (pp_c_attributes): Remove.

gcc/cp/ChangeLog:

        * error.cc: Include "attribs.h".
        (dump_type_prefix): Print only GNU attributes here.
        (dump_type_suffix): Print standard attributes here.

gcc/testsuite/ChangeLog:

        * c-c++-common/pointer-to-fn1.c: New test.
---
  gcc/c-family/c-pretty-print.cc              | 96 ++++++++++++---------
  gcc/c-family/c-pretty-print.h               |  1 -
  gcc/cp/error.cc                             | 16 +++-
  gcc/testsuite/c-c++-common/pointer-to-fn1.c | 18 ++++
  4 files changed, 86 insertions(+), 45 deletions(-)
  create mode 100644 gcc/testsuite/c-c++-common/pointer-to-fn1.c

diff --git a/gcc/c-family/c-pretty-print.cc b/gcc/c-family/c-pretty-print.cc
index efa1768f4d6..c99b2ceffe6 100644
--- a/gcc/c-family/c-pretty-print.cc
+++ b/gcc/c-family/c-pretty-print.cc
@@ -466,7 +466,12 @@ pp_c_specifier_qualifier_list (c_pretty_printer *pp, tree 
t)
          {
            pp_c_whitespace (pp);
            pp_c_left_paren (pp);
-           pp_c_attributes_display (pp, TYPE_ATTRIBUTES (pointee));
+           /* If we're dealing with the GNU form of attributes, print this:
+                void (__attribute__((noreturn)) *f) ();
+              If it is the standard [[]] attribute, we'll print the attribute
+              in c_pretty_printer::direct_abstract_declarator/FUNCTION_TYPE.  
*/
+           if (!cxx11_attribute_p (TYPE_ATTRIBUTES (pointee)))
+             pp_c_attributes_display (pp, TYPE_ATTRIBUTES (pointee));
          }
        else if (!c_dialect_cxx ())
          pp_c_whitespace (pp);
@@ -595,6 +600,13 @@ c_pretty_printer::direct_abstract_declarator (tree t)
      case FUNCTION_TYPE:
        pp_c_parameter_type_list (this, t);
        direct_abstract_declarator (TREE_TYPE (t));
+      /* If this is the standard [[]] attribute, print
+        void (*)() [[noreturn]];  */
+      if (cxx11_attribute_p (TYPE_ATTRIBUTES (t)))
+       {
+         pp_space (this);
+         pp_c_attributes_display (this, TYPE_ATTRIBUTES (t));
+       }
        break;
case ARRAY_TYPE:
@@ -850,32 +862,7 @@ c_pretty_printer::declaration (tree t)
    pp_c_init_declarator (this, t);
  }
-/* Pretty-print ATTRIBUTES using GNU C extension syntax. */
-
-void
-pp_c_attributes (c_pretty_printer *pp, tree attributes)
-{
-  if (attributes == NULL_TREE)
-    return;
-
-  pp_c_ws_string (pp, "__attribute__");
-  pp_c_left_paren (pp);
-  pp_c_left_paren (pp);
-  for (; attributes != NULL_TREE; attributes = TREE_CHAIN (attributes))
-    {
-      pp_tree_identifier (pp, TREE_PURPOSE (attributes));
-      if (TREE_VALUE (attributes))
-       pp_c_call_argument_list (pp, TREE_VALUE (attributes));
-
-      if (TREE_CHAIN (attributes))
-       pp_separate_with (pp, ',');
-    }
-  pp_c_right_paren (pp);
-  pp_c_right_paren (pp);
-}
-
-/* Pretty-print ATTRIBUTES using GNU C extension syntax for attributes
-   marked to be displayed on disgnostic.  */
+/* Pretty-print ATTRIBUTES marked to be displayed on diagnostic.  */
void
  pp_c_attributes_display (c_pretty_printer *pp, tree a)
@@ -885,10 +872,12 @@ pp_c_attributes_display (c_pretty_printer *pp, tree a)
    if (a == NULL_TREE)
      return;
+ const bool std_p = cxx11_attribute_p (a);
+
    for (; a != NULL_TREE; a = TREE_CHAIN (a))
      {
-      const struct attribute_spec *as;
-      as = lookup_attribute_spec (TREE_PURPOSE (a));
+      const struct attribute_spec *as
+       = lookup_attribute_spec (get_attribute_name (a));
        if (!as || as->affects_type_identity == false)
          continue;
        if (c_dialect_cxx ()
@@ -896,26 +885,47 @@ pp_c_attributes_display (c_pretty_printer *pp, tree a)
        /* In C++ transaction_safe is printed at the end of the declarator.  */
        continue;
        if (is_first)
-       {
-         pp_c_ws_string (pp, "__attribute__");
-         pp_c_left_paren (pp);
-         pp_c_left_paren (pp);
-         is_first = false;
-       }
+       {
+         if (std_p)
+           {
+             pp_c_left_bracket (pp);
+             pp_c_left_bracket (pp);
+           }
+         else
+           {
+             pp_c_ws_string (pp, "__attribute__");
+             pp_c_left_paren (pp);
+             pp_c_left_paren (pp);
+           }
+         is_first = false;
+       }
        else
-       {
-         pp_separate_with (pp, ',');
-       }
-      pp_tree_identifier (pp, TREE_PURPOSE (a));
+       pp_separate_with (pp, ',');
+      tree ns;
+      if (std_p && (ns = get_attribute_namespace (a)))
+       {
+         pp_tree_identifier (pp, ns);
+         pp_colon (pp);
+         pp_colon (pp);
+       }
+      pp_tree_identifier (pp, get_attribute_name (a));
        if (TREE_VALUE (a))
-       pp_c_call_argument_list (pp, TREE_VALUE (a));
+       pp_c_call_argument_list (pp, TREE_VALUE (a));
      }
if (!is_first)
      {
-      pp_c_right_paren (pp);
-      pp_c_right_paren (pp);
-      pp_c_whitespace (pp);
+      if (std_p)
+       {
+         pp_c_right_bracket (pp);
+         pp_c_right_bracket (pp);
+       }
+      else
+       {
+         pp_c_right_paren (pp);
+         pp_c_right_paren (pp);
+         pp_c_whitespace (pp);
+       }
      }
  }
diff --git a/gcc/c-family/c-pretty-print.h b/gcc/c-family/c-pretty-print.h
index be86bed4fee..92674ab4d06 100644
--- a/gcc/c-family/c-pretty-print.h
+++ b/gcc/c-family/c-pretty-print.h
@@ -119,7 +119,6 @@ void pp_c_space_for_pointer_operator (c_pretty_printer *, 
tree);
  /* Declarations.  */
  void pp_c_tree_decl_identifier (c_pretty_printer *, tree);
  void pp_c_function_definition (c_pretty_printer *, tree);
-void pp_c_attributes (c_pretty_printer *, tree);
  void pp_c_attributes_display (c_pretty_printer *, tree);
  void pp_c_cv_qualifiers (c_pretty_printer *pp, int qualifiers, bool 
func_type);
  void pp_c_type_qualifier_list (c_pretty_printer *, tree);
diff --git a/gcc/cp/error.cc b/gcc/cp/error.cc
index 4514c8bbb44..da8c95c9b43 100644
--- a/gcc/cp/error.cc
+++ b/gcc/cp/error.cc
@@ -36,6 +36,7 @@ along with GCC; see the file COPYING3.  If not see
  #include "internal-fn.h"
  #include "gcc-rich-location.h"
  #include "cp-name-hint.h"
+#include "attribs.h"
#define pp_separate_with_comma(PP) pp_cxx_separate_with (PP, ',')
  #define pp_separate_with_semicolon(PP) pp_cxx_separate_with (PP, ';')
@@ -897,7 +898,12 @@ dump_type_prefix (cxx_pretty_printer *pp, tree t, int 
flags)
          {
            pp_cxx_whitespace (pp);
            pp_cxx_left_paren (pp);
-           pp_c_attributes_display (pp, TYPE_ATTRIBUTES (sub));
+           /* If we're dealing with the GNU form of attributes, print this:
+                void (__attribute__((noreturn)) *f) ();
+              If it is the standard [[]] attribute, we'll print the attribute
+              in dump_type_suffix.  */
+           if (!cxx11_attribute_p (TYPE_ATTRIBUTES (sub)))
+             pp_c_attributes_display (pp, TYPE_ATTRIBUTES (sub));
          }
        if (TYPE_PTR_P (t))
          pp_star (pp);
@@ -1030,6 +1036,14 @@ dump_type_suffix (cxx_pretty_printer *pp, tree t, int 
flags)
        if (tx_safe_fn_type_p (t))
          pp_cxx_ws_string (pp, "transaction_safe");
        dump_exception_spec (pp, TYPE_RAISES_EXCEPTIONS (t), flags);
+       /* If this is the standard [[]] attribute, print
+            void (*)() [[noreturn]];  */
+       if (cxx11_attribute_p (TYPE_ATTRIBUTES (t)))
+         {
+           pp_space (pp);
+           pp_c_attributes_display (pp, TYPE_ATTRIBUTES (t));
+           pp->padding = pp_before;
+         }
        dump_type_suffix (pp, TREE_TYPE (t), flags);
        break;
        }
diff --git a/gcc/testsuite/c-c++-common/pointer-to-fn1.c 
b/gcc/testsuite/c-c++-common/pointer-to-fn1.c
new file mode 100644
index 00000000000..975885462e9
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pointer-to-fn1.c
@@ -0,0 +1,18 @@
+/* PR c++/106937 */
+/* { dg-options "-fcf-protection" } */
+/* { dg-additional-options "-std=c++11 -fpermissive" { target c++ } } */
+/* Test printing a pointer to function with attribute.  */
+
+__attribute__((nocf_check)) typedef void (*FPA1)();
+[[gnu::nocf_check]] typedef void (*FPA2)(int);
+typedef void (*FP1)();
+typedef void (*FP2)(int);
+
+void
+g (FP1 f1, FP2 f2)
+{
+  FPA1 p1 = f1; // { dg-warning {aka 'void \(__attribute__\(\(nocf_check\)\) 
\*\)\(\)'} }
+  FPA2 p2 = f2; // { dg-warning {aka 'void \(\*\)\(int\) 
\[\[gnu::nocf_check\]\]'} }
+  FP1 p3 = p1; // { dg-warning {aka 'void \(__attribute__\(\(nocf_check\)\) 
\*\)\(\)'} }
+  FP2 p4 = p2; // { dg-warning {aka 'void \(\*\)\(int\) 
\[\[gnu::nocf_check\]\]'} }
+}

base-commit: 248c8aeebc49aae3fd96bd587367d12e7c8b3c3a

Reply via email to