Hi Joseph,
(sorry for the long delay)
On 9/2/25 11:14, Joseph Myers wrote:
> On Fri, 22 Aug 2025, David Faust wrote:
>
>> + {
>> + /* Treat btf_type_tag applied to a function type as applying to the
>> + return type instead. Otherwise with GNU syntax there is no way to
>> + apply type_tag to the return type; the parser always associates it
>> + to the function_type node. btf_decl_tag can (should) be used to
>> + annotate the function itself. */
>> + tree ret_type = TREE_TYPE (*node);
>> + tree new_attrs = tree_cons (name, args, TYPE_ATTRIBUTES (ret_type));
>> + tree new_type = build_type_attribute_qual_variant (ret_type,
>> + new_attrs,
>> + TYPE_QUALS (ret_type));
>> + *node = lang_hooks.types.reconstruct_complex_type (*node, new_type);
>
> I don't think reconstruct_complex_type is actually right here; at least
> the C version is quite specific to vector attributes and will recurse down
> when given a pointer type, which is not what's wanted here (if the
> function returns a pointer, I think you want to stop at that point, not
> recurse down and end up replacing the innermost type). So you should be
> reconstructing the function type directly given the new return type rather
> than doing something that would do more reconstruction within the return
> type. Now c_reconstruct_complex_type does
>
> outer = c_build_function_type (inner, TYPE_ARG_TYPES (type),
> TYPE_NO_NAMED_ARGS_STDARG_P (type));
>
> for its reconstruction of a function type, which is using a function
> c_build_function_type not available for C++; I don't know what you should
> do here (apart from adding another language hook) to ensure the
> appropriate language-specific function type construction in this c-family
> code.
>
Yes, you are right. This does not work as I intended.
The more I think on this, the more I think that the better option may be to
simply delete this whole if () block which tries to do special handling for
functions, or to replace the body with a warning and ignore the attribute.
It was an attempt to solve a problem which is actually solved much better by
simply using the C23 attribute syntax, something that we are strongly
encouraging already for the btf_type_tag attribute for multiple reasons
boiling down to the fact that it is _much_ easier with the C23 syntax to
annotate exactly the desired type with btf_type_tag.
I looked into adding a new langhook, and it can be done. But with C23
attribute syntax in mind, from user perspective I would rather the compiler
inform me that I am doing something wrong, rather than it try to be clever
and slide the attribute to the return type.
So, I think I would change the above to (tests/documentation accordingly):
if (TREE_CODE (*node) == FUNCTION_TYPE || TREE_CODE (*node) == METHOD_TYPE)
{
warning (OPT_Wattributes,
"%qE attribute does not apply to functions", name);
*no_add_attrs = true;
return NULL_TREE;
}
What do you think? Is this an acceptable approach?
Maybe a message along the lines of below is also warranted:
"btf_type_tag attribute does not apply to functions; C23 attribute syntax
can be used to annotate the return type"
Thanks,
David
