On Mon Jul 20, 2026 at 5:47 PM CEST, Jakub Jelinek wrote:
> On Mon, Jul 20, 2026 at 05:28:42PM +0200, Josef Melcr wrote:
>> gcc/ChangeLog:
>> 
>>      * attr-callback.cc (callback_edge_callee_has_attr): Add gnu
>>      namespace to lookup.
>>      (callback_fetch_attr_by_edge): Likewise.
>>      (handle_callback_attribute): Warn when the attribute is used
>>      without the gnu namespace, fix bounds check.
>>      * attr-callback.h (CALLBACK_ATTR_IDENT): Change name to 'callback_only'.
>>      * builtin-attrs.def (ATTR_CALLBACK): Likewise.
>>      * cgraph.cc (cgraph_edge::redirect_call_stmt_to_callee): Add gnu
>>      namespace to lookup.
>>      (cgraph_node::verify_node): Likewise.
>>      * doc/extend.texi: Add gnu::callback_only docs.
>>      * ipa-cp.cc (purge_useless_callback_edges): Add gnu namespace to
>>      lookup.
>>      * ipa-prop.cc (ipa_compute_jump_functions_for_edge): Likewise.
>>      * tree-core.h: Fix comment describing the ECF_CB flag.
>> 
>> gcc/testsuite/ChangeLog:
>> 
>>      * gcc.dg/attr-callback.c: New test.
>>      * gcc.dg/ipa/ipcp-cb-noprop.c: New test.
>>      * gcc.dg/ipa/ipcp-cb2.c: New test.
>> 
>> Signed-off-by: Josef Melcr <[email protected]>
>> --- a/gcc/attr-callback.cc
>> +++ b/gcc/attr-callback.cc
>> @@ -22,6 +22,7 @@
>>  #include "system.h"
>>  #include "coretypes.h"
>>  #include "backend.h"
>> +#include "tree-core.h"
>
> Why?
> tree.h already includes tree-core.h.

I missed the tree.h include right below, superfluous of course.  I will
remove it, sorry.

>
>>  #include "tree.h"
>>  #include "gimple.h"
>>  #include "alloc-pool.h"
>> @@ -87,7 +88,7 @@ callback_special_case_attr (tree decl)
>>  bool
>>  callback_edge_callee_has_attr (cgraph_edge *e)
>>  {
>> -  return lookup_attribute (CALLBACK_ATTR_IDENT,
>> +  return lookup_attribute ("gnu", CALLBACK_ATTR_IDENT,
>
> My preference would be to s/CALLBACK_ATTR_IDENT/"callback_only"/g now,
> we don't obfuscate through macros other attributes either.

Yeah, I don't expect another name change, so that can be done away with.
Will do.

>
>> @@ -190,8 +191,23 @@ get_nth_list_elem (tree list, unsigned idx)
>>     struct attribute_spec.handler.  */
>>  tree
>>  handle_callback_attribute (tree *node, tree name, tree args,
>> -                       int ARG_UNUSED (flags), bool *no_add_attrs)
>> +                       int flags, bool *no_add_attrs)
>>  {
>> +  bool in_gnu_namespace = flags & (ATTR_FLAG_CXX11 | ATTR_FLAG_BUILT_IN);
>> +
>> +  /* Reject the spelling __attribute__((callback_only (...))).  We want the 
>> user
>> +     to explicitly specify the gnu namespace to avoid clashes with clang's
>> +     implementation.  The namespace cannot be specified for builtins, ignore
>> +     that case.  */
>> +  if (!in_gnu_namespace)
>> +    {
>> +      warning (OPT_Wattributes,
>> +           "%<__attribute__((callback_only (...)))%> is not supported; use "
>> +           "%<[[gnu::callback_only (...)]]%> instead");
>
> Unsure if this is the best recommendation for C++98 or C89 to C17.
> For C89 to C17 to avoid warnings with -pedantic or errors with
> -pedantic-errors, one can use [[__extension__ gnu::callback_only (...)]]
> instead.  That doesn't work for C++98 though, in that case one needs
> to use __extension__ [[gnu::callback_only (...)]] (but it only works like
> that if the attribute is at the start of the function declaration, if
> it is specified in other locations, __extension__ needs to be separated
> from the attribute.

I didn't really think of that to be honest.  It's not very
user-friendly.  Maybe just permitting the __attribute__ syntax would be
for the best, I don't see many people using the attribute if this is
what it takes.

>
> Any reason why the handle_callback_attribute function is in the middle-end
> rather than say c-family/c-attribs.cc ?  In there you could
> emit different warning wording depending on c_dialect_cxx () and
> for e.g. C++ depending on cxx_dialect == cxx98 or in C on !flag_isoc23.

Not particularly.  I was inspired by the format attribute when I first
wrote the implementation for callback_only.  Format has its handler in a
separate file, so I took the same route.  It can be moved to
c-attribs.cc if you'd prefer that.  Then c-attribs.cc wouldn't have to
include attr-callback.h at all (once the macro is removed).

>
> Otherwise LGTM.
>
>       Jakub

Best regards,
Josef

Reply via email to