* [PATCH] attribs: Allow optional second arg for attr deprecated [PR102049]
@ 2021-09-23 16:16 Marek Polacek
2021-10-11 15:17 ` Marek Polacek
0 siblings, 1 reply; 5+ messages in thread
From: Marek Polacek @ 2021-09-23 16:16 UTC (permalink / raw)
To: GCC Patches
Clang implements something we don't have:
__attribute__((deprecated("message", "replacement")));
which seems pretty neat so I wrote this patch to add it to gcc.
It doesn't allow the optional second argument in the standard [[]]
form so as not to clash with possible future standard additions.
I had hoped we could print a nice fix-it replacement hint, but that
won't be possible until warn_deprecated_use gets something better than
input_location.
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
PR c++/102049
gcc/c-family/ChangeLog:
* c-attribs.c (c_common_attribute_table): Increase max_len for
deprecated.
(handle_deprecated_attribute): Allow an optional second argument
in the GNU form of attribute deprecated.
gcc/c/ChangeLog:
* c-parser.c (c_parser_std_attribute): Give a diagnostic when
the standard form of an attribute deprecated has a second argument.
gcc/ChangeLog:
* doc/extend.texi: Document attribute deprecated with an
optional second argument.
* tree.c (warn_deprecated_use): Print the replacement argument,
if any.
gcc/testsuite/ChangeLog:
* gcc.dg/c2x-attr-deprecated-3.c: Adjust dg-error.
* c-c++-common/Wdeprecated-arg-1.c: New test.
---
gcc/c-family/c-attribs.c | 17 ++++++++++++-
gcc/c/c-parser.c | 8 ++++++
gcc/doc/extend.texi | 24 ++++++++++++++++++
.../c-c++-common/Wdeprecated-arg-1.c | 21 ++++++++++++++++
gcc/testsuite/gcc.dg/c2x-attr-deprecated-3.c | 2 +-
gcc/tree.c | 25 +++++++++++++++----
6 files changed, 90 insertions(+), 7 deletions(-)
create mode 100644 gcc/testsuite/c-c++-common/Wdeprecated-arg-1.c
diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index 007b928c54b..ef857a9ae2c 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -409,7 +409,7 @@ const struct attribute_spec c_common_attribute_table[] =
to prevent its usage in source code. */
{ "no vops", 0, 0, true, false, false, false,
handle_novops_attribute, NULL },
- { "deprecated", 0, 1, false, false, false, false,
+ { "deprecated", 0, 2, false, false, false, false,
handle_deprecated_attribute, NULL },
{ "unavailable", 0, 1, false, false, false, false,
handle_unavailable_attribute, NULL },
@@ -4107,6 +4107,21 @@ handle_deprecated_attribute (tree *node, tree name,
error ("deprecated message is not a string");
*no_add_attrs = true;
}
+ else if (TREE_CHAIN (args) != NULL_TREE)
+ {
+ /* We allow an optional second argument in the GNU form of
+ attribute deprecated, which specifies the replacement. */
+ if (flags & ATTR_FLAG_CXX11)
+ {
+ error ("replacement argument only allowed in GNU attributes");
+ *no_add_attrs = true;
+ }
+ else if (TREE_CODE (TREE_VALUE (TREE_CHAIN (args))) != STRING_CST)
+ {
+ error ("replacement argument is not a string");
+ *no_add_attrs = true;
+ }
+ }
if (DECL_P (*node))
{
diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index fa29d2c15fc..2b47f01d166 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -4952,6 +4952,14 @@ c_parser_std_attribute (c_parser *parser, bool for_tm)
TREE_VALUE (attribute)
= c_parser_attribute_arguments (parser, takes_identifier,
require_string, false);
+ if (c_parser_next_token_is (parser, CPP_COMMA)
+ && strcmp (IDENTIFIER_POINTER (name), "deprecated") == 0)
+ {
+ error_at (open_loc, "replacement argument only allowed in "
+ "GNU attributes");
+ c_parser_skip_until_found (parser, CPP_CLOSE_PAREN, NULL);
+ return error_mark_node;
+ }
}
else
c_parser_balanced_token_sequence (parser);
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 9501a60f20e..7d399f4b2bc 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -2860,6 +2860,7 @@ StrongAlias (allocate, alloc);
@item deprecated
@itemx deprecated (@var{msg})
+@itemx deprecated (@var{msg}, @var{replacement})
@cindex @code{deprecated} function attribute
The @code{deprecated} attribute results in a warning if the function
is used anywhere in the source file. This is useful when identifying
@@ -2880,6 +2881,13 @@ results in a warning on line 3 but not line 2. The optional @var{msg}
argument, which must be a string, is printed in the warning if
present.
+This attribute accepts an optional second argument which specifies
+the replacement that ought to be used instead:
+
+@smallexample
+int old_fn () __attribute__ ((deprecated("unsafe", "new_fn")));
+@end smallexample
+
The @code{deprecated} attribute can also be used for variables and
types (@pxref{Variable Attributes}, @pxref{Type Attributes}.)
@@ -7409,6 +7417,7 @@ attribute is also not copied. @xref{Common Function Attributes}.
@item deprecated
@itemx deprecated (@var{msg})
+@itemx deprecated (@var{msg}, @var{replacement})
@cindex @code{deprecated} variable attribute
The @code{deprecated} attribute results in a warning if the variable
is used anywhere in the source file. This is useful when identifying
@@ -7429,6 +7438,13 @@ results in a warning on line 3 but not line 2. The optional @var{msg}
argument, which must be a string, is printed in the warning if
present.
+This attribute accepts an optional second argument which specifies
+the replacement that ought to be used instead:
+
+@smallexample
+extern int old_var __attribute__ ((deprecated("unsafe", "new_var")));
+@end smallexample
+
The @code{deprecated} attribute can also be used for functions and
types (@pxref{Common Function Attributes},
@pxref{Common Type Attributes}).
@@ -8492,6 +8508,7 @@ struct __attribute__ ((copy ( (struct A *)0)) B @{ /* @r{@dots{}} */ @};
@item deprecated
@itemx deprecated (@var{msg})
+@itemx deprecated (@var{msg}, @var{replacement})
@cindex @code{deprecated} type attribute
The @code{deprecated} attribute results in a warning if the type
is used anywhere in the source file. This is useful when identifying
@@ -8522,6 +8539,13 @@ present. Control characters in the string will be replaced with
escape sequences, and if the @option{-fmessage-length} option is set
to 0 (its default value) then any newline characters will be ignored.
+This attribute accepts an optional second argument which specifies
+the replacement that ought to be used instead:
+
+@smallexample
+typedef int oldtype __attribute__ ((deprecated("unsafe", "newtype")));
+@end smallexample
+
The @code{deprecated} attribute can also be used for functions and
variables (@pxref{Function Attributes}, @pxref{Variable Attributes}.)
diff --git a/gcc/testsuite/c-c++-common/Wdeprecated-arg-1.c b/gcc/testsuite/c-c++-common/Wdeprecated-arg-1.c
new file mode 100644
index 00000000000..b03e0e42ad6
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wdeprecated-arg-1.c
@@ -0,0 +1,21 @@
+/* PR c++/102049 */
+/* { dg-do compile } */
+/* { dg-additional-options "-std=c++11" { target c++ } } */
+
+typedef int __attribute__((deprecated("obsolete", "newtype"))) oldtype;
+__attribute__((deprecated("unsafe", "get_it_s"))) void get_it (int); /* { dg-message "declared here" } */
+[[gnu::deprecated("g", "")]] void g(); /* { dg-error "only allowed in GNU attributes" } */
+[[deprecated("h", "")]] void h(); /* { dg-error "only allowed in GNU attributes" } */
+__attribute__((deprecated("unsafe", 1))) void f2(); /* { dg-error "argument is not a string" } */
+__attribute__((deprecated("unsafe", "newvar"))) int oldvar; /* { dg-message "declared here" } */
+
+int
+main ()
+{
+ get_it (42); /* { dg-warning "is deprecated: unsafe" } */
+ /* { dg-message "use .get_it_s. instead" "" { target *-*-* } .-1 } */
+ oldtype t; /* { dg-warning "is deprecated: obsolete" } */
+ /* { dg-message "use .newtype. instead" "" { target *-*-* } .-1 } */
+ int i = oldvar; /* { dg-warning "is deprecated: unsafe" } */
+ /* { dg-message "use .newvar. instead" "" { target *-*-* } .-1 } */
+}
diff --git a/gcc/testsuite/gcc.dg/c2x-attr-deprecated-3.c b/gcc/testsuite/gcc.dg/c2x-attr-deprecated-3.c
index 044725e5123..6cb70705bbf 100644
--- a/gcc/testsuite/gcc.dg/c2x-attr-deprecated-3.c
+++ b/gcc/testsuite/gcc.dg/c2x-attr-deprecated-3.c
@@ -6,6 +6,6 @@
[[deprecated(0)]] int b; /* { dg-error "expected" } */
-[[deprecated("", 123)]] int c; /* { dg-error "expected" } */
+[[deprecated("", 123)]] int c; /* { dg-error "only allowed in GNU attributes" } */
[[deprecated((""))]] int d; /* { dg-error "expected" } */
diff --git a/gcc/tree.c b/gcc/tree.c
index 561b9cd56bd..5cb2d57a960 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -12002,8 +12002,9 @@ bool
warn_deprecated_use (tree node, tree attr)
{
escaped_string msg;
+ escaped_string rep;
- if (node == 0 || !warn_deprecated_decl)
+ if (node == NULL_TREE || !warn_deprecated_decl)
return false;
if (!attr)
@@ -12023,7 +12024,12 @@ warn_deprecated_use (tree node, tree attr)
attr = lookup_attribute ("deprecated", attr);
if (attr)
- msg.escape (TREE_STRING_POINTER (TREE_VALUE (TREE_VALUE (attr))));
+ {
+ tree val = TREE_VALUE (attr);
+ msg.escape (TREE_STRING_POINTER (TREE_VALUE (val)));
+ if (TREE_CHAIN (val))
+ rep.escape (TREE_STRING_POINTER (TREE_VALUE (TREE_CHAIN (val))));
+ }
bool w = false;
if (DECL_P (node))
@@ -12036,7 +12042,11 @@ warn_deprecated_use (tree node, tree attr)
w = warning (OPT_Wdeprecated_declarations,
"%qD is deprecated", node);
if (w)
- inform (DECL_SOURCE_LOCATION (node), "declared here");
+ {
+ if (rep)
+ inform (input_location, "use %qs instead", (const char *) rep);
+ inform (DECL_SOURCE_LOCATION (node), "declared here");
+ }
}
else if (TYPE_P (node))
{
@@ -12072,8 +12082,13 @@ warn_deprecated_use (tree node, tree attr)
"type is deprecated");
}
- if (w && decl)
- inform (DECL_SOURCE_LOCATION (decl), "declared here");
+ if (w)
+ {
+ if (rep)
+ inform (input_location, "use %qs instead", (const char *) rep);
+ if (decl)
+ inform (DECL_SOURCE_LOCATION (decl), "declared here");
+ }
}
return w;
base-commit: f6a05b23cc2e53c38e8321ddb5d2cbe40737e506
--
2.31.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] attribs: Allow optional second arg for attr deprecated [PR102049]
2021-09-23 16:16 [PATCH] attribs: Allow optional second arg for attr deprecated [PR102049] Marek Polacek
@ 2021-10-11 15:17 ` Marek Polacek
2021-10-29 21:51 ` Eric Gallager
2021-11-01 23:15 ` Martin Sebor
0 siblings, 2 replies; 5+ messages in thread
From: Marek Polacek @ 2021-10-11 15:17 UTC (permalink / raw)
To: GCC Patches
Any thoughts?
On Thu, Sep 23, 2021 at 12:16:36PM -0400, Marek Polacek via Gcc-patches wrote:
> Clang implements something we don't have:
>
> __attribute__((deprecated("message", "replacement")));
>
> which seems pretty neat so I wrote this patch to add it to gcc.
>
> It doesn't allow the optional second argument in the standard [[]]
> form so as not to clash with possible future standard additions.
>
> I had hoped we could print a nice fix-it replacement hint, but that
> won't be possible until warn_deprecated_use gets something better than
> input_location.
>
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
>
> PR c++/102049
>
> gcc/c-family/ChangeLog:
>
> * c-attribs.c (c_common_attribute_table): Increase max_len for
> deprecated.
> (handle_deprecated_attribute): Allow an optional second argument
> in the GNU form of attribute deprecated.
>
> gcc/c/ChangeLog:
>
> * c-parser.c (c_parser_std_attribute): Give a diagnostic when
> the standard form of an attribute deprecated has a second argument.
>
> gcc/ChangeLog:
>
> * doc/extend.texi: Document attribute deprecated with an
> optional second argument.
> * tree.c (warn_deprecated_use): Print the replacement argument,
> if any.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.dg/c2x-attr-deprecated-3.c: Adjust dg-error.
> * c-c++-common/Wdeprecated-arg-1.c: New test.
> ---
> gcc/c-family/c-attribs.c | 17 ++++++++++++-
> gcc/c/c-parser.c | 8 ++++++
> gcc/doc/extend.texi | 24 ++++++++++++++++++
> .../c-c++-common/Wdeprecated-arg-1.c | 21 ++++++++++++++++
> gcc/testsuite/gcc.dg/c2x-attr-deprecated-3.c | 2 +-
> gcc/tree.c | 25 +++++++++++++++----
> 6 files changed, 90 insertions(+), 7 deletions(-)
> create mode 100644 gcc/testsuite/c-c++-common/Wdeprecated-arg-1.c
>
> diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
> index 007b928c54b..ef857a9ae2c 100644
> --- a/gcc/c-family/c-attribs.c
> +++ b/gcc/c-family/c-attribs.c
> @@ -409,7 +409,7 @@ const struct attribute_spec c_common_attribute_table[] =
> to prevent its usage in source code. */
> { "no vops", 0, 0, true, false, false, false,
> handle_novops_attribute, NULL },
> - { "deprecated", 0, 1, false, false, false, false,
> + { "deprecated", 0, 2, false, false, false, false,
> handle_deprecated_attribute, NULL },
> { "unavailable", 0, 1, false, false, false, false,
> handle_unavailable_attribute, NULL },
> @@ -4107,6 +4107,21 @@ handle_deprecated_attribute (tree *node, tree name,
> error ("deprecated message is not a string");
> *no_add_attrs = true;
> }
> + else if (TREE_CHAIN (args) != NULL_TREE)
> + {
> + /* We allow an optional second argument in the GNU form of
> + attribute deprecated, which specifies the replacement. */
> + if (flags & ATTR_FLAG_CXX11)
> + {
> + error ("replacement argument only allowed in GNU attributes");
> + *no_add_attrs = true;
> + }
> + else if (TREE_CODE (TREE_VALUE (TREE_CHAIN (args))) != STRING_CST)
> + {
> + error ("replacement argument is not a string");
> + *no_add_attrs = true;
> + }
> + }
>
> if (DECL_P (*node))
> {
> diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
> index fa29d2c15fc..2b47f01d166 100644
> --- a/gcc/c/c-parser.c
> +++ b/gcc/c/c-parser.c
> @@ -4952,6 +4952,14 @@ c_parser_std_attribute (c_parser *parser, bool for_tm)
> TREE_VALUE (attribute)
> = c_parser_attribute_arguments (parser, takes_identifier,
> require_string, false);
> + if (c_parser_next_token_is (parser, CPP_COMMA)
> + && strcmp (IDENTIFIER_POINTER (name), "deprecated") == 0)
> + {
> + error_at (open_loc, "replacement argument only allowed in "
> + "GNU attributes");
> + c_parser_skip_until_found (parser, CPP_CLOSE_PAREN, NULL);
> + return error_mark_node;
> + }
> }
> else
> c_parser_balanced_token_sequence (parser);
> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> index 9501a60f20e..7d399f4b2bc 100644
> --- a/gcc/doc/extend.texi
> +++ b/gcc/doc/extend.texi
> @@ -2860,6 +2860,7 @@ StrongAlias (allocate, alloc);
>
> @item deprecated
> @itemx deprecated (@var{msg})
> +@itemx deprecated (@var{msg}, @var{replacement})
> @cindex @code{deprecated} function attribute
> The @code{deprecated} attribute results in a warning if the function
> is used anywhere in the source file. This is useful when identifying
> @@ -2880,6 +2881,13 @@ results in a warning on line 3 but not line 2. The optional @var{msg}
> argument, which must be a string, is printed in the warning if
> present.
>
> +This attribute accepts an optional second argument which specifies
> +the replacement that ought to be used instead:
> +
> +@smallexample
> +int old_fn () __attribute__ ((deprecated("unsafe", "new_fn")));
> +@end smallexample
> +
> The @code{deprecated} attribute can also be used for variables and
> types (@pxref{Variable Attributes}, @pxref{Type Attributes}.)
>
> @@ -7409,6 +7417,7 @@ attribute is also not copied. @xref{Common Function Attributes}.
>
> @item deprecated
> @itemx deprecated (@var{msg})
> +@itemx deprecated (@var{msg}, @var{replacement})
> @cindex @code{deprecated} variable attribute
> The @code{deprecated} attribute results in a warning if the variable
> is used anywhere in the source file. This is useful when identifying
> @@ -7429,6 +7438,13 @@ results in a warning on line 3 but not line 2. The optional @var{msg}
> argument, which must be a string, is printed in the warning if
> present.
>
> +This attribute accepts an optional second argument which specifies
> +the replacement that ought to be used instead:
> +
> +@smallexample
> +extern int old_var __attribute__ ((deprecated("unsafe", "new_var")));
> +@end smallexample
> +
> The @code{deprecated} attribute can also be used for functions and
> types (@pxref{Common Function Attributes},
> @pxref{Common Type Attributes}).
> @@ -8492,6 +8508,7 @@ struct __attribute__ ((copy ( (struct A *)0)) B @{ /* @r{@dots{}} */ @};
>
> @item deprecated
> @itemx deprecated (@var{msg})
> +@itemx deprecated (@var{msg}, @var{replacement})
> @cindex @code{deprecated} type attribute
> The @code{deprecated} attribute results in a warning if the type
> is used anywhere in the source file. This is useful when identifying
> @@ -8522,6 +8539,13 @@ present. Control characters in the string will be replaced with
> escape sequences, and if the @option{-fmessage-length} option is set
> to 0 (its default value) then any newline characters will be ignored.
>
> +This attribute accepts an optional second argument which specifies
> +the replacement that ought to be used instead:
> +
> +@smallexample
> +typedef int oldtype __attribute__ ((deprecated("unsafe", "newtype")));
> +@end smallexample
> +
> The @code{deprecated} attribute can also be used for functions and
> variables (@pxref{Function Attributes}, @pxref{Variable Attributes}.)
>
> diff --git a/gcc/testsuite/c-c++-common/Wdeprecated-arg-1.c b/gcc/testsuite/c-c++-common/Wdeprecated-arg-1.c
> new file mode 100644
> index 00000000000..b03e0e42ad6
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/Wdeprecated-arg-1.c
> @@ -0,0 +1,21 @@
> +/* PR c++/102049 */
> +/* { dg-do compile } */
> +/* { dg-additional-options "-std=c++11" { target c++ } } */
> +
> +typedef int __attribute__((deprecated("obsolete", "newtype"))) oldtype;
> +__attribute__((deprecated("unsafe", "get_it_s"))) void get_it (int); /* { dg-message "declared here" } */
> +[[gnu::deprecated("g", "")]] void g(); /* { dg-error "only allowed in GNU attributes" } */
> +[[deprecated("h", "")]] void h(); /* { dg-error "only allowed in GNU attributes" } */
> +__attribute__((deprecated("unsafe", 1))) void f2(); /* { dg-error "argument is not a string" } */
> +__attribute__((deprecated("unsafe", "newvar"))) int oldvar; /* { dg-message "declared here" } */
> +
> +int
> +main ()
> +{
> + get_it (42); /* { dg-warning "is deprecated: unsafe" } */
> + /* { dg-message "use .get_it_s. instead" "" { target *-*-* } .-1 } */
> + oldtype t; /* { dg-warning "is deprecated: obsolete" } */
> + /* { dg-message "use .newtype. instead" "" { target *-*-* } .-1 } */
> + int i = oldvar; /* { dg-warning "is deprecated: unsafe" } */
> + /* { dg-message "use .newvar. instead" "" { target *-*-* } .-1 } */
> +}
> diff --git a/gcc/testsuite/gcc.dg/c2x-attr-deprecated-3.c b/gcc/testsuite/gcc.dg/c2x-attr-deprecated-3.c
> index 044725e5123..6cb70705bbf 100644
> --- a/gcc/testsuite/gcc.dg/c2x-attr-deprecated-3.c
> +++ b/gcc/testsuite/gcc.dg/c2x-attr-deprecated-3.c
> @@ -6,6 +6,6 @@
>
> [[deprecated(0)]] int b; /* { dg-error "expected" } */
>
> -[[deprecated("", 123)]] int c; /* { dg-error "expected" } */
> +[[deprecated("", 123)]] int c; /* { dg-error "only allowed in GNU attributes" } */
>
> [[deprecated((""))]] int d; /* { dg-error "expected" } */
> diff --git a/gcc/tree.c b/gcc/tree.c
> index 561b9cd56bd..5cb2d57a960 100644
> --- a/gcc/tree.c
> +++ b/gcc/tree.c
> @@ -12002,8 +12002,9 @@ bool
> warn_deprecated_use (tree node, tree attr)
> {
> escaped_string msg;
> + escaped_string rep;
>
> - if (node == 0 || !warn_deprecated_decl)
> + if (node == NULL_TREE || !warn_deprecated_decl)
> return false;
>
> if (!attr)
> @@ -12023,7 +12024,12 @@ warn_deprecated_use (tree node, tree attr)
> attr = lookup_attribute ("deprecated", attr);
>
> if (attr)
> - msg.escape (TREE_STRING_POINTER (TREE_VALUE (TREE_VALUE (attr))));
> + {
> + tree val = TREE_VALUE (attr);
> + msg.escape (TREE_STRING_POINTER (TREE_VALUE (val)));
> + if (TREE_CHAIN (val))
> + rep.escape (TREE_STRING_POINTER (TREE_VALUE (TREE_CHAIN (val))));
> + }
>
> bool w = false;
> if (DECL_P (node))
> @@ -12036,7 +12042,11 @@ warn_deprecated_use (tree node, tree attr)
> w = warning (OPT_Wdeprecated_declarations,
> "%qD is deprecated", node);
> if (w)
> - inform (DECL_SOURCE_LOCATION (node), "declared here");
> + {
> + if (rep)
> + inform (input_location, "use %qs instead", (const char *) rep);
> + inform (DECL_SOURCE_LOCATION (node), "declared here");
> + }
> }
> else if (TYPE_P (node))
> {
> @@ -12072,8 +12082,13 @@ warn_deprecated_use (tree node, tree attr)
> "type is deprecated");
> }
>
> - if (w && decl)
> - inform (DECL_SOURCE_LOCATION (decl), "declared here");
> + if (w)
> + {
> + if (rep)
> + inform (input_location, "use %qs instead", (const char *) rep);
> + if (decl)
> + inform (DECL_SOURCE_LOCATION (decl), "declared here");
> + }
> }
>
> return w;
>
> base-commit: f6a05b23cc2e53c38e8321ddb5d2cbe40737e506
> --
> 2.31.1
>
Marek
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] attribs: Allow optional second arg for attr deprecated [PR102049]
2021-10-11 15:17 ` Marek Polacek
@ 2021-10-29 21:51 ` Eric Gallager
2021-11-01 23:15 ` Martin Sebor
1 sibling, 0 replies; 5+ messages in thread
From: Eric Gallager @ 2021-10-29 21:51 UTC (permalink / raw)
To: Marek Polacek; +Cc: GCC Patches
On Mon, Oct 11, 2021 at 11:19 AM Marek Polacek via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Any thoughts?
I think it's a good idea, but then again I can't approve it, so...
well, who can, then?
>
> On Thu, Sep 23, 2021 at 12:16:36PM -0400, Marek Polacek via Gcc-patches wrote:
> > Clang implements something we don't have:
> >
> > __attribute__((deprecated("message", "replacement")));
> >
> > which seems pretty neat so I wrote this patch to add it to gcc.
> >
> > It doesn't allow the optional second argument in the standard [[]]
> > form so as not to clash with possible future standard additions.
> >
> > I had hoped we could print a nice fix-it replacement hint, but that
> > won't be possible until warn_deprecated_use gets something better than
> > input_location.
Looking forward to the fix-it hint support being added!
> >
> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> >
> > PR c++/102049
> >
> > gcc/c-family/ChangeLog:
> >
> > * c-attribs.c (c_common_attribute_table): Increase max_len for
> > deprecated.
> > (handle_deprecated_attribute): Allow an optional second argument
> > in the GNU form of attribute deprecated.
> >
> > gcc/c/ChangeLog:
> >
> > * c-parser.c (c_parser_std_attribute): Give a diagnostic when
> > the standard form of an attribute deprecated has a second argument.
> >
> > gcc/ChangeLog:
> >
> > * doc/extend.texi: Document attribute deprecated with an
> > optional second argument.
> > * tree.c (warn_deprecated_use): Print the replacement argument,
> > if any.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * gcc.dg/c2x-attr-deprecated-3.c: Adjust dg-error.
> > * c-c++-common/Wdeprecated-arg-1.c: New test.
> > ---
> > gcc/c-family/c-attribs.c | 17 ++++++++++++-
> > gcc/c/c-parser.c | 8 ++++++
> > gcc/doc/extend.texi | 24 ++++++++++++++++++
> > .../c-c++-common/Wdeprecated-arg-1.c | 21 ++++++++++++++++
> > gcc/testsuite/gcc.dg/c2x-attr-deprecated-3.c | 2 +-
> > gcc/tree.c | 25 +++++++++++++++----
> > 6 files changed, 90 insertions(+), 7 deletions(-)
> > create mode 100644 gcc/testsuite/c-c++-common/Wdeprecated-arg-1.c
> >
> > diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
> > index 007b928c54b..ef857a9ae2c 100644
> > --- a/gcc/c-family/c-attribs.c
> > +++ b/gcc/c-family/c-attribs.c
> > @@ -409,7 +409,7 @@ const struct attribute_spec c_common_attribute_table[] =
> > to prevent its usage in source code. */
> > { "no vops", 0, 0, true, false, false, false,
> > handle_novops_attribute, NULL },
> > - { "deprecated", 0, 1, false, false, false, false,
> > + { "deprecated", 0, 2, false, false, false, false,
> > handle_deprecated_attribute, NULL },
> > { "unavailable", 0, 1, false, false, false, false,
> > handle_unavailable_attribute, NULL },
> > @@ -4107,6 +4107,21 @@ handle_deprecated_attribute (tree *node, tree name,
> > error ("deprecated message is not a string");
> > *no_add_attrs = true;
> > }
> > + else if (TREE_CHAIN (args) != NULL_TREE)
> > + {
> > + /* We allow an optional second argument in the GNU form of
> > + attribute deprecated, which specifies the replacement. */
> > + if (flags & ATTR_FLAG_CXX11)
> > + {
> > + error ("replacement argument only allowed in GNU attributes");
> > + *no_add_attrs = true;
> > + }
> > + else if (TREE_CODE (TREE_VALUE (TREE_CHAIN (args))) != STRING_CST)
> > + {
> > + error ("replacement argument is not a string");
> > + *no_add_attrs = true;
> > + }
> > + }
> >
> > if (DECL_P (*node))
> > {
> > diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
> > index fa29d2c15fc..2b47f01d166 100644
> > --- a/gcc/c/c-parser.c
> > +++ b/gcc/c/c-parser.c
> > @@ -4952,6 +4952,14 @@ c_parser_std_attribute (c_parser *parser, bool for_tm)
> > TREE_VALUE (attribute)
> > = c_parser_attribute_arguments (parser, takes_identifier,
> > require_string, false);
> > + if (c_parser_next_token_is (parser, CPP_COMMA)
> > + && strcmp (IDENTIFIER_POINTER (name), "deprecated") == 0)
> > + {
> > + error_at (open_loc, "replacement argument only allowed in "
> > + "GNU attributes");
> > + c_parser_skip_until_found (parser, CPP_CLOSE_PAREN, NULL);
> > + return error_mark_node;
> > + }
> > }
> > else
> > c_parser_balanced_token_sequence (parser);
> > diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> > index 9501a60f20e..7d399f4b2bc 100644
> > --- a/gcc/doc/extend.texi
> > +++ b/gcc/doc/extend.texi
> > @@ -2860,6 +2860,7 @@ StrongAlias (allocate, alloc);
> >
> > @item deprecated
> > @itemx deprecated (@var{msg})
> > +@itemx deprecated (@var{msg}, @var{replacement})
> > @cindex @code{deprecated} function attribute
> > The @code{deprecated} attribute results in a warning if the function
> > is used anywhere in the source file. This is useful when identifying
> > @@ -2880,6 +2881,13 @@ results in a warning on line 3 but not line 2. The optional @var{msg}
> > argument, which must be a string, is printed in the warning if
> > present.
> >
> > +This attribute accepts an optional second argument which specifies
> > +the replacement that ought to be used instead:
> > +
> > +@smallexample
> > +int old_fn () __attribute__ ((deprecated("unsafe", "new_fn")));
> > +@end smallexample
> > +
> > The @code{deprecated} attribute can also be used for variables and
> > types (@pxref{Variable Attributes}, @pxref{Type Attributes}.)
> >
> > @@ -7409,6 +7417,7 @@ attribute is also not copied. @xref{Common Function Attributes}.
> >
> > @item deprecated
> > @itemx deprecated (@var{msg})
> > +@itemx deprecated (@var{msg}, @var{replacement})
> > @cindex @code{deprecated} variable attribute
> > The @code{deprecated} attribute results in a warning if the variable
> > is used anywhere in the source file. This is useful when identifying
> > @@ -7429,6 +7438,13 @@ results in a warning on line 3 but not line 2. The optional @var{msg}
> > argument, which must be a string, is printed in the warning if
> > present.
> >
> > +This attribute accepts an optional second argument which specifies
> > +the replacement that ought to be used instead:
> > +
> > +@smallexample
> > +extern int old_var __attribute__ ((deprecated("unsafe", "new_var")));
> > +@end smallexample
> > +
> > The @code{deprecated} attribute can also be used for functions and
> > types (@pxref{Common Function Attributes},
> > @pxref{Common Type Attributes}).
> > @@ -8492,6 +8508,7 @@ struct __attribute__ ((copy ( (struct A *)0)) B @{ /* @r{@dots{}} */ @};
> >
> > @item deprecated
> > @itemx deprecated (@var{msg})
> > +@itemx deprecated (@var{msg}, @var{replacement})
> > @cindex @code{deprecated} type attribute
> > The @code{deprecated} attribute results in a warning if the type
> > is used anywhere in the source file. This is useful when identifying
> > @@ -8522,6 +8539,13 @@ present. Control characters in the string will be replaced with
> > escape sequences, and if the @option{-fmessage-length} option is set
> > to 0 (its default value) then any newline characters will be ignored.
> >
> > +This attribute accepts an optional second argument which specifies
> > +the replacement that ought to be used instead:
> > +
> > +@smallexample
> > +typedef int oldtype __attribute__ ((deprecated("unsafe", "newtype")));
> > +@end smallexample
> > +
> > The @code{deprecated} attribute can also be used for functions and
> > variables (@pxref{Function Attributes}, @pxref{Variable Attributes}.)
> >
> > diff --git a/gcc/testsuite/c-c++-common/Wdeprecated-arg-1.c b/gcc/testsuite/c-c++-common/Wdeprecated-arg-1.c
> > new file mode 100644
> > index 00000000000..b03e0e42ad6
> > --- /dev/null
> > +++ b/gcc/testsuite/c-c++-common/Wdeprecated-arg-1.c
> > @@ -0,0 +1,21 @@
> > +/* PR c++/102049 */
> > +/* { dg-do compile } */
> > +/* { dg-additional-options "-std=c++11" { target c++ } } */
> > +
> > +typedef int __attribute__((deprecated("obsolete", "newtype"))) oldtype;
> > +__attribute__((deprecated("unsafe", "get_it_s"))) void get_it (int); /* { dg-message "declared here" } */
> > +[[gnu::deprecated("g", "")]] void g(); /* { dg-error "only allowed in GNU attributes" } */
> > +[[deprecated("h", "")]] void h(); /* { dg-error "only allowed in GNU attributes" } */
> > +__attribute__((deprecated("unsafe", 1))) void f2(); /* { dg-error "argument is not a string" } */
> > +__attribute__((deprecated("unsafe", "newvar"))) int oldvar; /* { dg-message "declared here" } */
> > +
> > +int
> > +main ()
> > +{
> > + get_it (42); /* { dg-warning "is deprecated: unsafe" } */
> > + /* { dg-message "use .get_it_s. instead" "" { target *-*-* } .-1 } */
> > + oldtype t; /* { dg-warning "is deprecated: obsolete" } */
> > + /* { dg-message "use .newtype. instead" "" { target *-*-* } .-1 } */
> > + int i = oldvar; /* { dg-warning "is deprecated: unsafe" } */
> > + /* { dg-message "use .newvar. instead" "" { target *-*-* } .-1 } */
> > +}
> > diff --git a/gcc/testsuite/gcc.dg/c2x-attr-deprecated-3.c b/gcc/testsuite/gcc.dg/c2x-attr-deprecated-3.c
> > index 044725e5123..6cb70705bbf 100644
> > --- a/gcc/testsuite/gcc.dg/c2x-attr-deprecated-3.c
> > +++ b/gcc/testsuite/gcc.dg/c2x-attr-deprecated-3.c
> > @@ -6,6 +6,6 @@
> >
> > [[deprecated(0)]] int b; /* { dg-error "expected" } */
> >
> > -[[deprecated("", 123)]] int c; /* { dg-error "expected" } */
> > +[[deprecated("", 123)]] int c; /* { dg-error "only allowed in GNU attributes" } */
> >
> > [[deprecated((""))]] int d; /* { dg-error "expected" } */
> > diff --git a/gcc/tree.c b/gcc/tree.c
> > index 561b9cd56bd..5cb2d57a960 100644
> > --- a/gcc/tree.c
> > +++ b/gcc/tree.c
> > @@ -12002,8 +12002,9 @@ bool
> > warn_deprecated_use (tree node, tree attr)
> > {
> > escaped_string msg;
> > + escaped_string rep;
> >
> > - if (node == 0 || !warn_deprecated_decl)
> > + if (node == NULL_TREE || !warn_deprecated_decl)
> > return false;
> >
> > if (!attr)
> > @@ -12023,7 +12024,12 @@ warn_deprecated_use (tree node, tree attr)
> > attr = lookup_attribute ("deprecated", attr);
> >
> > if (attr)
> > - msg.escape (TREE_STRING_POINTER (TREE_VALUE (TREE_VALUE (attr))));
> > + {
> > + tree val = TREE_VALUE (attr);
> > + msg.escape (TREE_STRING_POINTER (TREE_VALUE (val)));
> > + if (TREE_CHAIN (val))
> > + rep.escape (TREE_STRING_POINTER (TREE_VALUE (TREE_CHAIN (val))));
> > + }
> >
> > bool w = false;
> > if (DECL_P (node))
> > @@ -12036,7 +12042,11 @@ warn_deprecated_use (tree node, tree attr)
> > w = warning (OPT_Wdeprecated_declarations,
> > "%qD is deprecated", node);
> > if (w)
> > - inform (DECL_SOURCE_LOCATION (node), "declared here");
> > + {
> > + if (rep)
> > + inform (input_location, "use %qs instead", (const char *) rep);
> > + inform (DECL_SOURCE_LOCATION (node), "declared here");
> > + }
> > }
> > else if (TYPE_P (node))
> > {
> > @@ -12072,8 +12082,13 @@ warn_deprecated_use (tree node, tree attr)
> > "type is deprecated");
> > }
> >
> > - if (w && decl)
> > - inform (DECL_SOURCE_LOCATION (decl), "declared here");
> > + if (w)
> > + {
> > + if (rep)
> > + inform (input_location, "use %qs instead", (const char *) rep);
> > + if (decl)
> > + inform (DECL_SOURCE_LOCATION (decl), "declared here");
> > + }
> > }
> >
> > return w;
> >
> > base-commit: f6a05b23cc2e53c38e8321ddb5d2cbe40737e506
> > --
> > 2.31.1
> >
>
> Marek
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] attribs: Allow optional second arg for attr deprecated [PR102049]
2021-10-11 15:17 ` Marek Polacek
2021-10-29 21:51 ` Eric Gallager
@ 2021-11-01 23:15 ` Martin Sebor
2021-11-02 20:35 ` Marek Polacek
1 sibling, 1 reply; 5+ messages in thread
From: Martin Sebor @ 2021-11-01 23:15 UTC (permalink / raw)
To: Marek Polacek, GCC Patches
On 10/11/21 9:17 AM, Marek Polacek via Gcc-patches wrote:
> Any thoughts?
I'm a little unsure. Clang just uses the replacement string
as the text of the fix-it note as is, so it does nothing to
help programmers make sure the replacement is in sync with
what it's supposed to replace. E.g., for this Clang output
is below:
__attribute__ ((deprecated ("foo is bad", "use bar instead")))
void foo (void);
void baz (void) { foo (); }
int bar;
<source>:2:19: warning: 'foo' is deprecated: foo is bad
[-Wdeprecated-declarations]
void baz (void) { foo (); }
^~~
use bar instead
Since bar is a variable it's hard to see how it might be used
instead of the function foo(). Fix-its, as I understand them,
are meant not just as a visual clue but also to let IDEs and
other tools automatically apply the fixes. With buggy fix-its
this obviously wouldn't work.
I think the replacement would be useful if it had to reference
an existing symbol of the same kind, and if the compiler helped
enforce it. Otherwise it seems like a recipe for bit rot and
for tings/tools not working well together.
Martin
>
> On Thu, Sep 23, 2021 at 12:16:36PM -0400, Marek Polacek via Gcc-patches wrote:
>> Clang implements something we don't have:
>>
>> __attribute__((deprecated("message", "replacement")));
>>
>> which seems pretty neat so I wrote this patch to add it to gcc.
>>
>> It doesn't allow the optional second argument in the standard [[]]
>> form so as not to clash with possible future standard additions.
>>
>> I had hoped we could print a nice fix-it replacement hint, but that
>> won't be possible until warn_deprecated_use gets something better than
>> input_location.
>>
>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
>>
>> PR c++/102049
>>
>> gcc/c-family/ChangeLog:
>>
>> * c-attribs.c (c_common_attribute_table): Increase max_len for
>> deprecated.
>> (handle_deprecated_attribute): Allow an optional second argument
>> in the GNU form of attribute deprecated.
>>
>> gcc/c/ChangeLog:
>>
>> * c-parser.c (c_parser_std_attribute): Give a diagnostic when
>> the standard form of an attribute deprecated has a second argument.
>>
>> gcc/ChangeLog:
>>
>> * doc/extend.texi: Document attribute deprecated with an
>> optional second argument.
>> * tree.c (warn_deprecated_use): Print the replacement argument,
>> if any.
>>
>> gcc/testsuite/ChangeLog:
>>
>> * gcc.dg/c2x-attr-deprecated-3.c: Adjust dg-error.
>> * c-c++-common/Wdeprecated-arg-1.c: New test.
>> ---
>> gcc/c-family/c-attribs.c | 17 ++++++++++++-
>> gcc/c/c-parser.c | 8 ++++++
>> gcc/doc/extend.texi | 24 ++++++++++++++++++
>> .../c-c++-common/Wdeprecated-arg-1.c | 21 ++++++++++++++++
>> gcc/testsuite/gcc.dg/c2x-attr-deprecated-3.c | 2 +-
>> gcc/tree.c | 25 +++++++++++++++----
>> 6 files changed, 90 insertions(+), 7 deletions(-)
>> create mode 100644 gcc/testsuite/c-c++-common/Wdeprecated-arg-1.c
>>
>> diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
>> index 007b928c54b..ef857a9ae2c 100644
>> --- a/gcc/c-family/c-attribs.c
>> +++ b/gcc/c-family/c-attribs.c
>> @@ -409,7 +409,7 @@ const struct attribute_spec c_common_attribute_table[] =
>> to prevent its usage in source code. */
>> { "no vops", 0, 0, true, false, false, false,
>> handle_novops_attribute, NULL },
>> - { "deprecated", 0, 1, false, false, false, false,
>> + { "deprecated", 0, 2, false, false, false, false,
>> handle_deprecated_attribute, NULL },
>> { "unavailable", 0, 1, false, false, false, false,
>> handle_unavailable_attribute, NULL },
>> @@ -4107,6 +4107,21 @@ handle_deprecated_attribute (tree *node, tree name,
>> error ("deprecated message is not a string");
>> *no_add_attrs = true;
>> }
>> + else if (TREE_CHAIN (args) != NULL_TREE)
>> + {
>> + /* We allow an optional second argument in the GNU form of
>> + attribute deprecated, which specifies the replacement. */
>> + if (flags & ATTR_FLAG_CXX11)
>> + {
>> + error ("replacement argument only allowed in GNU attributes");
>> + *no_add_attrs = true;
>> + }
>> + else if (TREE_CODE (TREE_VALUE (TREE_CHAIN (args))) != STRING_CST)
>> + {
>> + error ("replacement argument is not a string");
>> + *no_add_attrs = true;
>> + }
>> + }
>>
>> if (DECL_P (*node))
>> {
>> diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
>> index fa29d2c15fc..2b47f01d166 100644
>> --- a/gcc/c/c-parser.c
>> +++ b/gcc/c/c-parser.c
>> @@ -4952,6 +4952,14 @@ c_parser_std_attribute (c_parser *parser, bool for_tm)
>> TREE_VALUE (attribute)
>> = c_parser_attribute_arguments (parser, takes_identifier,
>> require_string, false);
>> + if (c_parser_next_token_is (parser, CPP_COMMA)
>> + && strcmp (IDENTIFIER_POINTER (name), "deprecated") == 0)
>> + {
>> + error_at (open_loc, "replacement argument only allowed in "
>> + "GNU attributes");
>> + c_parser_skip_until_found (parser, CPP_CLOSE_PAREN, NULL);
>> + return error_mark_node;
>> + }
>> }
>> else
>> c_parser_balanced_token_sequence (parser);
>> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
>> index 9501a60f20e..7d399f4b2bc 100644
>> --- a/gcc/doc/extend.texi
>> +++ b/gcc/doc/extend.texi
>> @@ -2860,6 +2860,7 @@ StrongAlias (allocate, alloc);
>>
>> @item deprecated
>> @itemx deprecated (@var{msg})
>> +@itemx deprecated (@var{msg}, @var{replacement})
>> @cindex @code{deprecated} function attribute
>> The @code{deprecated} attribute results in a warning if the function
>> is used anywhere in the source file. This is useful when identifying
>> @@ -2880,6 +2881,13 @@ results in a warning on line 3 but not line 2. The optional @var{msg}
>> argument, which must be a string, is printed in the warning if
>> present.
>>
>> +This attribute accepts an optional second argument which specifies
>> +the replacement that ought to be used instead:
>> +
>> +@smallexample
>> +int old_fn () __attribute__ ((deprecated("unsafe", "new_fn")));
>> +@end smallexample
>> +
>> The @code{deprecated} attribute can also be used for variables and
>> types (@pxref{Variable Attributes}, @pxref{Type Attributes}.)
>>
>> @@ -7409,6 +7417,7 @@ attribute is also not copied. @xref{Common Function Attributes}.
>>
>> @item deprecated
>> @itemx deprecated (@var{msg})
>> +@itemx deprecated (@var{msg}, @var{replacement})
>> @cindex @code{deprecated} variable attribute
>> The @code{deprecated} attribute results in a warning if the variable
>> is used anywhere in the source file. This is useful when identifying
>> @@ -7429,6 +7438,13 @@ results in a warning on line 3 but not line 2. The optional @var{msg}
>> argument, which must be a string, is printed in the warning if
>> present.
>>
>> +This attribute accepts an optional second argument which specifies
>> +the replacement that ought to be used instead:
>> +
>> +@smallexample
>> +extern int old_var __attribute__ ((deprecated("unsafe", "new_var")));
>> +@end smallexample
>> +
>> The @code{deprecated} attribute can also be used for functions and
>> types (@pxref{Common Function Attributes},
>> @pxref{Common Type Attributes}).
>> @@ -8492,6 +8508,7 @@ struct __attribute__ ((copy ( (struct A *)0)) B @{ /* @r{@dots{}} */ @};
>>
>> @item deprecated
>> @itemx deprecated (@var{msg})
>> +@itemx deprecated (@var{msg}, @var{replacement})
>> @cindex @code{deprecated} type attribute
>> The @code{deprecated} attribute results in a warning if the type
>> is used anywhere in the source file. This is useful when identifying
>> @@ -8522,6 +8539,13 @@ present. Control characters in the string will be replaced with
>> escape sequences, and if the @option{-fmessage-length} option is set
>> to 0 (its default value) then any newline characters will be ignored.
>>
>> +This attribute accepts an optional second argument which specifies
>> +the replacement that ought to be used instead:
>> +
>> +@smallexample
>> +typedef int oldtype __attribute__ ((deprecated("unsafe", "newtype")));
>> +@end smallexample
>> +
>> The @code{deprecated} attribute can also be used for functions and
>> variables (@pxref{Function Attributes}, @pxref{Variable Attributes}.)
>>
>> diff --git a/gcc/testsuite/c-c++-common/Wdeprecated-arg-1.c b/gcc/testsuite/c-c++-common/Wdeprecated-arg-1.c
>> new file mode 100644
>> index 00000000000..b03e0e42ad6
>> --- /dev/null
>> +++ b/gcc/testsuite/c-c++-common/Wdeprecated-arg-1.c
>> @@ -0,0 +1,21 @@
>> +/* PR c++/102049 */
>> +/* { dg-do compile } */
>> +/* { dg-additional-options "-std=c++11" { target c++ } } */
>> +
>> +typedef int __attribute__((deprecated("obsolete", "newtype"))) oldtype;
>> +__attribute__((deprecated("unsafe", "get_it_s"))) void get_it (int); /* { dg-message "declared here" } */
>> +[[gnu::deprecated("g", "")]] void g(); /* { dg-error "only allowed in GNU attributes" } */
>> +[[deprecated("h", "")]] void h(); /* { dg-error "only allowed in GNU attributes" } */
>> +__attribute__((deprecated("unsafe", 1))) void f2(); /* { dg-error "argument is not a string" } */
>> +__attribute__((deprecated("unsafe", "newvar"))) int oldvar; /* { dg-message "declared here" } */
>> +
>> +int
>> +main ()
>> +{
>> + get_it (42); /* { dg-warning "is deprecated: unsafe" } */
>> + /* { dg-message "use .get_it_s. instead" "" { target *-*-* } .-1 } */
>> + oldtype t; /* { dg-warning "is deprecated: obsolete" } */
>> + /* { dg-message "use .newtype. instead" "" { target *-*-* } .-1 } */
>> + int i = oldvar; /* { dg-warning "is deprecated: unsafe" } */
>> + /* { dg-message "use .newvar. instead" "" { target *-*-* } .-1 } */
>> +}
>> diff --git a/gcc/testsuite/gcc.dg/c2x-attr-deprecated-3.c b/gcc/testsuite/gcc.dg/c2x-attr-deprecated-3.c
>> index 044725e5123..6cb70705bbf 100644
>> --- a/gcc/testsuite/gcc.dg/c2x-attr-deprecated-3.c
>> +++ b/gcc/testsuite/gcc.dg/c2x-attr-deprecated-3.c
>> @@ -6,6 +6,6 @@
>>
>> [[deprecated(0)]] int b; /* { dg-error "expected" } */
>>
>> -[[deprecated("", 123)]] int c; /* { dg-error "expected" } */
>> +[[deprecated("", 123)]] int c; /* { dg-error "only allowed in GNU attributes" } */
>>
>> [[deprecated((""))]] int d; /* { dg-error "expected" } */
>> diff --git a/gcc/tree.c b/gcc/tree.c
>> index 561b9cd56bd..5cb2d57a960 100644
>> --- a/gcc/tree.c
>> +++ b/gcc/tree.c
>> @@ -12002,8 +12002,9 @@ bool
>> warn_deprecated_use (tree node, tree attr)
>> {
>> escaped_string msg;
>> + escaped_string rep;
>>
>> - if (node == 0 || !warn_deprecated_decl)
>> + if (node == NULL_TREE || !warn_deprecated_decl)
>> return false;
>>
>> if (!attr)
>> @@ -12023,7 +12024,12 @@ warn_deprecated_use (tree node, tree attr)
>> attr = lookup_attribute ("deprecated", attr);
>>
>> if (attr)
>> - msg.escape (TREE_STRING_POINTER (TREE_VALUE (TREE_VALUE (attr))));
>> + {
>> + tree val = TREE_VALUE (attr);
>> + msg.escape (TREE_STRING_POINTER (TREE_VALUE (val)));
>> + if (TREE_CHAIN (val))
>> + rep.escape (TREE_STRING_POINTER (TREE_VALUE (TREE_CHAIN (val))));
>> + }
>>
>> bool w = false;
>> if (DECL_P (node))
>> @@ -12036,7 +12042,11 @@ warn_deprecated_use (tree node, tree attr)
>> w = warning (OPT_Wdeprecated_declarations,
>> "%qD is deprecated", node);
>> if (w)
>> - inform (DECL_SOURCE_LOCATION (node), "declared here");
>> + {
>> + if (rep)
>> + inform (input_location, "use %qs instead", (const char *) rep);
>> + inform (DECL_SOURCE_LOCATION (node), "declared here");
>> + }
>> }
>> else if (TYPE_P (node))
>> {
>> @@ -12072,8 +12082,13 @@ warn_deprecated_use (tree node, tree attr)
>> "type is deprecated");
>> }
>>
>> - if (w && decl)
>> - inform (DECL_SOURCE_LOCATION (decl), "declared here");
>> + if (w)
>> + {
>> + if (rep)
>> + inform (input_location, "use %qs instead", (const char *) rep);
>> + if (decl)
>> + inform (DECL_SOURCE_LOCATION (decl), "declared here");
>> + }
>> }
>>
>> return w;
>>
>> base-commit: f6a05b23cc2e53c38e8321ddb5d2cbe40737e506
>> --
>> 2.31.1
>>
>
> Marek
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] attribs: Allow optional second arg for attr deprecated [PR102049]
2021-11-01 23:15 ` Martin Sebor
@ 2021-11-02 20:35 ` Marek Polacek
0 siblings, 0 replies; 5+ messages in thread
From: Marek Polacek @ 2021-11-02 20:35 UTC (permalink / raw)
To: Martin Sebor; +Cc: GCC Patches
On Mon, Nov 01, 2021 at 05:15:03PM -0600, Martin Sebor wrote:
> On 10/11/21 9:17 AM, Marek Polacek via Gcc-patches wrote:
> > Any thoughts?
>
> I'm a little unsure. Clang just uses the replacement string
> as the text of the fix-it note as is, so it does nothing to
> help programmers make sure the replacement is in sync with
> what it's supposed to replace. E.g., for this Clang output
> is below:
>
> __attribute__ ((deprecated ("foo is bad", "use bar instead")))
> void foo (void);
> void baz (void) { foo (); }
>
> int bar;
>
> <source>:2:19: warning: 'foo' is deprecated: foo is bad
> [-Wdeprecated-declarations]
> void baz (void) { foo (); }
> ^~~
> use bar instead
>
> Since bar is a variable it's hard to see how it might be used
> instead of the function foo(). Fix-its, as I understand them,
> are meant not just as a visual clue but also to let IDEs and
> other tools automatically apply the fixes. With buggy fix-its
> this obviously wouldn't work.
>
> I think the replacement would be useful if it had to reference
> an existing symbol of the same kind, and if the compiler helped
> enforce it. Otherwise it seems like a recipe for bit rot and
> for tings/tools not working well together.
OK, I think I'm not going to pursue this further. I'll close the
PR and reference my patch in it, should this come up again in the
future.
Thanks,
> >
> > On Thu, Sep 23, 2021 at 12:16:36PM -0400, Marek Polacek via Gcc-patches wrote:
> > > Clang implements something we don't have:
> > >
> > > __attribute__((deprecated("message", "replacement")));
> > >
> > > which seems pretty neat so I wrote this patch to add it to gcc.
> > >
> > > It doesn't allow the optional second argument in the standard [[]]
> > > form so as not to clash with possible future standard additions.
> > >
> > > I had hoped we could print a nice fix-it replacement hint, but that
> > > won't be possible until warn_deprecated_use gets something better than
> > > input_location.
> > >
> > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> > >
> > > PR c++/102049
> > >
> > > gcc/c-family/ChangeLog:
> > >
> > > * c-attribs.c (c_common_attribute_table): Increase max_len for
> > > deprecated.
> > > (handle_deprecated_attribute): Allow an optional second argument
> > > in the GNU form of attribute deprecated.
> > >
> > > gcc/c/ChangeLog:
> > >
> > > * c-parser.c (c_parser_std_attribute): Give a diagnostic when
> > > the standard form of an attribute deprecated has a second argument.
> > >
> > > gcc/ChangeLog:
> > >
> > > * doc/extend.texi: Document attribute deprecated with an
> > > optional second argument.
> > > * tree.c (warn_deprecated_use): Print the replacement argument,
> > > if any.
> > >
> > > gcc/testsuite/ChangeLog:
> > >
> > > * gcc.dg/c2x-attr-deprecated-3.c: Adjust dg-error.
> > > * c-c++-common/Wdeprecated-arg-1.c: New test.
> > > ---
> > > gcc/c-family/c-attribs.c | 17 ++++++++++++-
> > > gcc/c/c-parser.c | 8 ++++++
> > > gcc/doc/extend.texi | 24 ++++++++++++++++++
> > > .../c-c++-common/Wdeprecated-arg-1.c | 21 ++++++++++++++++
> > > gcc/testsuite/gcc.dg/c2x-attr-deprecated-3.c | 2 +-
> > > gcc/tree.c | 25 +++++++++++++++----
> > > 6 files changed, 90 insertions(+), 7 deletions(-)
> > > create mode 100644 gcc/testsuite/c-c++-common/Wdeprecated-arg-1.c
> > >
> > > diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
> > > index 007b928c54b..ef857a9ae2c 100644
> > > --- a/gcc/c-family/c-attribs.c
> > > +++ b/gcc/c-family/c-attribs.c
> > > @@ -409,7 +409,7 @@ const struct attribute_spec c_common_attribute_table[] =
> > > to prevent its usage in source code. */
> > > { "no vops", 0, 0, true, false, false, false,
> > > handle_novops_attribute, NULL },
> > > - { "deprecated", 0, 1, false, false, false, false,
> > > + { "deprecated", 0, 2, false, false, false, false,
> > > handle_deprecated_attribute, NULL },
> > > { "unavailable", 0, 1, false, false, false, false,
> > > handle_unavailable_attribute, NULL },
> > > @@ -4107,6 +4107,21 @@ handle_deprecated_attribute (tree *node, tree name,
> > > error ("deprecated message is not a string");
> > > *no_add_attrs = true;
> > > }
> > > + else if (TREE_CHAIN (args) != NULL_TREE)
> > > + {
> > > + /* We allow an optional second argument in the GNU form of
> > > + attribute deprecated, which specifies the replacement. */
> > > + if (flags & ATTR_FLAG_CXX11)
> > > + {
> > > + error ("replacement argument only allowed in GNU attributes");
> > > + *no_add_attrs = true;
> > > + }
> > > + else if (TREE_CODE (TREE_VALUE (TREE_CHAIN (args))) != STRING_CST)
> > > + {
> > > + error ("replacement argument is not a string");
> > > + *no_add_attrs = true;
> > > + }
> > > + }
> > > if (DECL_P (*node))
> > > {
> > > diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
> > > index fa29d2c15fc..2b47f01d166 100644
> > > --- a/gcc/c/c-parser.c
> > > +++ b/gcc/c/c-parser.c
> > > @@ -4952,6 +4952,14 @@ c_parser_std_attribute (c_parser *parser, bool for_tm)
> > > TREE_VALUE (attribute)
> > > = c_parser_attribute_arguments (parser, takes_identifier,
> > > require_string, false);
> > > + if (c_parser_next_token_is (parser, CPP_COMMA)
> > > + && strcmp (IDENTIFIER_POINTER (name), "deprecated") == 0)
> > > + {
> > > + error_at (open_loc, "replacement argument only allowed in "
> > > + "GNU attributes");
> > > + c_parser_skip_until_found (parser, CPP_CLOSE_PAREN, NULL);
> > > + return error_mark_node;
> > > + }
> > > }
> > > else
> > > c_parser_balanced_token_sequence (parser);
> > > diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> > > index 9501a60f20e..7d399f4b2bc 100644
> > > --- a/gcc/doc/extend.texi
> > > +++ b/gcc/doc/extend.texi
> > > @@ -2860,6 +2860,7 @@ StrongAlias (allocate, alloc);
> > > @item deprecated
> > > @itemx deprecated (@var{msg})
> > > +@itemx deprecated (@var{msg}, @var{replacement})
> > > @cindex @code{deprecated} function attribute
> > > The @code{deprecated} attribute results in a warning if the function
> > > is used anywhere in the source file. This is useful when identifying
> > > @@ -2880,6 +2881,13 @@ results in a warning on line 3 but not line 2. The optional @var{msg}
> > > argument, which must be a string, is printed in the warning if
> > > present.
> > > +This attribute accepts an optional second argument which specifies
> > > +the replacement that ought to be used instead:
> > > +
> > > +@smallexample
> > > +int old_fn () __attribute__ ((deprecated("unsafe", "new_fn")));
> > > +@end smallexample
> > > +
> > > The @code{deprecated} attribute can also be used for variables and
> > > types (@pxref{Variable Attributes}, @pxref{Type Attributes}.)
> > > @@ -7409,6 +7417,7 @@ attribute is also not copied. @xref{Common Function Attributes}.
> > > @item deprecated
> > > @itemx deprecated (@var{msg})
> > > +@itemx deprecated (@var{msg}, @var{replacement})
> > > @cindex @code{deprecated} variable attribute
> > > The @code{deprecated} attribute results in a warning if the variable
> > > is used anywhere in the source file. This is useful when identifying
> > > @@ -7429,6 +7438,13 @@ results in a warning on line 3 but not line 2. The optional @var{msg}
> > > argument, which must be a string, is printed in the warning if
> > > present.
> > > +This attribute accepts an optional second argument which specifies
> > > +the replacement that ought to be used instead:
> > > +
> > > +@smallexample
> > > +extern int old_var __attribute__ ((deprecated("unsafe", "new_var")));
> > > +@end smallexample
> > > +
> > > The @code{deprecated} attribute can also be used for functions and
> > > types (@pxref{Common Function Attributes},
> > > @pxref{Common Type Attributes}).
> > > @@ -8492,6 +8508,7 @@ struct __attribute__ ((copy ( (struct A *)0)) B @{ /* @r{@dots{}} */ @};
> > > @item deprecated
> > > @itemx deprecated (@var{msg})
> > > +@itemx deprecated (@var{msg}, @var{replacement})
> > > @cindex @code{deprecated} type attribute
> > > The @code{deprecated} attribute results in a warning if the type
> > > is used anywhere in the source file. This is useful when identifying
> > > @@ -8522,6 +8539,13 @@ present. Control characters in the string will be replaced with
> > > escape sequences, and if the @option{-fmessage-length} option is set
> > > to 0 (its default value) then any newline characters will be ignored.
> > > +This attribute accepts an optional second argument which specifies
> > > +the replacement that ought to be used instead:
> > > +
> > > +@smallexample
> > > +typedef int oldtype __attribute__ ((deprecated("unsafe", "newtype")));
> > > +@end smallexample
> > > +
> > > The @code{deprecated} attribute can also be used for functions and
> > > variables (@pxref{Function Attributes}, @pxref{Variable Attributes}.)
> > > diff --git a/gcc/testsuite/c-c++-common/Wdeprecated-arg-1.c b/gcc/testsuite/c-c++-common/Wdeprecated-arg-1.c
> > > new file mode 100644
> > > index 00000000000..b03e0e42ad6
> > > --- /dev/null
> > > +++ b/gcc/testsuite/c-c++-common/Wdeprecated-arg-1.c
> > > @@ -0,0 +1,21 @@
> > > +/* PR c++/102049 */
> > > +/* { dg-do compile } */
> > > +/* { dg-additional-options "-std=c++11" { target c++ } } */
> > > +
> > > +typedef int __attribute__((deprecated("obsolete", "newtype"))) oldtype;
> > > +__attribute__((deprecated("unsafe", "get_it_s"))) void get_it (int); /* { dg-message "declared here" } */
> > > +[[gnu::deprecated("g", "")]] void g(); /* { dg-error "only allowed in GNU attributes" } */
> > > +[[deprecated("h", "")]] void h(); /* { dg-error "only allowed in GNU attributes" } */
> > > +__attribute__((deprecated("unsafe", 1))) void f2(); /* { dg-error "argument is not a string" } */
> > > +__attribute__((deprecated("unsafe", "newvar"))) int oldvar; /* { dg-message "declared here" } */
> > > +
> > > +int
> > > +main ()
> > > +{
> > > + get_it (42); /* { dg-warning "is deprecated: unsafe" } */
> > > + /* { dg-message "use .get_it_s. instead" "" { target *-*-* } .-1 } */
> > > + oldtype t; /* { dg-warning "is deprecated: obsolete" } */
> > > + /* { dg-message "use .newtype. instead" "" { target *-*-* } .-1 } */
> > > + int i = oldvar; /* { dg-warning "is deprecated: unsafe" } */
> > > + /* { dg-message "use .newvar. instead" "" { target *-*-* } .-1 } */
> > > +}
> > > diff --git a/gcc/testsuite/gcc.dg/c2x-attr-deprecated-3.c b/gcc/testsuite/gcc.dg/c2x-attr-deprecated-3.c
> > > index 044725e5123..6cb70705bbf 100644
> > > --- a/gcc/testsuite/gcc.dg/c2x-attr-deprecated-3.c
> > > +++ b/gcc/testsuite/gcc.dg/c2x-attr-deprecated-3.c
> > > @@ -6,6 +6,6 @@
> > > [[deprecated(0)]] int b; /* { dg-error "expected" } */
> > > -[[deprecated("", 123)]] int c; /* { dg-error "expected" } */
> > > +[[deprecated("", 123)]] int c; /* { dg-error "only allowed in GNU attributes" } */
> > > [[deprecated((""))]] int d; /* { dg-error "expected" } */
> > > diff --git a/gcc/tree.c b/gcc/tree.c
> > > index 561b9cd56bd..5cb2d57a960 100644
> > > --- a/gcc/tree.c
> > > +++ b/gcc/tree.c
> > > @@ -12002,8 +12002,9 @@ bool
> > > warn_deprecated_use (tree node, tree attr)
> > > {
> > > escaped_string msg;
> > > + escaped_string rep;
> > > - if (node == 0 || !warn_deprecated_decl)
> > > + if (node == NULL_TREE || !warn_deprecated_decl)
> > > return false;
> > > if (!attr)
> > > @@ -12023,7 +12024,12 @@ warn_deprecated_use (tree node, tree attr)
> > > attr = lookup_attribute ("deprecated", attr);
> > > if (attr)
> > > - msg.escape (TREE_STRING_POINTER (TREE_VALUE (TREE_VALUE (attr))));
> > > + {
> > > + tree val = TREE_VALUE (attr);
> > > + msg.escape (TREE_STRING_POINTER (TREE_VALUE (val)));
> > > + if (TREE_CHAIN (val))
> > > + rep.escape (TREE_STRING_POINTER (TREE_VALUE (TREE_CHAIN (val))));
> > > + }
> > > bool w = false;
> > > if (DECL_P (node))
> > > @@ -12036,7 +12042,11 @@ warn_deprecated_use (tree node, tree attr)
> > > w = warning (OPT_Wdeprecated_declarations,
> > > "%qD is deprecated", node);
> > > if (w)
> > > - inform (DECL_SOURCE_LOCATION (node), "declared here");
> > > + {
> > > + if (rep)
> > > + inform (input_location, "use %qs instead", (const char *) rep);
> > > + inform (DECL_SOURCE_LOCATION (node), "declared here");
> > > + }
> > > }
> > > else if (TYPE_P (node))
> > > {
> > > @@ -12072,8 +12082,13 @@ warn_deprecated_use (tree node, tree attr)
> > > "type is deprecated");
> > > }
> > > - if (w && decl)
> > > - inform (DECL_SOURCE_LOCATION (decl), "declared here");
> > > + if (w)
> > > + {
> > > + if (rep)
> > > + inform (input_location, "use %qs instead", (const char *) rep);
> > > + if (decl)
> > > + inform (DECL_SOURCE_LOCATION (decl), "declared here");
> > > + }
> > > }
> > > return w;
> > >
> > > base-commit: f6a05b23cc2e53c38e8321ddb5d2cbe40737e506
> > > --
> > > 2.31.1
> > >
> >
> > Marek
> >
>
Marek
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-11-02 20:36 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-23 16:16 [PATCH] attribs: Allow optional second arg for attr deprecated [PR102049] Marek Polacek
2021-10-11 15:17 ` Marek Polacek
2021-10-29 21:51 ` Eric Gallager
2021-11-01 23:15 ` Martin Sebor
2021-11-02 20:35 ` 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).