* [PATCH] c++: Lost deprecated/unavailable attr in class tmpl [PR104682]
@ 2022-02-25 21:59 Marek Polacek
2022-02-28 16:13 ` Jason Merrill
0 siblings, 1 reply; 5+ messages in thread
From: Marek Polacek @ 2022-02-25 21:59 UTC (permalink / raw)
To: GCC Patches, Jason Merrill
[ 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:
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.
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
--
2.35.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] c++: Lost deprecated/unavailable attr in class tmpl [PR104682]
2022-02-25 21:59 [PATCH] c++: Lost deprecated/unavailable attr in class tmpl [PR104682] Marek Polacek
@ 2022-02-28 16:13 ` Jason Merrill
2022-02-28 16:18 ` Marek Polacek
2022-02-28 16:30 ` Iain Sandoe
0 siblings, 2 replies; 5+ messages in thread
From: Jason Merrill @ 2022-02-28 16:13 UTC (permalink / raw)
To: Marek Polacek, GCC Patches; +Cc: Iain Sandoe
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?
> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] c++: Lost deprecated/unavailable attr in class tmpl [PR104682]
2022-02-28 16:13 ` Jason Merrill
@ 2022-02-28 16:18 ` Marek Polacek
2022-02-28 16:30 ` Iain Sandoe
1 sibling, 0 replies; 5+ messages in thread
From: Marek Polacek @ 2022-02-28 16:18 UTC (permalink / raw)
To: Jason Merrill; +Cc: GCC Patches, Iain Sandoe
On Mon, Feb 28, 2022 at 12:13:36PM -0400, Jason Merrill 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?
FWIW, I'm in favor of treating deprecated/unavailable the same, that is,
adding unavailable...
> > is_late_template_attribute has
> >
> > /* But some attributes specifically apply to templates. */
> > && !is_attribute_p ("abi_tag", name)
> > && !is_attribute_p ("deprecated", name)
...here. But that really does seem like a GCC 13 change.
> > && !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.
Will do, thanks.
Marek
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] c++: Lost deprecated/unavailable attr in class tmpl [PR104682]
2022-02-28 16:13 ` Jason Merrill
2022-02-28 16:18 ` Marek Polacek
@ 2022-02-28 16:30 ` Iain Sandoe
2022-02-28 16:31 ` Marek Polacek
1 sibling, 1 reply; 5+ messages in thread
From: Iain Sandoe @ 2022-02-28 16:30 UTC (permalink / raw)
To: Jason Merrill; +Cc: Marek Polacek, GCC Patches
> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] c++: Lost deprecated/unavailable attr in class tmpl [PR104682]
2022-02-28 16:30 ` Iain Sandoe
@ 2022-02-28 16:31 ` Marek Polacek
0 siblings, 0 replies; 5+ messages in thread
From: Marek Polacek @ 2022-02-28 16:31 UTC (permalink / raw)
To: Iain Sandoe; +Cc: Jason Merrill, GCC Patches
On Mon, Feb 28, 2022 at 04:30:01PM +0000, Iain Sandoe wrote:
>
>
> > 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.
Thanks. I'll send a patch soon.
Marek
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-02-28 16:31 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-25 21:59 [PATCH] c++: Lost deprecated/unavailable attr in class tmpl [PR104682] Marek Polacek
2022-02-28 16:13 ` Jason Merrill
2022-02-28 16:18 ` Marek Polacek
2022-02-28 16:30 ` Iain Sandoe
2022-02-28 16:31 ` Marek Polacek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).