c++: is_late_template_attribute and tsubst_attribute fixes

Message ID aTP-J9gYVsOPeV0g@tucnak
State New
Headers
Series c++: is_late_template_attribute and tsubst_attribute fixes |

Commit Message

Jakub Jelinek Dec. 6, 2025, 9:57 a.m. UTC
  Hi!

This has been discussed in the 1/9 Reflection thread, but doesn't depend on
reglection in any way.
cp_parser_std_attribute calls lookup_attribute_spec as:
    const attribute_spec *as
      = lookup_attribute_spec (TREE_PURPOSE (attribute));
so with TREE_LIST where TREE_VALUE is attribute name and TREE_PURPOSE
attribute ns.  Similarly c_parser_std_attribute.  And for
attribute_takes_identifier_p those do:
    else if (attr_ns == gnu_identifier
             && attribute_takes_identifier_p (attr_id))
and
        bool takes_identifier
          = (ns != NULL_TREE
             && strcmp (IDENTIFIER_POINTER (ns), "gnu") == 0
             && attribute_takes_identifier_p (name));
when handling std attributes (for GNU attributes they just call those
with the IDENTIFIER_NODE name.
is_late_template_attribute and tsubst_attribute pass to these functions
just get_attribute_name though, so handle attributes in all namespaces
as GNU attributes only, which means that lookup_attribute_spec can
return NULL or find a different attribute if it is not from gnu:: or
say standard attribute mapped to gnu::, or attribute_takes_identifier_p
can return true even for attributes for which it shouldn't.

I thought about changing attribute_takes_identifier_p to take optionally
TREE_LIST, but that would mean handling it in the target hooks too and
they only care about GNU attributes right now, so given the above
parser.cc/c-parser.cc snippets, the following patch just follow
what they do.

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

2025-12-06  Jakub Jelinek  <jakub@redhat.com>

	* decl2.cc (is_late_template_attribute): Call lookup_attribute_spec
	on TREE_PURPOSE (attr) rather than name.  Only call
	attribute_takes_identifier_p if get_attribute_namespace (attr) is
	gnu_identifier.
	* pt.cc (tsubst_attribute): Only call attribute_takes_identifier_p
	if get_attribute_namespace (t) is gnu_identifier.


	Jakub
  

Comments

Jason Merrill Dec. 6, 2025, 10:02 a.m. UTC | #1
On 12/6/25 3:27 PM, Jakub Jelinek wrote:
> Hi!
> 
> This has been discussed in the 1/9 Reflection thread, but doesn't depend on
> reglection in any way.
> cp_parser_std_attribute calls lookup_attribute_spec as:
>      const attribute_spec *as
>        = lookup_attribute_spec (TREE_PURPOSE (attribute));
> so with TREE_LIST where TREE_VALUE is attribute name and TREE_PURPOSE
> attribute ns.  Similarly c_parser_std_attribute.  And for
> attribute_takes_identifier_p those do:
>      else if (attr_ns == gnu_identifier
>               && attribute_takes_identifier_p (attr_id))
> and
>          bool takes_identifier
>            = (ns != NULL_TREE
>               && strcmp (IDENTIFIER_POINTER (ns), "gnu") == 0
>               && attribute_takes_identifier_p (name));
> when handling std attributes (for GNU attributes they just call those
> with the IDENTIFIER_NODE name.
> is_late_template_attribute and tsubst_attribute pass to these functions
> just get_attribute_name though, so handle attributes in all namespaces
> as GNU attributes only, which means that lookup_attribute_spec can
> return NULL or find a different attribute if it is not from gnu:: or
> say standard attribute mapped to gnu::, or attribute_takes_identifier_p
> can return true even for attributes for which it shouldn't.
> 
> I thought about changing attribute_takes_identifier_p to take optionally
> TREE_LIST, but that would mean handling it in the target hooks too and
> they only care about GNU attributes right now, so given the above
> parser.cc/c-parser.cc snippets, the following patch just follow
> what they do.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

> 2025-12-06  Jakub Jelinek  <jakub@redhat.com>
> 
> 	* decl2.cc (is_late_template_attribute): Call lookup_attribute_spec
> 	on TREE_PURPOSE (attr) rather than name.  Only call
> 	attribute_takes_identifier_p if get_attribute_namespace (attr) is
> 	gnu_identifier.
> 	* pt.cc (tsubst_attribute): Only call attribute_takes_identifier_p
> 	if get_attribute_namespace (t) is gnu_identifier.
> 
> --- gcc/cp/decl2.cc.jj	2025-11-07 10:35:07.247570768 +0100
> +++ gcc/cp/decl2.cc	2025-12-05 22:19:30.445029079 +0100
> @@ -1468,7 +1468,8 @@ is_late_template_attribute (tree attr, t
>   {
>     tree name = get_attribute_name (attr);
>     tree args = TREE_VALUE (attr);
> -  const struct attribute_spec *spec = lookup_attribute_spec (name);
> +  const struct attribute_spec *spec
> +    = lookup_attribute_spec (TREE_PURPOSE (attr));
>     tree arg;
>   
>     if (!spec)
> @@ -1512,7 +1513,9 @@ is_late_template_attribute (tree attr, t
>   	 second and following arguments.  Attributes like mode, format,
>   	 cleanup and several target specific attributes aren't late
>   	 just because they have an IDENTIFIER_NODE as first argument.  */
> -      if (arg == args && attribute_takes_identifier_p (name)
> +      if (arg == args
> +	  && get_attribute_namespace (attr) == gnu_identifier
> +	  && attribute_takes_identifier_p (name)
>   	  && identifier_p (t))
>   	continue;
>   
> --- gcc/cp/pt.cc.jj	2025-12-05 11:50:08.806462730 +0100
> +++ gcc/cp/pt.cc	2025-12-05 22:20:17.831206632 +0100
> @@ -12386,7 +12386,8 @@ tsubst_attribute (tree t, tree *decl_p,
>        pass it through tsubst.  Attributes like mode, format,
>        cleanup and several target specific attributes expect it
>        unmodified.  */
> -  else if (attribute_takes_identifier_p (get_attribute_name (t)))
> +  else if (get_attribute_namespace (t) == gnu_identifier
> +	   && attribute_takes_identifier_p (get_attribute_name (t)))
>       {
>         tree chain
>   	= tsubst_expr (TREE_CHAIN (val), args, complain, in_decl);
> 
> 	Jakub
>
  

Patch

--- gcc/cp/decl2.cc.jj	2025-11-07 10:35:07.247570768 +0100
+++ gcc/cp/decl2.cc	2025-12-05 22:19:30.445029079 +0100
@@ -1468,7 +1468,8 @@  is_late_template_attribute (tree attr, t
 {
   tree name = get_attribute_name (attr);
   tree args = TREE_VALUE (attr);
-  const struct attribute_spec *spec = lookup_attribute_spec (name);
+  const struct attribute_spec *spec
+    = lookup_attribute_spec (TREE_PURPOSE (attr));
   tree arg;
 
   if (!spec)
@@ -1512,7 +1513,9 @@  is_late_template_attribute (tree attr, t
 	 second and following arguments.  Attributes like mode, format,
 	 cleanup and several target specific attributes aren't late
 	 just because they have an IDENTIFIER_NODE as first argument.  */
-      if (arg == args && attribute_takes_identifier_p (name)
+      if (arg == args
+	  && get_attribute_namespace (attr) == gnu_identifier
+	  && attribute_takes_identifier_p (name)
 	  && identifier_p (t))
 	continue;
 
--- gcc/cp/pt.cc.jj	2025-12-05 11:50:08.806462730 +0100
+++ gcc/cp/pt.cc	2025-12-05 22:20:17.831206632 +0100
@@ -12386,7 +12386,8 @@  tsubst_attribute (tree t, tree *decl_p,
      pass it through tsubst.  Attributes like mode, format,
      cleanup and several target specific attributes expect it
      unmodified.  */
-  else if (attribute_takes_identifier_p (get_attribute_name (t)))
+  else if (get_attribute_namespace (t) == gnu_identifier
+	   && attribute_takes_identifier_p (get_attribute_name (t)))
     {
       tree chain
 	= tsubst_expr (TREE_CHAIN (val), args, complain, in_decl);