Re: [PATCH] c++: Lost deprecated/unavailable attr in class tmpl [PR104682]

[Iain Sandoe <iain@sandoe.co.uk>]

> On 28 Feb 2022, at 16:13, Jason Merrill <jason@redhat.com> wrote:
> 
> On 2/25/22 17:59, Marek Polacek wrote:
>> [ Most likely a GCC 13 patch, but I'm posting it now so that I don't lose it. ]
>> When looking into the other PR I noticed that we fail to give a warning
>> for a deprecated enumerator when the enum is in a class template.  This
>> only happens when the attribute doesn't have an argument.  The reason is
>> that when we tsubst_enum, we create a new enumerator:
>>       build_enumerator (DECL_NAME (decl), value, newtag,
>>            DECL_ATTRIBUTES (decl), DECL_SOURCE_LOCATION (decl));
>> but DECL_ATTRIBUTES (decl) is null when the attribute was provided
>> without an argument -- in that case it simply melts into a tree flag.
>> handle_deprecated_attribute has:
>>       if (!args)
>>          *no_add_attrs = true;
>> so the attribute isn't retained and we lose it when tsubsting.  Same
>> thing when the attribute is on the enum itself.
>> Attribute unavailable is a similar case, but it's different in that
>> it can be a late attribute whereas "deprecated" can't:
> 
> Iain, was this difference intentional?

The intent was to treat the two attributes the same way - so any difference
is unintentional.

Iain

