diff mbox series

[RFC] c++: Disallow [[deprecated]] on types other than class/enum definitions [PR110345]

Message ID Zr5vgd6TSWjfppko@tucnak
State New
Headers show
Series [RFC] c++: Disallow [[deprecated]] on types other than class/enum definitions [PR110345] | expand

Commit Message

Jakub Jelinek Aug. 15, 2024, 9:13 p.m. UTC 
Hi!

For C++ 26 P2552R3 I went through all the spots (except modules) where
attribute-specifier-seq appears in the grammar and tried to construct
a testcase in all those spots, for now for [[deprecated]] attribute.

The patch below contains that testcase.  One needed change for this
particular attribute was that currently we handle [[deprecated]]
exactly the same as [[gnu::deprecated]], but for the latter unlike C++14
or later we allow it also on almost all types, while the standard
is strict and allows it only on
https://eel.is/c++draft/dcl.attr#deprecated-2
The attribute may be applied to the declaration of a class, a typedef-name,
a variable, a non-static data member, a function, a namespace,
an enumeration, an enumerator, a concept, or a template specialization.

The following patch just adds a pedwarn for the cases that gnu::deprecated
allows but C++14 disallows, so integral/floating/boolean types,
pointers/references, array types, function types etc.
Basically, for TYPE_P, if the attribute is applied in place (which means
the struct/union/class/enum definition), it is allowed, otherwise pedwarned.

The testcase still contains some FIXMEs I'd like to discuss.

I've tried to compile it also with latest clang and there is agreement in
most of the diagnostics, just at block scope (inside of foo) it doesn't
diagnose
  auto e = new int [n] [[deprecated]];
  auto e2 = new int [n] [[deprecated]] [42];
  [[deprecated]] lab:;
and at namespace scope
[[deprecated]];
I think that all feels like clang++ bug.
On the other side, clang++ diagnoses
enum B { B0 } [[deprecated]];
but GCC with all the patches I've posted today doesn't, is that a GCC bug?
We diagnose struct A { } [[deprecated]]; ...

The FIXMEs are where there is agreement with clang++, but I'm not sure.
One thing is I'm not sure if "a variable" above is meant to include function
parameters, and/or unused function parameters without a specified name,
function parameters inside of a function declaration rather than definition
and/or static data members.

Also unsure about
  [[deprecated]] int : 0;
at class scope, that isn't a non-static data member...

Thoughts on all of these FIXMEs?

I guess to mark the paper as implemented (or what has been already voted
into C++23 earlier) we'll need to add similar testcase for all the other
standard attributes and make sure we check what the attributes can appertain
to and what they can't.

Bootstrapped/regtested on x86_64-linux and i686-linux.

2024-08-15  Jakub Jelinek  <jakub@redhat.com>

	PR c++/110345
	* parser.cc (cp_parser_std_attribute): Don't transform
	[[deprecated]] into [[gnu::deprecated]].
	* tree.cc (handle_std_deprecated_attribute): New function.
	(std_attributes): Add deprecated entry.

	* g++.dg/cpp0x/attr-deprecated1.C: New test.


	Jakub

Comments

Jason Merrill Aug. 19, 2024, 9:05 p.m. UTC | #1 
On 8/15/24 5:13 PM, Jakub Jelinek wrote:
> Hi!
> 
> For C++ 26 P2552R3 I went through all the spots (except modules) where
> attribute-specifier-seq appears in the grammar and tried to construct
> a testcase in all those spots, for now for [[deprecated]] attribute.
> 
> The patch below contains that testcase.  One needed change for this
> particular attribute was that currently we handle [[deprecated]]
> exactly the same as [[gnu::deprecated]], but for the latter unlike C++14
> or later we allow it also on almost all types, while the standard
> is strict and allows it only on
> https://eel.is/c++draft/dcl.attr#deprecated-2
> The attribute may be applied to the declaration of a class, a typedef-name,
> a variable, a non-static data member, a function, a namespace,
> an enumeration, an enumerator, a concept, or a template specialization.
> 
> The following patch just adds a pedwarn for the cases that gnu::deprecated
> allows but C++14 disallows, so integral/floating/boolean types,
> pointers/references, array types, function types etc.
> Basically, for TYPE_P, if the attribute is applied in place (which means
> the struct/union/class/enum definition), it is allowed, otherwise pedwarned.
> 
> The testcase still contains some FIXMEs I'd like to discuss.
> 
> I've tried to compile it also with latest clang and there is agreement in
> most of the diagnostics, just at block scope (inside of foo) it doesn't
> diagnose
>    auto e = new int [n] [[deprecated]];
>    auto e2 = new int [n] [[deprecated]] [42];
>    [[deprecated]] lab:;
> and at namespace scope
> [[deprecated]];
> I think that all feels like clang++ bug.
> On the other side, clang++ diagnoses
> enum B { B0 } [[deprecated]];
> but GCC with all the patches I've posted today doesn't, is that a GCC bug?

