Attributes on templates are mishandled

[Quuxplusone]

Bugzilla Link	38302
Version	trunk
OS	All
CC	@DougGregor

Extended Description

// https://godbolt.org/g/pqog4L
template<class T>
[[noreturn]] void one();

template<>
void one<int>() {}

template<class T>
void two();

template<>
[[noreturn]] void two<int>() {}

As far as I can tell, this program produces bogus diagnostics in both directions.
First, the explicit specialization one<int> seems to be inheriting [[noreturn]] from the primary template; I believe this is unwanted behavior.

    test.cc:2:18: warning: function declared 'noreturn' should not return [-Winvalid-noreturn]
    void one<int>() {}
                     ^

Second, Clang complains that the explicit specialization two<int> is marked [[noreturn]] when its "first declaration" is not so marked. Clang seems to be considering the primary template as the "first" declaration of every specialization (even though the accompanying note nonsensically points to the specialization's own declaration). This is problematic, because maybe the programmer wants to apply [[noreturn]] NOT to ALL possible instantiations of the primary template, but only to this one explicit specialization.

    test.cc:9:3: error: function declared '[[noreturn]]' after its first declaration
    [[noreturn]] void two<int>() {}
      ^
    test.cc:9:19: note: declaration missing '[[noreturn]]' attribute is here
    [[noreturn]] void two<int>() {}
                      ^

After some discussion on Slack, I think what ought to happen here is that the attribute on the primary template ought to be applied only to instantiations that come directly from that primary (unspecialized) template; and the attribute on the explicit specialization ought to be applied only to that explicit specialization. But we see that Clang doesn't do either of those things.

This is highly relevant to my interests because of P1144 [[trivially_relocatable]], and I'd love to put together a patch to fix it, but at least at the moment I'm completely lost as to where I would even start looking.

See also: #21132

[llvmbot]

@llvm/issue-subscribers-clang-frontend

[aaronpuchert]

Came across the same issue. Especially when adding [[noreturn]] to specializations I don't see any reason to disallow that.

The error is likely referring to [dcl.attr.noreturn]p1 and might not necessarily affect other attributes. My understanding is that an explicit specialization is not a redeclaration and thus the rule does not apply here.

[aaronpuchert]

My understanding is that an explicit specialization is not a redeclaration and thus the rule does not apply here.

Can't find explicit language for that, but [temp.expl.spec]p13 says that e.g. inline and constexpr on explicit specializations are independent, suggesting that these are not redeclarations.

@AaronBallman, any thoughts on this?

[AaronBallman]

My understanding is that an explicit specialization is not a redeclaration and thus the rule does not apply here.

A function template is not a function (template) declaration (https://eel.is/c++draft/temp.fct#general-1), see also https://en.cppreference.com/w/cpp/language/function_template for more human-readable information on it.

I think https://eel.is/c++draft/temp.expl.spec#13 is intended to clarify that the explicit specialization is independent of the primary template, but it's not clear whether attributes are explicitly elided from that list or an accidental omission. I've raised a question on the core reflectors.

[AaronBallman]

From the Core reflector (not an official committee response, just early feedback):

I think that https://eel.is/c++draft/temp.expl.spec#13 implies that the attributes of the specialization are also independent of the primary template, but I can't find anything in the standard which says what should happen one way or the other.

I think that's plainly the right interpretation, since explicit specializations must be declared (though not defined) everywhere they are used. Accordingly, there's never a need to consult the primary template declaration.

So it seems like we've confirmed this report, assuming nobody in Core disagrees.

[aaronpuchert]

The error seems to come out of Sema::MergeFunctionDecl. So is the problem that we call this for specializations at all, or should we just exclude specializations for CXX11NoReturnAttr?

If specializations are generally considered as independent, we should probably not even be there. But maybe there is a good reason why we are.

Of course fully independent they are not, e.g.

template<typename T> T f();
template<> double f<int>();

is not allowed. The signatures have to match, if only to find which template we're specializing. We also can't override the storage class. But it seems like attributes can generally be independent, and that this is generally desirable. After all, if there is need for a specialization, it might also behave differently.

[AaronBallman]

So is the problem that we call this for specializations at all, or should we just exclude specializations for CXX11NoReturnAttr?

I don't think we should handle attributes on a case by case basis (not unless we absolutely have to), but I think we have to call MergeFunctionDecl() because that's doing the type checking to ensure that the two functions with the same name are or are not a valid redeclaration and we have special logic in there for function template specializations. I'm not yet certain what the correct approach is here.

After all, if there is need for a specialization, it might also behave differently.

That's my thinking in general, but then you have a case like: https://godbolt.org/z/ba19jn16c where the user may reasonably have thought they were marking all specializations as being deprecated.

That said, I think there's more power to requiring users to specify the attributes on the explicit specializations. If we we don't add it automatically and that's wrong, the user can add it themselves. However, if we add it automatically and that's wrong, the user has no recourse.

[aaronpuchert]

That's my thinking in general, but then you have a case like: https://godbolt.org/z/ba19jn16c where the user may reasonably have thought they were marking all specializations as being deprecated.

Good point. Maybe there is a difference between "general" declaration attributes ([[maybe_unused]], [[deprecated]]) and function declaration attributes ([[noreturn]], or many static analysis attributes) here: the specialization still refers to the original (template) declaration and thus should probably inherit general declaration attributes, but it should not inherit function declaration attributes, because the specialization is a "new" function.

[aaronpuchert]

Perhaps we can say that "general declaration attributes" like [[maybe_unused]] or [[deprecated]] are considered to sit on the outer template declaration, whereas function declaration attributes are considered to sit on the contained template function. Syntactically of course they all sit on the function, because (unless I'm missing something) it's not possible to put attributes on the template itself.

As an example, for

template<typename T>
[[deprecated]] [[noreturn]] int f();

we have

FunctionTemplateDecl f
|-TemplateTypeParmDecl typename depth 0 index 0 T
`-FunctionDecl f 'int ()'
  |-DeprecatedAttr "" ""
  `-CXX11NoReturnAttr

and I would suggest to make this

FunctionTemplateDecl f
|-TemplateTypeParmDecl typename depth 0 index 0 T
|-DeprecatedAttr "" ""
`-FunctionDecl f 'int ()'
  `-CXX11NoReturnAttr

at least in spirit, if it would be a bad idea to actually change the AST.

Then we could also warn on

template<typename T>
class [[deprecated]] X;

template<template<typename> class T>
void f();

void test() {
    f<X>();
}

like GCC does. This doesn't instantiate the deprecated class. Understanding the attribute as belonging to the template declaration itself would justify a warning though. This is surely a bit contrived, because templates are usually instantiated and not just referenced.

[AaronBallman]

I'd like to hear what Core thinks about this, because it turns out I'm not the only person on the committee who would like the standard to say something about this. It's a bit difficult for me to see a general rule to apply here because standard attributes and vendor-specified attributes are pretty decent beasts. For example, the standard currently doesn't expose any type attributes, but vendor-specific calling convention attributes may be written on the type (this seems like a case where you'd want the specialization to inherit the attribute from the primary template), but [[maybe_unused]] seems like something you could potentially want to use on a case-by-case basis for specializations (maybe one of the specializations only exists to be called in an assert() macro).

While thinking on this, I realized there was a related case the standard also is pretty quiet about -- attributes written on the parameters of the primary function template. There's implementation divergence there too: https://godbolt.org/z/1eTrTorn1

So I've gone ahead and filed a core issue at cplusplus/CWG#68 to track this on the committee's side.

My current thinking is that we do not want attributes from the primary template to also be applied to an explicit specialization in general. That removes expressivity from the explicit specialization, and I think there are too many situations in which the specialization may need a different set of attributes than the primary template. I would really like to avoid trying to do anything on an attribute-by-attribute basis, but the one situation I'm still thinking about heavily is with type attributes (thinking about things like calling convention attributes).

[AaronBallman]

So I've gone ahead and filed a core issue at cplusplus/CWG#68 to track this on the committee's side.

This is now CWG2604.

[aaronpuchert]

So I've gone ahead and filed a core issue at cplusplus/CWG#68 to track this on the committee's side.

Thanks!

My current thinking is that we do not want attributes from the primary template to also be applied to an explicit specialization in general. That removes expressivity from the explicit specialization, and I think there are too many situations in which the specialization may need a different set of attributes than the primary template. I would really like to avoid trying to do anything on an attribute-by-attribute basis, but the one situation I'm still thinking about heavily is with type attributes (thinking about things like calling convention attributes).

Fully agreed. As we consider calling conventions as non-erasable part of the function type and allow overloading on calling conventions, we probably need to require them consistent across specializations.

Erasable type attributes however (like nullability) would not need to be consistent. (Though we could still require it.)

[AaronBallman]

Erasable type attributes however (like nullability) would not need to be consistent. (Though we could still require it.)

Agreed, there are type attributes which are erasable as well. There are also type attributes which act as qualifiers (like address_space). So it's a complicated space where it may be impossible to come up with a one-size-fits-all approach.