On Wed, Jan 31, 2024 at 12:21:38PM -0800, Andi Kleen wrote:
> > > + case RID_RETURN:
> > > + {
> > > + bool musttail_p = false;
> > > + std_attrs = process_stmt_hotness_attribute (std_attrs, attrs_loc);
> > > + if (lookup_attribute ("", "musttail", std_attrs))
> > > + {
> > > + musttail_p = true;
> > > + std_attrs = remove_attribute ("", "musttail", std_attrs);
> > > + }
Using "" looks wrong to me, that is for standard attributes which
are also gnu attributes, say [[noreturn]]/[[gnu::noreturn]].
That is not the case here. Even the __attribute__((musttail)) form will have
gnu namespace.
> > > + // support this for compatibility
> > > + if (lookup_attribute ("clang", "musttail", std_attrs))
> > > + {
> > > + musttail_p = true;
> > > + std_attrs = remove_attribute ("clang", "musttail", std_attrs);
> > > + }
> >
> > Doing lookup_attribute unconditionally twice seems like a lot.
> > You could do just lookup_attribute ("musttail", std_attrs) and then
> > check get_attribute_namespace() == nullptr/gnu_identifier?
I agree with Marek here. The fact that it is most often NULL std_attrs is
indeed already optimized by lookup_attribute, but people write all kinds of
code. The remove_attribute can be done separately of course.
Though, I'd also prefer not to add clang attributes, just add gnu ones.
Jakub