I think so, yes.

> We diagnose struct A { } [[deprecated]]; ...

That seems correct.

> The FIXMEs are where there is agreement with clang++, but I'm not sure.
> One thing is I'm not sure if "a variable" above is meant to include function
> parameters, and/or unused function parameters without a specified name,
> function parameters inside of a function declaration rather than definition
> and/or static data members.

All of those, I believe.

> Also unsure about
>    [[deprecated]] int : 0;
> at class scope, that isn't a non-static data member...

Indeed, pedantically the standard says it can't apply to an unnamed 
bit-field.

Jason
diff mbox series

Patch

--- gcc/cp/parser.cc.jj	2024-08-15 18:56:12.254139651 +0200
+++ gcc/cp/parser.cc	2024-08-15 19:07:28.821875374 +0200
@@ -30340,12 +30340,11 @@  cp_parser_std_attribute (cp_parser *pars
 
       /* We used to treat C++11 noreturn attribute as equivalent to GNU's,
 	 but no longer: we have to be able to tell [[noreturn]] and
-	 __attribute__((noreturn)) apart.  */
-      /* C++14 deprecated attribute is equivalent to GNU's.  */
-      if (is_attribute_p ("deprecated", attr_id))
-	TREE_PURPOSE (TREE_PURPOSE (attribute)) = gnu_identifier;
+	 __attribute__((noreturn)) apart.
+	 Similarly for C++14 deprecated attribute, we need to emit extra
+	 diagnostics for [[deprecated]] compared to [[gnu::deprecated]].  */
       /* C++17 fallthrough attribute is equivalent to GNU's.  */
-      else if (is_attribute_p ("fallthrough", attr_id))
+      if (is_attribute_p ("fallthrough", attr_id))
 	TREE_PURPOSE (TREE_PURPOSE (attribute)) = gnu_identifier;
       /* C++23 assume attribute is equivalent to GNU's.  */
       else if (is_attribute_p ("assume", attr_id))
--- gcc/cp/tree.cc.jj	2024-08-15 17:36:40.109928397 +0200
+++ gcc/cp/tree.cc	2024-08-15 19:07:28.815875447 +0200
@@ -5087,6 +5087,22 @@  handle_likeliness_attribute (tree *node,
     return error_mark_node;
 }
 
+/* The C++14 [[deprecated]] attribute mostly maps to the GNU deprecated
+   attribute.  */
+
+static tree
+handle_std_deprecated_attribute (tree *node, tree name, tree args, int flags,
+				 bool *no_add_attrs)
+{
+  tree t = *node;
+  tree ret = handle_deprecated_attribute (node, name, args, flags,
+					  no_add_attrs);
+  if (TYPE_P (*node) && t != *node)
+    pedwarn (input_location, OPT_Wattributes,
+	     "%qE on a type other than class or enumeration definition", name);
+  return ret;
+}
+
 /* Table of valid C++ attributes.  */
 static const attribute_spec cxx_gnu_attributes[] =
 {
@@ -5110,6 +5126,8 @@  static const attribute_spec std_attribut
 {
   /* { name, min_len, max_len, decl_req, type_req, fn_type_req,
        affects_type_identity, handler, exclude } */
+  { "deprecated", 0, 1, false, false, false, false,
+    handle_std_deprecated_attribute, NULL },
   { "maybe_unused", 0, 0, false, false, false, false,
     handle_unused_attribute, NULL },
   { "nodiscard", 0, 1, false, false, false, false,
--- gcc/testsuite/g++.dg/cpp0x/attr-deprecated1.C.jj	2024-08-15 19:07:28.822875362 +0200
+++ gcc/testsuite/g++.dg/cpp0x/attr-deprecated1.C	2024-08-15 19:11:54.312666904 +0200
@@ -0,0 +1,126 @@ 
+// C++ 26 P2552R3 - On the ignorability of standard attributes
+// { dg-do compile { target c++11 } }
+
+int arr[2];
+struct S { int a, b; };
+S arr2[2];
+
+void
+foo (int n)
+{
+  [[deprecated]] int x1;
+  [[deprecated ("foobar")]] int x2;
+  [[deprecated (0)]] int x3;			// { dg-error "deprecated message is not a string" }
+						// { dg-error "expected string-literal before numeric constant" "" { target c++26 } .-1 }
+  [[deprecated ("foo", "bar", "baz")]] int x4;	// { dg-error "wrong number of arguments specified for 'deprecated' attribute" }
+  [[deprecated (0, 1, 2)]] int x5;		// { dg-error "wrong number of arguments specified for 'deprecated' attribute" }
+						// { dg-error "expected string-literal before numeric constant" "" { target c++26 } .-1 }
+
+  auto a = [] [[deprecated]] () {};
+  auto b = [] constexpr [[deprecated]] {};	// { dg-error "'deprecated' on a type other than class or enumeration definition" }
+						// { dg-error "parameter declaration before lambda declaration specifiers only optional with" "" { target c++20_down } .-1 }
+						// { dg-error "'constexpr' lambda only available with" "" { target c++14_down } .-2 }
+  auto c = [] noexcept [[deprecated]] {};	// { dg-error "'deprecated' on a type other than class or enumeration definition" }
+						// { dg-error "parameter declaration before lambda exception specification only optional with" "" { target c++20_down } .-1 }
+  auto d = [] () [[deprecated]] {};		// { dg-error "'deprecated' on a type other than class or enumeration definition" }
+  auto e = new int [n] [[deprecated]];		// { dg-warning "attributes ignored on outermost array type in new expression" }
+  auto e2 = new int [n] [[deprecated]] [42];	// { dg-warning "attributes ignored on outermost array type in new expression" }
+  auto f = new int [n][42] [[deprecated]];	// { dg-error "'deprecated' on a type other than class or enumeration definition" }
+  [[deprecated]];				// { dg-warning "attributes at the beginning of statement are ignored" }
+  [[deprecated]] {}				// { dg-warning "attributes at the beginning of statement are ignored" }
+  [[deprecated]] if (true) {}			// { dg-warning "attributes at the beginning of statement are ignored" }
+  [[deprecated]] while (false) {}		// { dg-warning "attributes at the beginning of statement are ignored" }
+  [[deprecated]] goto lab;			// { dg-warning "attributes at the beginning of statement are ignored" }
+  [[deprecated]] lab:;				// { dg-error "'deprecated' attribute ignored" }
+  [[deprecated]] try {} catch (int) {}		// { dg-warning "attributes at the beginning of statement are ignored" }
+  if ([[deprecated]] int x = 0) {}
+  switch (n)
+    {
+    [[deprecated]] case 1:			// { dg-error "'deprecated' attribute ignored" }
+    [[deprecated]] break;			// { dg-warning "attributes at the beginning of statement are ignored" }
+    [[deprecated]] default:			// { dg-error "'deprecated' attribute ignored" }
+	 break;
+    }
+  for ([[deprecated]] auto a : arr) {}
+  for ([[deprecated]] auto [a, b] : arr2) {}	// { dg-error "structured bindings only available with" "" { target c++14_down } }
+  [[deprecated]] asm ("");			// { dg-warning "attributes ignored on 'asm' declaration" }
+  try {} catch ([[deprecated]] int x) {}
+  try {} catch ([[deprecated]] int) {}		// FIXME: shouldn't this be diagnosed?
+}
+
+[[deprecated]] int bar ();
+using foobar [[deprecated]] = int;
+[[deprecated]] int a;
+[[deprecated]] auto [b, c] = arr;		// { dg-error "structured bindings only available with" "" { target c++14_down } }
+[[deprecated]];					// { dg-warning "attribute ignored" }
+inline [[deprecated]] void baz () {}		// { dg-warning "attribute ignored" }
+						// { dg-error "standard attributes in middle of decl-specifiers" "" { target *-*-* } .-1 }
+constexpr [[deprecated]] int qux () { return 0; }	// { dg-warning "attribute ignored" }
+						// { dg-error "standard attributes in middle of decl-specifiers" "" { target *-*-* } .-1 }
+int [[deprecated]] d;				// { dg-warning "attribute ignored" }
+int const [[deprecated]] e = 1;			// { dg-warning "attribute ignored" }
+struct A {} [[deprecated]];			// { dg-warning "attribute ignored in declaration of 'struct A'" }
+enum B { B0 } [[deprecated]];			// FIXME: clang diagnoses this, is it modification of enum after it has been finalized?
+struct [[deprecated]] C {};
+int f [[deprecated]];
+int g[2] [[deprecated]];			// { dg-error "'deprecated' on a type other than class or enumeration definition" }
+int g2 [[deprecated]] [2];
+int corge () [[deprecated]];			// { dg-error "'deprecated' on a type other than class or enumeration definition" }
+int *[[deprecated]] h;				// { dg-error "'deprecated' on a type other than class or enumeration definition" }
+int & [[deprecated]] i = f;			// { dg-error "'deprecated' on a type other than class or enumeration definition" }
+						// { dg-warning "'f' is deprecated" "" { target *-*-* } .-1 }
+int && [[deprecated]] j = 0;			// { dg-error "'deprecated' on a type other than class or enumeration definition" }
+int S::* [[deprecated]] k;			// { dg-error "'deprecated' on a type other than class or enumeration definition" }
+auto l = sizeof (int [2] [[deprecated]]);	// { dg-error "'deprecated' on a type other than class or enumeration definition" }
+int freddy ([[deprecated]] int a,		// FIXME: parameters aren't listed, can it apply to them?
+	    [[deprecated]] int,			// FIXME: what about this?
+	    [[deprecated]] int c = 0,
+	    [[deprecated]] int = 0);
+void
+corge ([[deprecated]] int a,
+       [[deprecated]] int,
+       [[deprecated]] int c = 0,
+       [[deprecated]] int = 0)
+{
+}
+[[deprecated]] void
+garply ()
+{
+}
+enum [[deprecated]] D { D0 };
+enum class [[deprecated]] E { E0 };
+enum F {};
+enum [[deprecated]] F;				// { dg-warning "type attributes ignored after type is already defined" }
+enum G {
+  G0 [[deprecated]],
+  G1 [[deprecated]] = 2
+};
+namespace [[deprecated]] H { using H0 = int; }
+namespace [[deprecated]] {}			// { dg-warning "ignoring 'deprecated' attribute on anonymous namespace" }
+[[deprecated]] using namespace H;		// { dg-warning "'deprecated' attribute directive ignored" }
+						// { dg-warning "'H' is deprecated" "" { target *-*-* } .-1 }
+struct [[deprecated]] I
+{
+  [[deprecated]];				// { dg-error "declaration does not declare anything" }
+  [[deprecated]] int i;
+  [[deprecated]] int foo ();
+  [[deprecated]] int bar () { return 1; }
+  [[deprecated]] int : 0;			// FIXME: Shouldn't this be diagnosed?
+  [[deprecated]] int i2 : 5;
+  [[deprecated]] static int i3;			// FIXME: the paper says: For example, no compiler diagnoses [[deprecated]] or [[maybe_unused]] on static data members
+						// but is that correct? Aren't static data members variables?
+  static int i4;
+};
+[[deprecated]] int I::i4 = 0;
+struct J : [[deprecated]] C {};			// { dg-warning "attributes on base specifiers are ignored" }
+#if __cpp_concepts >= 201907L
+template <typename T>
+concept K [[deprecated]] = requires { true; };
+#endif
+typedef int L [[deprecated]];
+template <typename T>
+struct M {};
+template <>
+struct [[deprecated]] M<int> { int m; };
+typedef int N[2] [[deprecated]];		// { dg-error "'deprecated' on a type other than class or enumeration definition" }
+typedef int O [[deprecated]] [2];