> 
>> is_late_template_attribute has
>>                 /* But some attributes specifically apply to templates.  */
>>                 && !is_attribute_p ("abi_tag", name)
>>                 && !is_attribute_p ("deprecated", name)
>>                 && !is_attribute_p ("visibility", name))
>>          return true;
>>        else
>>          return false;
>> which looks strange, but attr-unavailable-9.C tests that we don't error when
>> the attribute is applied on a template.
>> Bootstrapped/regtested on x86_64-pc-linux-gnu.
> 
> This looks extremely safe, so let's go ahead and apply it to trunk.
> 
>> 	PR c++/104682
>> gcc/cp/ChangeLog:
>> 	* cp-tree.h (build_enumerator): Adjust.
>> 	* decl.cc (finish_enum): Make it return the new decl.
>> 	* pt.cc (tsubst_enum): Propagate TREE_DEPRECATED and TREE_UNAVAILABLE.
>> gcc/testsuite/ChangeLog:
>> 	* g++.dg/ext/attr-unavailable-10.C: New test.
>> 	* g++.dg/ext/attr-unavailable-11.C: New test.
>> 	* g++.dg/warn/deprecated-17.C: New test.
>> 	* g++.dg/warn/deprecated-18.C: New test.
>> ---
>>  gcc/cp/cp-tree.h                              |  2 +-
>>  gcc/cp/decl.cc                                |  4 +-
>>  gcc/cp/pt.cc                                  | 17 +++++++--
>>  .../g++.dg/ext/attr-unavailable-10.C          | 22 +++++++++++
>>  .../g++.dg/ext/attr-unavailable-11.C          | 22 +++++++++++
>>  gcc/testsuite/g++.dg/warn/deprecated-17.C     | 35 ++++++++++++++++++
>>  gcc/testsuite/g++.dg/warn/deprecated-18.C     | 37 +++++++++++++++++++
>>  7 files changed, 133 insertions(+), 6 deletions(-)
>>  create mode 100644 gcc/testsuite/g++.dg/ext/attr-unavailable-10.C
>>  create mode 100644 gcc/testsuite/g++.dg/ext/attr-unavailable-11.C
>>  create mode 100644 gcc/testsuite/g++.dg/warn/deprecated-17.C
>>  create mode 100644 gcc/testsuite/g++.dg/warn/deprecated-18.C
>> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
>> index 37d462fca6e..80994e94793 100644
>> --- a/gcc/cp/cp-tree.h
>> +++ b/gcc/cp/cp-tree.h
>> @@ -6833,7 +6833,7 @@ extern void xref_basetypes			(tree, tree);
>>  extern tree start_enum				(tree, tree, tree, tree, bool, bool *);
>>  extern void finish_enum_value_list		(tree);
>>  extern void finish_enum				(tree);
>> -extern void build_enumerator			(tree, tree, tree, tree, location_t);
>> +extern tree build_enumerator			(tree, tree, tree, tree, location_t);
>>  extern tree lookup_enumerator			(tree, tree);
>>  extern bool start_preparsed_function		(tree, tree, int);
>>  extern bool start_function			(cp_decl_specifier_seq *,
>> diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
>> index 7b48b56231b..7f80f9d4d7a 100644
>> --- a/gcc/cp/decl.cc
>> +++ b/gcc/cp/decl.cc
>> @@ -16409,7 +16409,7 @@ finish_enum (tree enumtype)
>>     Apply ATTRIBUTES if available.  LOC is the location of NAME.
>>     Assignment of sequential values by default is handled here.  */
>>  -void
>> +tree
>>  build_enumerator (tree name, tree value, tree enumtype, tree attributes,
>>  		  location_t loc)
>>  {
>> @@ -16611,6 +16611,8 @@ incremented enumerator value is too large for %<long%>"));
>>      /* Add this enumeration constant to the list for this type.  */
>>    TYPE_VALUES (enumtype) = tree_cons (name, decl, TYPE_VALUES (enumtype));
>> +
>> +  return decl;
>>  }
>>    /* Look for an enumerator with the given NAME within the enumeration
>> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
>> index 70f02db8757..8fb17349ee1 100644
>> --- a/gcc/cp/pt.cc
>> +++ b/gcc/cp/pt.cc
>> @@ -26944,9 +26944,8 @@ tsubst_enum (tree tag, tree newtag, tree args)
>>    for (e = TYPE_VALUES (tag); e; e = TREE_CHAIN (e))
>>      {
>>        tree value;
>> -      tree decl;
>> +      tree decl = TREE_VALUE (e);
>>  -      decl = TREE_VALUE (e);
>>        /* Note that in a template enum, the TREE_VALUE is the
>>  	 CONST_DECL, not the corresponding INTEGER_CST.  */
>>        value = tsubst_expr (DECL_INITIAL (decl),
>> @@ -26958,8 +26957,14 @@ tsubst_enum (tree tag, tree newtag, tree args)
>>          /* Actually build the enumerator itself.  Here we're assuming that
>>  	 enumerators can't have dependent attributes.  */
>> -      build_enumerator (DECL_NAME (decl), value, newtag,
>> -			DECL_ATTRIBUTES (decl), DECL_SOURCE_LOCATION (decl));
>> +      tree newdecl = build_enumerator (DECL_NAME (decl), value, newtag,
>> +				       DECL_ATTRIBUTES (decl),
>> +				       DECL_SOURCE_LOCATION (decl));
>> +      /* Attribute deprecated without an argument isn't sticky: it'll
>> +	 melt into a tree flag, so we need to propagate the flag here,
>> +	 since we just created a new enumerator.  */
>> +      TREE_DEPRECATED (newdecl) = TREE_DEPRECATED (decl);
>> +      TREE_UNAVAILABLE (newdecl) = TREE_UNAVAILABLE (decl);
>>      }
>>      if (SCOPED_ENUM_P (newtag))
>> @@ -26970,6 +26975,10 @@ tsubst_enum (tree tag, tree newtag, tree args)
>>      DECL_SOURCE_LOCATION (TYPE_NAME (newtag))
>>      = DECL_SOURCE_LOCATION (TYPE_NAME (tag));
>> +  TREE_DEPRECATED (newtag) = TREE_DEPRECATED (tag);
>> +  /* We don't need to propagate TREE_UNAVAILABLE here, because it is, unlike
>> +     deprecated, applied at instantiation time rather than template
>> +     definition time.  */
>>  }
>>    /* DECL is a FUNCTION_DECL that is a template specialization.  Return
>> diff --git a/gcc/testsuite/g++.dg/ext/attr-unavailable-10.C b/gcc/testsuite/g++.dg/ext/attr-unavailable-10.C
>> new file mode 100644
>> index 00000000000..cfb99476b9f
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/ext/attr-unavailable-10.C
>> @@ -0,0 +1,22 @@
>> +// PR c++/104682
>> +// { dg-do compile { target c++11 } }
>> +
>> +template<typename>
>> +struct S {
>> +  enum E1 {
>> +    A __attribute__((unavailable))
>> +  };
>> +};
>> +
>> +struct S2 {
>> +  enum E2 {
>> +    A __attribute__((unavailable))
>> +  };
>> +};
>> +
>> +void
>> +g ()
>> +{
>> +  auto a1 = S<int>::E1::A; // { dg-error "is unavailable" }
>> +  auto b1 = S2::E2::A; // { dg-error "is unavailable" }
>> +}
>> diff --git a/gcc/testsuite/g++.dg/ext/attr-unavailable-11.C b/gcc/testsuite/g++.dg/ext/attr-unavailable-11.C
>> new file mode 100644
>> index 00000000000..fce24528908
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/ext/attr-unavailable-11.C
>> @@ -0,0 +1,22 @@
>> +// PR c++/104682
>> +// { dg-do compile { target c++11 } }
>> +
>> +template<typename>
>> +struct S {
>> +  enum B {
>> +    A
>> +  } __attribute__((unavailable)) ;
>> +};
>> +
>> +struct S2 {
>> +  enum B {
>> +    A
>> +  } __attribute__((unavailable));
>> +};
>> +
>> +void
>> +g ()
>> +{
>> +  S<int>::B a1; // { dg-error "is unavailable" }
>> +  S2::B a2; // { dg-error "is unavailable" }
>> +}
>> diff --git a/gcc/testsuite/g++.dg/warn/deprecated-17.C b/gcc/testsuite/g++.dg/warn/deprecated-17.C
>> new file mode 100644
>> index 00000000000..c4e768538fa
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/warn/deprecated-17.C
>> @@ -0,0 +1,35 @@
>> +// PR c++/104682
>> +// { dg-do compile { target c++11 } }
>> +
>> +template<typename>
>> +struct S {
>> +  enum E1 {
>> +    A __attribute__((deprecated)),
>> +    B __attribute__((deprecated("B"))),
>> +    C [[deprecated]],
>> +    D [[deprecated("D")]]
>> +  };
>> +};
>> +
>> +struct S2 {
>> +  enum E2 {
>> +    A __attribute__((deprecated)),
>> +    B __attribute__((deprecated("B"))),
>> +    C [[deprecated]],
>> +    D [[deprecated("D")]]
>> +  };
>> +};
>> +
>> +void
>> +g ()
>> +{
>> +  auto a1 = S<int>::E1::A; // { dg-warning "is deprecated" }
>> +  auto a2 = S<int>::E1::B; // { dg-warning "is deprecated" }
>> +  auto a3 = S<int>::E1::C; // { dg-warning "is deprecated" }
>> +  auto a4 = S<int>::E1::D; // { dg-warning "is deprecated" }
>> +
>> +  auto b1 = S2::A; // { dg-warning "is deprecated" }
>> +  auto b2 = S2::B; // { dg-warning "is deprecated" }
>> +  auto b3 = S2::C; // { dg-warning "is deprecated" }
>> +  auto b4 = S2::D; // { dg-warning "is deprecated" }
>> +}
>> diff --git a/gcc/testsuite/g++.dg/warn/deprecated-18.C b/gcc/testsuite/g++.dg/warn/deprecated-18.C
>> new file mode 100644
>> index 00000000000..0d101a9cc90
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/warn/deprecated-18.C
>> @@ -0,0 +1,37 @@
>> +// PR c++/104682
>> +// { dg-do compile { target c++11 } }
>> +
>> +template<typename>
>> +struct S {
>> +  enum B {
>> +    A
>> +  } __attribute__((deprecated)) ;
>> +};
>> +
>> +struct S2 {
>> +  enum B {
>> +    A
>> +  } __attribute__((deprecated));
>> +};
>> +
>> +template<typename>
>> +struct S3 {
>> +  enum [[deprecated]] B {
>> +    A
>> +  };
>> +};
>> +
>> +struct S4 {
>> +  enum [[deprecated]] B {
>> +    A
>> +  };
>> +};
>> +
>> +void
>> +g ()
>> +{
>> +  S<int>::B a1; // { dg-warning "is deprecated" }
>> +  S2::B a2; // { dg-warning "is deprecated" }
>> +  S3<int>::B a3; // { dg-warning "is deprecated" }
>> +  S4::B a4; // { dg-warning "is deprecated" }
>> +}
>> base-commit: f62115c9b770a66c5378f78a2d5866243d560573