* [Patch c++] (__attribute__((deprecated)), part a) Fix PR17729 @ 2010-04-17 12:47 IainS 2010-06-08 23:12 ` Jason Merrill 0 siblings, 1 reply; 7+ messages in thread From: IainS @ 2010-04-17 12:47 UTC (permalink / raw) To: GCC Patches; +Cc: Joseph S. Myers [-- Attachment #1: Type: text/plain, Size: 676 bytes --] Split-up of http://gcc.gnu.org/ml/gcc-patches/2010-04/msg01037.html Part a. I guess the testcase is somewhat moot because of http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30612 ... but I included for completeness. regtested on i686-apple-darwin9 OK for trunk? (and since this dates back to 4.3 .. I guess at least 4.5 ? ) Iain. gcc/cp/Changelog: PR c++/17729 * typeck.c(build_class_member_access_expr): Don't check for deprecation here. * call.c(build_call_a): Ditto. (build_over_call): Ditto. (convert_like_real): Check for deprecation here instead. (build_new_method_call): Ditto. gcc/testsuite/Changelog: PR c++/17729 * g++.dg/warm/deprecated-7.C: New. [-- Attachment #2: 158459-cp-dup-errs-diff.txt --] [-- Type: text/plain, Size: 3664 bytes --] Index: gcc/cp/typeck.c =================================================================== --- gcc/cp/typeck.c (revision 158459) +++ gcc/cp/typeck.c (working copy) @@ -2149,9 +2149,7 @@ build_class_member_access_expr (tree object, tree { member_scope = DECL_CLASS_CONTEXT (member); mark_used (member); - if (TREE_DEPRECATED (member)) - warn_deprecated_use (member, NULL_TREE); - } + } else member_scope = BINFO_TYPE (BASELINK_ACCESS_BINFO (member)); /* If MEMBER is from an anonymous aggregate, MEMBER_SCOPE will Index: gcc/cp/call.c =================================================================== --- gcc/cp/call.c (revision 158459) +++ gcc/cp/call.c (working copy) @@ -344,8 +344,6 @@ build_call_a (tree function, int n, tree *argarray if (decl && TREE_THIS_VOLATILE (decl) && cfun && cp_function_chain) current_function_returns_abnormally = 1; - if (decl && TREE_DEPRECATED (decl)) - warn_deprecated_use (decl, NULL_TREE); require_complete_eh_spec_types (fntype, decl); if (decl && DECL_CONSTRUCTOR_P (decl)) @@ -4904,6 +4902,10 @@ convert_like_real (conversion *convs, tree expr, t for (i = 0; i < cand->num_convs; ++i) cand->convs[i]->user_conv_p = true; + /* Warn on deprecation of the candidate conversion func. */ + if (TREE_DEPRECATED (convfn)) + warn_deprecated_use (convfn, NULL_TREE); + expr = build_over_call (cand, LOOKUP_NORMAL, complain); /* If this is a constructor or a function returning an aggr type, @@ -5864,11 +5866,6 @@ build_over_call (struct z_candidate *cand, int fla ba_any, NULL); gcc_assert (binfo && binfo != error_mark_node); - /* Warn about deprecated virtual functions now, since we're about - to throw away the decl. */ - if (TREE_DEPRECATED (fn)) - warn_deprecated_use (fn, NULL_TREE); - argarray[0] = build_base_path (PLUS_EXPR, argarray[0], binfo, 1); if (TREE_SIDE_EFFECTS (argarray[0])) argarray[0] = save_expr (argarray[0]); @@ -6458,6 +6455,11 @@ build_new_method_call (tree instance, tree fns, VE /* Now we know what function is being called. */ if (fn_p) *fn_p = fn; + + /* Check for deprecation of the function to the called. */ + if (TREE_DEPRECATED (fn)) + warn_deprecated_use (fn, NULL_TREE); + /* Build the actual CALL_EXPR. */ call = build_over_call (cand, flags, complain); /* In an expression of the form `a->f()' where `f' turns Index: gcc/testsuite/g++.dg/warn/deprecated-7.C =================================================================== --- gcc/testsuite/g++.dg/warn/deprecated-7.C (revision 0) +++ gcc/testsuite/g++.dg/warn/deprecated-7.C (revision 0) @@ -0,0 +1,34 @@ +// PR c++/17729 Multiple warnings for single errors. +// { dg-do compile } + +void func(void) __attribute__((deprecated)); + +void f(void) { + func(); // { dg-warning "'void func\\(\\)' is deprecated" "func()" } +// { dg-warning "'void func\\(\\)' is deprecated" "func()" { ! target *-*-* } 7 } +} + +struct s1 { + int notdep ; + int depfield __attribute__((deprecated)); + union { + int notdep; + int deprec __attribute__((deprecated)); + } u1 ; + union { + int notdep1; + int notdep2; + } u2 __attribute__((deprecated)) ; + +} ; + +int main(void) { + struct s1 a; + int b; + b = a.notdep; + b += a.depfield; // { dg-warning "'s1::depfield' is deprecated .declared at" "depfield" } + b += a.u1.notdep; + b += a.u1.deprec; // { dg-warning "'s1::<anonymous union>::deprec' is deprecated .declared at" "s1::u1.deprec" } + b += a.u2.notdep1 ;// { dg-warning "'s1::u2' is deprecated .declared at" "s1::u2" } + return 0; +} [-- Attachment #3: Type: text/plain, Size: 1 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Patch c++] (__attribute__((deprecated)), part a) Fix PR17729 2010-04-17 12:47 [Patch c++] (__attribute__((deprecated)), part a) Fix PR17729 IainS @ 2010-06-08 23:12 ` Jason Merrill 2010-06-09 0:26 ` IainS 0 siblings, 1 reply; 7+ messages in thread From: Jason Merrill @ 2010-06-08 23:12 UTC (permalink / raw) To: IainS; +Cc: GCC Patches, Joseph S. Myers On 04/17/2010 07:16 AM, IainS wrote: > PR c++/17729 > * typeck.c(build_class_member_access_expr): Don't check for deprecation > here. I believe this will cause false negatives on template code. > * call.c(build_call_a): Ditto. > (build_over_call): Ditto. > (convert_like_real): Check for deprecation here instead. > (build_new_method_call): Ditto. And these cause false negatives on overloaded functions. We really need to handle functions in build_call_a and build_over_call, since that's when we know which function we're actually calling in the case of function overloading. In the testcase, if you add a non-deprecated func(int), you get no warning about calling the deprecated func(void). The testcase also doesn't test member functions at all. Sorry for the slow review; please feel free to ping me directly on C++ patches. Jason ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Patch c++] (__attribute__((deprecated)), part a) Fix PR17729 2010-06-08 23:12 ` Jason Merrill @ 2010-06-09 0:26 ` IainS 2010-06-09 3:29 ` Jason Merrill 0 siblings, 1 reply; 7+ messages in thread From: IainS @ 2010-06-09 0:26 UTC (permalink / raw) To: Jason Merrill; +Cc: GCC Patches, Joseph S. Myers On 8 Jun 2010, at 23:50, Jason Merrill wrote: > On 04/17/2010 07:16 AM, IainS wrote: >> PR c++/17729 >> * typeck.c(build_class_member_access_expr): Don't check for >> deprecation >> here. > > I believe this will cause false negatives on template code. > >> * call.c(build_call_a): Ditto. >> (build_over_call): Ditto. >> (convert_like_real): Check for deprecation here instead. >> (build_new_method_call): Ditto. > > And these cause false negatives on overloaded functions. We really > need to handle functions in build_call_a and build_over_call, since > that's when we know which function we're actually calling in the > case of function overloading. the question then, is how to prevent the duplicates, since those places are called from more than one part of the process (or, at least, they were when I last looked). > In the testcase, if you add a non-deprecated func(int), you get no > warning about calling the deprecated func(void). The testcase also > doesn't test member functions at all. OK. I wasn't amazingly happy that this had all bases covered. I expanded the test-cases a little - but they are mostly based on what's already in the test-suite. ---- Actually, I intended to get back to this in the next few weeks - since __attribute__((unavailable)) is a prerequisite of ObjC V1. Before I put this to one side, I had improved the diagnosis of deprecated function arguments - and also harmonized the behavior of function deprecation messages between C and C++. I am sure all those patches will have severe bit-rot now - but I'll try and resurrect them soon. > Sorry for the slow review; please feel free to ping me directly on C+ > + patches. thanks, I'm going slowly here anyway, Iain ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Patch c++] (__attribute__((deprecated)), part a) Fix PR17729 2010-06-09 0:26 ` IainS @ 2010-06-09 3:29 ` Jason Merrill 2010-07-19 17:11 ` IainS 0 siblings, 1 reply; 7+ messages in thread From: Jason Merrill @ 2010-06-09 3:29 UTC (permalink / raw) To: IainS; +Cc: GCC Patches, Joseph S. Myers On 06/08/2010 07:13 PM, IainS wrote: > the question then, is how to prevent the duplicates, since those places > are called from more than one part of the process (or, at least, they > were when I last looked). For non-function members, I think removing the call in finish_class_member_access_expr ought to do the trick, letting build_class_member_access_expr handle the warning. For functions, perhaps just warning in mark_used would work. Then removing the calls in build_over_call and build_call_a and conditioning the one in finish_id_expression on !is_overloaded_fn. Jason ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Patch c++] (__attribute__((deprecated)), part a) Fix PR17729 2010-06-09 3:29 ` Jason Merrill @ 2010-07-19 17:11 ` IainS 2010-07-19 17:59 ` Jason Merrill 0 siblings, 1 reply; 7+ messages in thread From: IainS @ 2010-07-19 17:11 UTC (permalink / raw) To: Jason Merrill; +Cc: GCC Patches, Joseph S. Myers [-- Attachment #1: Type: text/plain, Size: 2184 bytes --] Hi Jason, It's taken a while to find some more time for this - (ObJC LTO and emutls.... etc. etc.) On 9 Jun 2010, at 04:19, Jason Merrill wrote: > On 06/08/2010 07:13 PM, IainS wrote: >> the question then, is how to prevent the duplicates, since those >> places >> are called from more than one part of the process (or, at least, they >> were when I last looked). > > For non-function members, I think removing the call in > finish_class_member_access_expr ought to do the trick, letting > build_class_member_access_expr handle the warning. done > For functions, perhaps just warning in mark_used would work. Then > removing the calls in build_over_call and build_call_a and > conditioning the one in finish_id_expression on !is_overloaded_fn. unfortunately, not mark_used is called even more often than build_over_call & build_call_a (at one stage I ended up with three warning messages)... It seems that there is no nice place to put these diagnostics, that is only visited once (for the current formulation***); Hence we end up checking in several places (which, I agree, does not seem ideal). The attached patch has been bootstrapped/checked on i686-apple-darwin9 I've added a test-case for overloaded functions, member funcs are covered in deprecated-{4,6} iirc. thoughts? Iain *** It seems to me that the diagnostics might be better done at the stage that the parser has decided that (other than deprecation) a given construct is OK. (I guess just before swallowing the ";"). However, I don't know if this is reasonable, &| there would be enough information preserved to make the diagnostic neat (although it seems that there should). gcc/cp: PR c++/17729 * typeck.c (finish_class_member_access_expr): Remove duplicate deprecation check. * call.c (build_call_a): Do not check deprecation here. (build_over_call): Ditto. (build_new_function_call): Check for deprecated overloaded funcs here. (convert_like_real): Check for deprecated functions. (build_new_method_call): Ditto. gcc/testsuite: PR c++/17729 * g++.dg/warn/pr17729-1-fields.C: New. * g++.dg/warn/pr17729-2-funcs.C: New. * g++.dg/warn/deprecated-7.C: New. [-- Attachment #2: 162302-pr17729.txt --] [-- Type: text/plain, Size: 5090 bytes --] Index: gcc/cp/typeck.c =================================================================== --- gcc/cp/typeck.c (revision 162309) +++ gcc/cp/typeck.c (working copy) @@ -2692,9 +2692,6 @@ finish_class_member_access_expr (tree object, tree } } - if (TREE_DEPRECATED (member)) - warn_deprecated_use (member, NULL_TREE); - if (template_p) check_template_keyword (member); Index: gcc/cp/call.c =================================================================== --- gcc/cp/call.c (revision 162309) +++ gcc/cp/call.c (working copy) @@ -347,8 +347,6 @@ build_call_a (tree function, int n, tree *argarray if (decl && TREE_THIS_VOLATILE (decl) && cfun && cp_function_chain) current_function_returns_abnormally = 1; - if (decl && TREE_DEPRECATED (decl)) - warn_deprecated_use (decl, NULL_TREE); require_complete_eh_spec_types (fntype, decl); if (decl && DECL_CONSTRUCTOR_P (decl)) @@ -3283,8 +3281,11 @@ build_new_function_call (tree fn, VEC(tree,gc) **a result = error_mark_node; } else - result = build_over_call (cand, LOOKUP_NORMAL, complain); - + { + if (koenig_p && TREE_DEPRECATED (cand->fn)) + warn_deprecated_use (cand->fn, NULL_TREE); + result = build_over_call (cand, LOOKUP_NORMAL, complain); + } /* Free all the conversions we allocated. */ obstack_free (&conversion_obstack, p); @@ -5018,6 +5019,10 @@ convert_like_real (conversion *convs, tree expr, t for (i = 0; i < cand->num_convs; ++i) cand->convs[i]->user_conv_p = true; + /* Warn on deprecation of the candidate conversion func. */ + if (TREE_DEPRECATED (convfn)) + warn_deprecated_use (convfn, NULL_TREE); + expr = build_over_call (cand, LOOKUP_NORMAL, complain); /* If this is a constructor or a function returning an aggr type, @@ -6010,11 +6015,6 @@ build_over_call (struct z_candidate *cand, int fla ba_any, NULL); gcc_assert (binfo && binfo != error_mark_node); - /* Warn about deprecated virtual functions now, since we're about - to throw away the decl. */ - if (TREE_DEPRECATED (fn)) - warn_deprecated_use (fn, NULL_TREE); - argarray[0] = build_base_path (PLUS_EXPR, argarray[0], binfo, 1); if (TREE_SIDE_EFFECTS (argarray[0])) argarray[0] = save_expr (argarray[0]); @@ -6575,6 +6575,11 @@ build_new_method_call (tree instance, tree fns, VE /* Now we know what function is being called. */ if (fn_p) *fn_p = fn; + + /* Check for deprecation of the function to be called. */ + if (TREE_DEPRECATED (fn)) + warn_deprecated_use (fn, NULL_TREE); + /* Build the actual CALL_EXPR. */ call = build_over_call (cand, flags, complain); /* In an expression of the form `a->f()' where `f' turns Index: gcc/testsuite/g++.dg/warn/pr17729-2-funcs.C =================================================================== --- gcc/testsuite/g++.dg/warn/pr17729-2-funcs.C (revision 0) +++ gcc/testsuite/g++.dg/warn/pr17729-2-funcs.C (revision 0) @@ -0,0 +1,9 @@ +/* PR 17729 */ +/* { dg-do compile } */ +int fun (void) __attribute__((deprecated("no fun"))) ; + +void foo (void) +{ + fun (); // { dg-bogus "int fun.* no fun.*int fun" "duplicate func. warning" } + // { dg-warning "'int fun..' is deprecated .declared at \[^\\)\]*.: no fun" "" { target *-*-* } 7 } +} Index: gcc/testsuite/g++.dg/warn/deprecated-7.C =================================================================== --- gcc/testsuite/g++.dg/warn/deprecated-7.C (revision 0) +++ gcc/testsuite/g++.dg/warn/deprecated-7.C (revision 0) @@ -0,0 +1,16 @@ +/* { dg-do compile } */ + +int f1 (int, int); +int f1 (float, int) __attribute__((deprecated("no flt, int"))) ; +int f1 (float, float); + +void foo (void) +{ + int a, b, r; + float c,d; + + r = f1 (a, b); + r = f1 (c, a); // { dg-warning "'int f1.float, int.' is deprecated .declared at \[^\\)\]*.: no flt, int" } + r = f1 (c, d); + +} \ No newline at end of file Index: gcc/testsuite/g++.dg/warn/pr17729-1-fields.C =================================================================== --- gcc/testsuite/g++.dg/warn/pr17729-1-fields.C (revision 0) +++ gcc/testsuite/g++.dg/warn/pr17729-1-fields.C (revision 0) @@ -0,0 +1,20 @@ +/* PR 17729 */ +/* { dg-do compile } */ +typedef int INT1 __attribute__((deprecated("no INT1"))) ; + +struct _a { + int field1 ; + int field2 __attribute__((deprecated("no field2"))) ; + INT1 field3; /* { dg-warning "'INT1' is deprecated .declared \[^\\)\]*.: no INT1 " } */ + INT1 field4 __attribute__((deprecated("no field4"))); /* { dg-warning "'INT1' is deprecated .declared .* no INT1 " } */ +} s; + +void foo (void) +{ + s.field1 = 1; + s.field2 = 2; // { dg-bogus "_a::field2.*_a::field2" "duplicate field warning" } + // { dg-warning "'_a::field2' is deprecated .declared at \[^\\)\]*.: no field2" "" { target *-*-* } 15 } + s.field3 = 3; + s.field4 = 4; // { dg-bogus "_a::field4.*_a::field4" "duplicate field warning" } + // { dg-warning "'_a::field4' is deprecated .declared at .* no field4" "" { target *-*-* } 18 } +} \ No newline at end of file [-- Attachment #3: Type: text/plain, Size: 2 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Patch c++] (__attribute__((deprecated)), part a) Fix PR17729 2010-07-19 17:11 ` IainS @ 2010-07-19 17:59 ` Jason Merrill 2010-07-21 13:43 ` IainS 0 siblings, 1 reply; 7+ messages in thread From: Jason Merrill @ 2010-07-19 17:59 UTC (permalink / raw) To: IainS; +Cc: GCC Patches, Joseph S. Myers On 07/19/2010 01:11 PM, IainS wrote: > unfortunately, not > mark_used is called even more often than build_over_call & build_call_a > (at one stage I ended up with three warning messages)... > > It seems that there is no nice place to put these diagnostics, that is > only visited once (for the current formulation***); > Hence we end up checking in several places (which, I agree, does not > seem ideal). How about just using a hash table, as in maybe_explain_implicit_delete? Jason ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Patch c++] (__attribute__((deprecated)), part a) Fix PR17729 2010-07-19 17:59 ` Jason Merrill @ 2010-07-21 13:43 ` IainS 0 siblings, 0 replies; 7+ messages in thread From: IainS @ 2010-07-21 13:43 UTC (permalink / raw) To: Jason Merrill; +Cc: GCC Patches, Joseph S. Myers On 19 Jul 2010, at 18:58, Jason Merrill wrote: > On 07/19/2010 01:11 PM, IainS wrote: >> unfortunately, not >> mark_used is called even more often than build_over_call & >> build_call_a >> (at one stage I ended up with three warning messages)... >> >> It seems that there is no nice place to put these diagnostics, that >> is >> only visited once (for the current formulation***); >> Hence we end up checking in several places (which, I agree, does not >> seem ideal). > > How about just using a hash table, as in > maybe_explain_implicit_delete? I did some more back-tracing yesterday (to find where the duplicate calls are coming from); I'll analyze that a bit more later, and maybe augment it ... one point that this revealed is the types are not marked as used (at least not in this manner). presumably there is some equivalent action so that we know which ones to emit for debug? cheers, Iain ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2010-07-21 13:43 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2010-04-17 12:47 [Patch c++] (__attribute__((deprecated)), part a) Fix PR17729 IainS 2010-06-08 23:12 ` Jason Merrill 2010-06-09 0:26 ` IainS 2010-06-09 3:29 ` Jason Merrill 2010-07-19 17:11 ` IainS 2010-07-19 17:59 ` Jason Merrill 2010-07-21 13:43 ` IainS
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).