| Message ID | 20180314222600.GM8577@tucnak |
|---|---|
| State | New |
| Headers | show |
| Series | [C++] Fix up -Wdeprecated (PR c++/84222) | expand |
On Wed, Mar 14, 2018 at 6:26 PM, Jakub Jelinek <jakub@redhat.com> wrote: > As the following testcase shows, if we have a deprecated class, we warn > about any uses, including e.g. arguments of methods of that class (how can > one e.g. declare or define a copy ctor without warnings?). > > The following patch changes it, so that we don't warn about deprecated uses > in methods of that deprecated class (warn about uses of other deprecated > classes of course). There is still one xfailed case where we warn about > template-id in the containing scope if the deprecated class is a template. > > clang++ warns about the bar (const C &, const C &) function, both about > the parameters and about the use in the body (like g++) and doesn't warn > inside of (perhaps uninstantiated only) templates at all (which I think is > better that we do warn). > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > 2018-03-14 Jakub Jelinek <jakub@redhat.com> > > PR c++/84222 > * cp-tree.h (cp_warn_deprecated_use): Declare. > * tree.c (cp_warn_deprecated_use): New function. > * typeck2.c (build_functional_cast): Use it. > * decl.c (grokparms): Likewise. I like these. > (grokdeclarator): Likewise. Temporarily push nested class scope > around grokparms call for out of class member definitions. > > * g++.dg/warn/deprecated.C (T::member3): Change dg-warning to dg-bogus. > * g++.dg/warn/deprecated-6.C (T::member3): Likewise. > * g++.dg/warn/deprecated-13.C: New test. > > --- gcc/cp/cp-tree.h.jj 2018-03-11 17:48:36.360061435 +0100 > +++ gcc/cp/cp-tree.h 2018-03-14 11:49:58.924816419 +0100 > @@ -7064,6 +7064,7 @@ extern tree cxx_copy_lang_qualifiers (c > > extern void cxx_print_statistics (void); > extern bool maybe_warn_zero_as_null_pointer_constant (tree, location_t); > +extern void cp_warn_deprecated_use (tree); > > /* in ptree.c */ > extern void cxx_print_xnode (FILE *, tree, int); > --- gcc/cp/tree.c.jj 2018-03-07 22:51:58.671478659 +0100 > +++ gcc/cp/tree.c 2018-03-14 11:49:58.926816421 +0100 > @@ -5347,6 +5347,19 @@ cp_tree_code_length (enum tree_code code > } > } > > +/* Wrapper around warn_deprecated_use that doesn't warn for > + current_class_type. */ > + > +void > +cp_warn_deprecated_use (tree node) > +{ > + if (TYPE_P (node) > + && current_class_type > + && TYPE_MAIN_VARIANT (node) == current_class_type) > + return; > + warn_deprecated_use (node, NULL_TREE); > +} > + > /* Implement -Wzero_as_null_pointer_constant. Return true if the > conditions for the warning hold, false otherwise. */ > bool > --- gcc/cp/typeck2.c.jj 2018-03-02 00:15:54.096781050 +0100 > +++ gcc/cp/typeck2.c 2018-03-14 11:49:58.931816424 +0100 > @@ -2057,7 +2057,7 @@ build_functional_cast (tree exp, tree pa > if (complain & tf_warning > && TREE_DEPRECATED (type) > && DECL_ARTIFICIAL (exp)) > - warn_deprecated_use (type, NULL_TREE); > + cp_warn_deprecated_use (type); > } > else > type = exp; > --- gcc/cp/decl.c.jj 2018-03-14 09:44:55.744974946 +0100 > +++ gcc/cp/decl.c 2018-03-14 12:18:08.094012453 +0100 > @@ -10448,7 +10448,7 @@ grokdeclarator (const cp_declarator *dec > suppress reports of deprecated items. */ > if (type && TREE_DEPRECATED (type) > && deprecated_state != DEPRECATED_SUPPRESS) > - warn_deprecated_use (type, NULL_TREE); > + cp_warn_deprecated_use (type); > if (type && TREE_CODE (type) == TYPE_DECL) > { > typedef_decl = type; > @@ -10456,7 +10456,7 @@ grokdeclarator (const cp_declarator *dec > if (TREE_DEPRECATED (type) > && DECL_ARTIFICIAL (typedef_decl) > && deprecated_state != DEPRECATED_SUPPRESS) > - warn_deprecated_use (type, NULL_TREE); > + cp_warn_deprecated_use (type); > } > /* No type at all: default to `int', and set DEFAULTED_INT > because it was not a user-defined typedef. */ > @@ -11271,8 +11271,18 @@ grokdeclarator (const cp_declarator *dec > explicitp = 2; > } > > - arg_types = grokparms (declarator->u.function.parameters, > - &parms); > + tree pushed_scope = NULL_TREE; > + if (funcdecl_p > + && decl_context != FIELD > + && inner_declarator->u.id.qualifying_scope > + && CLASS_TYPE_P (inner_declarator->u.id.qualifying_scope)) > + pushed_scope > + = push_scope (inner_declarator->u.id.qualifying_scope); Can't we use ctype here? Jason
On Wed, Mar 14, 2018 at 08:44:48PM -0400, Jason Merrill wrote: > > --- gcc/cp/decl.c.jj 2018-03-14 09:44:55.744974946 +0100 > > +++ gcc/cp/decl.c 2018-03-14 12:18:08.094012453 +0100 > > @@ -10448,7 +10448,7 @@ grokdeclarator (const cp_declarator *dec > > suppress reports of deprecated items. */ > > if (type && TREE_DEPRECATED (type) > > && deprecated_state != DEPRECATED_SUPPRESS) > > - warn_deprecated_use (type, NULL_TREE); > > + cp_warn_deprecated_use (type); > > if (type && TREE_CODE (type) == TYPE_DECL) > > { > > typedef_decl = type; > > @@ -10456,7 +10456,7 @@ grokdeclarator (const cp_declarator *dec > > if (TREE_DEPRECATED (type) > > && DECL_ARTIFICIAL (typedef_decl) > > && deprecated_state != DEPRECATED_SUPPRESS) > > - warn_deprecated_use (type, NULL_TREE); > > + cp_warn_deprecated_use (type); > > } > > /* No type at all: default to `int', and set DEFAULTED_INT > > because it was not a user-defined typedef. */ > > @@ -11271,8 +11271,18 @@ grokdeclarator (const cp_declarator *dec > > explicitp = 2; > > } > > > > - arg_types = grokparms (declarator->u.function.parameters, > > - &parms); > > + tree pushed_scope = NULL_TREE; > > + if (funcdecl_p > > + && decl_context != FIELD > > + && inner_declarator->u.id.qualifying_scope > > + && CLASS_TYPE_P (inner_declarator->u.id.qualifying_scope)) > > + pushed_scope > > + = push_scope (inner_declarator->u.id.qualifying_scope); > > Can't we use ctype here? Inside of classes ctype is non-NULL, but we don't need to push anything, current_class_type is already the class we care about. That's the tree ctype = current_class_type; on line 10130. Then we have this for (id_declarator = declarator; id_declarator; id_declarator = id_declarator->declarator) loop, where for cdk_id at line 10242 it tweaks ctype: else if (TYPE_P (qualifying_scope)) { ctype = qualifying_scope; if (!MAYBE_CLASS_TYPE_P (ctype)) { error ("%q#T is not a class or a namespace", ctype); ctype = NULL_TREE; } and indeed for the cases I care about (out of class method definitions) ctype is set to non-NULL here. But then at line 10542 it is cleared again: ctype = NULL_TREE; and at 11176: if (ctype == NULL_TREE && decl_context == FIELD && funcdecl_p && friendp == 0) ctype = current_class_type; set only for selected in-class definitions, and then tested and used a couple of times. And that is the state we call grokparms with. Only later at line 11529 it is set again: if (declarator && declarator->kind == cdk_id && declarator->u.id.qualifying_scope && MAYBE_CLASS_TYPE_P (declarator->u.id.qualifying_scope)) { ctype = declarator->u.id.qualifying_scope; ctype = TYPE_MAIN_VARIANT (ctype); So, if I were to use some variable without really changing the behavior of the grokdeclarator massively, it would need to be a copy of ctype saved into another variable (how it should be named?) above line 10542. Given the TYPE_MAIN_VARIANT, I guess we should be using TYPE_MAIN_VARIANT somewhere too. Jakub
On Thu, Mar 15, 2018 at 4:07 AM, Jakub Jelinek <jakub@redhat.com> wrote: > On Wed, Mar 14, 2018 at 08:44:48PM -0400, Jason Merrill wrote: >> > --- gcc/cp/decl.c.jj 2018-03-14 09:44:55.744974946 +0100 >> > +++ gcc/cp/decl.c 2018-03-14 12:18:08.094012453 +0100 >> > @@ -10448,7 +10448,7 @@ grokdeclarator (const cp_declarator *dec >> > suppress reports of deprecated items. */ >> > if (type && TREE_DEPRECATED (type) >> > && deprecated_state != DEPRECATED_SUPPRESS) >> > - warn_deprecated_use (type, NULL_TREE); >> > + cp_warn_deprecated_use (type); >> > if (type && TREE_CODE (type) == TYPE_DECL) >> > { >> > typedef_decl = type; >> > @@ -10456,7 +10456,7 @@ grokdeclarator (const cp_declarator *dec >> > if (TREE_DEPRECATED (type) >> > && DECL_ARTIFICIAL (typedef_decl) >> > && deprecated_state != DEPRECATED_SUPPRESS) >> > - warn_deprecated_use (type, NULL_TREE); >> > + cp_warn_deprecated_use (type); >> > } >> > /* No type at all: default to `int', and set DEFAULTED_INT >> > because it was not a user-defined typedef. */ >> > @@ -11271,8 +11271,18 @@ grokdeclarator (const cp_declarator *dec >> > explicitp = 2; >> > } >> > >> > - arg_types = grokparms (declarator->u.function.parameters, >> > - &parms); >> > + tree pushed_scope = NULL_TREE; >> > + if (funcdecl_p >> > + && decl_context != FIELD >> > + && inner_declarator->u.id.qualifying_scope >> > + && CLASS_TYPE_P (inner_declarator->u.id.qualifying_scope)) >> > + pushed_scope >> > + = push_scope (inner_declarator->u.id.qualifying_scope); >> >> Can't we use ctype here? > > Inside of classes ctype is non-NULL, but we don't need to push anything, > current_class_type is already the class we care about. > That's the > tree ctype = current_class_type; > on line 10130. Then we have this > for (id_declarator = declarator; > id_declarator; > id_declarator = id_declarator->declarator) > loop, where for cdk_id at line 10242 it tweaks ctype: > else if (TYPE_P (qualifying_scope)) > { > ctype = qualifying_scope; > if (!MAYBE_CLASS_TYPE_P (ctype)) > { > error ("%q#T is not a class or a namespace", ctype); > ctype = NULL_TREE; > } > and indeed for the cases I care about (out of class method definitions) > ctype is set to non-NULL here. But then at line 10542 it is cleared again: > ctype = NULL_TREE; > > and at 11176: > if (ctype == NULL_TREE > && decl_context == FIELD > && funcdecl_p > && friendp == 0) > ctype = current_class_type; > set only for selected in-class definitions, and then tested and used a couple of > times. And that is the state we call grokparms with. > Only later at line 11529 it is set again: > if (declarator > && declarator->kind == cdk_id > && declarator->u.id.qualifying_scope > && MAYBE_CLASS_TYPE_P (declarator->u.id.qualifying_scope)) > { > ctype = declarator->u.id.qualifying_scope; > ctype = TYPE_MAIN_VARIANT (ctype); > So, if I were to use some variable without really changing the behavior of > the grokdeclarator massively, it would need to be a copy of ctype saved into > another variable (how it should be named?) above line 10542. Given the > TYPE_MAIN_VARIANT, I guess we should be using TYPE_MAIN_VARIANT somewhere > too. Wow, yeah, the use of ctype in grokdeclarator is pretty bizarre. Your patch is OK. Jason
--- gcc/cp/cp-tree.h.jj 2018-03-11 17:48:36.360061435 +0100 +++ gcc/cp/cp-tree.h 2018-03-14 11:49:58.924816419 +0100 @@ -7064,6 +7064,7 @@ extern tree cxx_copy_lang_qualifiers (c extern void cxx_print_statistics (void); extern bool maybe_warn_zero_as_null_pointer_constant (tree, location_t); +extern void cp_warn_deprecated_use (tree); /* in ptree.c */ extern void cxx_print_xnode (FILE *, tree, int); --- gcc/cp/tree.c.jj 2018-03-07 22:51:58.671478659 +0100 +++ gcc/cp/tree.c 2018-03-14 11:49:58.926816421 +0100 @@ -5347,6 +5347,19 @@ cp_tree_code_length (enum tree_code code } } +/* Wrapper around warn_deprecated_use that doesn't warn for + current_class_type. */ + +void +cp_warn_deprecated_use (tree node) +{ + if (TYPE_P (node) + && current_class_type + && TYPE_MAIN_VARIANT (node) == current_class_type) + return; + warn_deprecated_use (node, NULL_TREE); +} + /* Implement -Wzero_as_null_pointer_constant. Return true if the conditions for the warning hold, false otherwise. */ bool --- gcc/cp/typeck2.c.jj 2018-03-02 00:15:54.096781050 +0100 +++ gcc/cp/typeck2.c 2018-03-14 11:49:58.931816424 +0100 @@ -2057,7 +2057,7 @@ build_functional_cast (tree exp, tree pa if (complain & tf_warning && TREE_DEPRECATED (type) && DECL_ARTIFICIAL (exp)) - warn_deprecated_use (type, NULL_TREE); + cp_warn_deprecated_use (type); } else type = exp; --- gcc/cp/decl.c.jj 2018-03-14 09:44:55.744974946 +0100 +++ gcc/cp/decl.c 2018-03-14 12:18:08.094012453 +0100 @@ -10448,7 +10448,7 @@ grokdeclarator (const cp_declarator *dec suppress reports of deprecated items. */ if (type && TREE_DEPRECATED (type) && deprecated_state != DEPRECATED_SUPPRESS) - warn_deprecated_use (type, NULL_TREE); + cp_warn_deprecated_use (type); if (type && TREE_CODE (type) == TYPE_DECL) { typedef_decl = type; @@ -10456,7 +10456,7 @@ grokdeclarator (const cp_declarator *dec if (TREE_DEPRECATED (type) && DECL_ARTIFICIAL (typedef_decl) && deprecated_state != DEPRECATED_SUPPRESS) - warn_deprecated_use (type, NULL_TREE); + cp_warn_deprecated_use (type); } /* No type at all: default to `int', and set DEFAULTED_INT because it was not a user-defined typedef. */ @@ -11271,8 +11271,18 @@ grokdeclarator (const cp_declarator *dec explicitp = 2; } - arg_types = grokparms (declarator->u.function.parameters, - &parms); + tree pushed_scope = NULL_TREE; + if (funcdecl_p + && decl_context != FIELD + && inner_declarator->u.id.qualifying_scope + && CLASS_TYPE_P (inner_declarator->u.id.qualifying_scope)) + pushed_scope + = push_scope (inner_declarator->u.id.qualifying_scope); + + arg_types = grokparms (declarator->u.function.parameters, &parms); + + if (pushed_scope) + pop_scope (pushed_scope); if (inner_declarator && inner_declarator->kind == cdk_id @@ -12799,7 +12809,7 @@ grokparms (tree parmlist, tree *parms) { tree deptype = type_is_deprecated (type); if (deptype) - warn_deprecated_use (deptype, NULL_TREE); + cp_warn_deprecated_use (deptype); } /* Top-level qualifiers on the parameters are --- gcc/testsuite/g++.dg/warn/deprecated.C.jj 2017-04-28 22:16:51.986480480 +0200 +++ gcc/testsuite/g++.dg/warn/deprecated.C 2018-03-14 12:51:54.847224549 +0100 @@ -102,7 +102,7 @@ T *p3; // { dg-warning "'T' is deprec inline void T::member1(int) {} -int T::member3(T *p) // { dg-warning "'T' is deprecated" } +int T::member3(T *p) // { dg-bogus "'T' is deprecated" } { p->member1(1); /* { dg-warning "'void T::member1\\(int\\)' is deprecated" } */ (*p).member1(2); /* { dg-warning "'void T::member1\\(int\\)' is deprecated" } */ @@ -113,5 +113,3 @@ int T::member3(T *p) // { dg-warning "' return f1(); /* { dg-warning "'INT1 f1\\(\\)' is deprecated" } */ } #endif - - --- gcc/testsuite/g++.dg/warn/deprecated-6.C.jj 2017-04-28 22:16:51.970480683 +0200 +++ gcc/testsuite/g++.dg/warn/deprecated-6.C 2018-03-14 12:51:15.721213235 +0100 @@ -98,7 +98,7 @@ T *p3; // { dg-warning "'T' is deprec inline void T::member1(int) {} -int T::member3(T *p) // { dg-warning "'T' is deprecated: Please avoid T" } +int T::member3(T *p) // { dg-bogus "'T' is deprecated: Please avoid T" } { p->member1(1); /* { dg-warning "'void T::member1\\(int\\)' is deprecated: Please avoid member1" } */ (*p).member1(2); /* { dg-warning "'void T::member1\\(int\\)' is deprecated: Please avoid member1" } */ --- gcc/testsuite/g++.dg/warn/deprecated-13.C.jj 2018-03-14 12:34:43.523210515 +0100 +++ gcc/testsuite/g++.dg/warn/deprecated-13.C 2018-03-14 12:34:13.103233036 +0100 @@ -0,0 +1,44 @@ +// PR c++/84222 +// { dg-do compile } + +struct __attribute__((deprecated)) C { // { dg-message "declared here" } + C () {} + C (const C &); // { dg-bogus "'C' is deprecated" } + C (const C &x, const C &y) { C z = x; } // { dg-bogus "'C' is deprecated" } + void foo (const C &x, const C &y); // { dg-bogus "'C' is deprecated" } +}; + +void +C::foo (const C &x, const C &y) // { dg-bogus "'C' is deprecated" } +{ + C z = x; // { dg-bogus "'C' is deprecated" } +} + +void +bar (const C &x, const C &y) // { dg-warning "'C' is deprecated" } +{ + C z = x; // { dg-warning "'C' is deprecated" } +} + +template <int N> +struct __attribute__((deprecated)) D { // { dg-message "declared here" } + D () {} + D (const D &); // { dg-bogus "is deprecated" } + D (const D &x, const D &y) { D z = x; } // { dg-bogus "is deprecated" } + void foo (const D &x, const D &y); // { dg-bogus "is deprecated" } +}; + +template <int N> +void +D<N>::foo // { dg-bogus "is deprecated" "" { xfail *-*-* } } +(const D &x, const D &y) // { dg-bogus "is deprecated" } +{ + D z = x; // { dg-bogus "is deprecated" } +} + +template <int N> +void +bar (const D<N> &x, const D<N> &y) // { dg-warning "is deprecated" } +{ + D<N> z = x; // { dg-warning "is deprecated" } +}