Message ID | CADNgcEzv0gdtADDwE1V4yn1ruwVE32eOocXk_HmLtcPhDiWu3w@mail.gmail.com |
---|---|
State | New |
Headers | show |
On Thu, Nov 7, 2013 at 11:53 AM, Mingjie Xing <mingjie.xing@gmail.com> wrote: > 2013/11/6 Richard Biener <richard.guenther@gmail.com>: >> You miss a testcase. >> >> Also why should the warning be omitted for unused automatic >> volatile variables? They cannot be used in any way. >> >> Richard. > > Thanks. I've updated the patch with a test case. > > c/ChangeLog > > PR 57258 Should be: PR c/57258 Thanks, bin > * c-decl.c (pop_scope): Don't emit unused variable warnings for > accessed volatile variables. > > testsuite/ChangeLog > > * gcc.dg/Wunused-var-4.c: New test. > > Mingjie
- warning_at (DECL_SOURCE_LOCATION (p), - OPT_Wunused_but_set_variable, - "variable %qD set but not used", p); + { + if (!TREE_THIS_VOLATILE (p)) + warning_at (DECL_SOURCE_LOCATION (p), + OPT_Wunused_but_set_variable, + "variable %qD set but not used", p); + } I'd prefer else if (DECL_CONTEXT (p) == current_function_decl && !TREE_THIS_VOLATILE (p)) which concises the logic and avoids indent change Thanks, Joey On Thu, Nov 7, 2013 at 3:37 PM, Bin.Cheng <amker.cheng@gmail.com> wrote: > On Thu, Nov 7, 2013 at 11:53 AM, Mingjie Xing <mingjie.xing@gmail.com> wrote: >> 2013/11/6 Richard Biener <richard.guenther@gmail.com>: >>> You miss a testcase. >>> >>> Also why should the warning be omitted for unused automatic >>> volatile variables? They cannot be used in any way. >>> >>> Richard. >> >> Thanks. I've updated the patch with a test case. >> >> c/ChangeLog >> >> PR 57258 > > Should be: > PR c/57258 > > Thanks, > bin >> * c-decl.c (pop_scope): Don't emit unused variable warnings for >> accessed volatile variables. >> >> testsuite/ChangeLog >> >> * gcc.dg/Wunused-var-4.c: New test. >> >> Mingjie > > > > -- > Best Regards.
On Thu, 7 Nov 2013, Mingjie Xing wrote: > 2013/11/6 Richard Biener <richard.guenther@gmail.com>: > > You miss a testcase. > > > > Also why should the warning be omitted for unused automatic > > volatile variables? They cannot be used in any way. > > > > Richard. > > Thanks. I've updated the patch with a test case. You don't seem to have answered Richard's question about why the change is desirable in the first place.
2013/11/7 Joseph S. Myers <joseph@codesourcery.com>: > On Thu, 7 Nov 2013, Mingjie Xing wrote: > >> 2013/11/6 Richard Biener <richard.guenther@gmail.com>: >> > You miss a testcase. >> > >> > Also why should the warning be omitted for unused automatic >> > volatile variables? They cannot be used in any way. >> > >> > Richard. >> >> Thanks. I've updated the patch with a test case. > > You don't seem to have answered Richard's question about why the change is > desirable in the first place. Well, it is my understanding that the warning should be emitted for a volatile variable only if it is not accessed. Initialization means accessing, even though it is not used anywhere. Mingjie
On Nov 7, 2013, at 5:13 PM, Mingjie Xing <mingjie.xing@gmail.com> wrote: > Well, it is my understanding that the warning should be emitted for a > volatile variable only if it is not accessed. Initialization means > accessing, even though it is not used anywhere. Let me try. A warning is useful, if there is no way a conforming program can tell that the variable exists or not. So, the question is, how can you notice the variable? Answer, there is no way, so, there is no utility in having the variable. The warning is to tell the user to remove the dead variable.
On Fri, Nov 8, 2013 at 5:03 PM, Mike Stump <mikestump@comcast.net> wrote: > On Nov 7, 2013, at 5:13 PM, Mingjie Xing <mingjie.xing@gmail.com> wrote: >> Well, it is my understanding that the warning should be emitted for a >> volatile variable only if it is not accessed. Initialization means >> accessing, even though it is not used anywhere. > > Let me try. A warning is useful, if there is no way a conforming program can tell that the variable exists or not. So, the question is, how can you notice the variable? Answer, there is no way, so, there is no utility in having the variable. The warning is to tell the user to remove the dead variable. I am sort of lost. The bug is straightforward and it suggests the volatile variable is actually used if it is declared with an initialization, and GCC should not emit warning message for it. The patch is fixing this, right? Any explanation? Thanks, bin
On Nov 8, 2013, at 1:32 AM, Bin.Cheng <amker.cheng@gmail.com> wrote: > On Fri, Nov 8, 2013 at 5:03 PM, Mike Stump <mikestump@comcast.net> wrote: >> On Nov 7, 2013, at 5:13 PM, Mingjie Xing <mingjie.xing@gmail.com> wrote: >>> Well, it is my understanding that the warning should be emitted for a >>> volatile variable only if it is not accessed. Initialization means >>> accessing, even though it is not used anywhere. >> >> Let me try. A warning is useful, if there is no way a conforming program can tell that the variable exists or not. So, the question is, how can you notice the variable? Answer, there is no way, so, there is no utility in having the variable. The warning is to tell the user to remove the dead variable. > > I am sort of lost. I can try again. Begin your sentence, the important utility of this construct is demonstrated by the following code: See if you can complete it. If not, then, then there is no utility. The warning says, there is no utility. This isn't a theoretic thing, it is an engineering thing.
On Fri, Nov 8, 2013 at 10:39 AM, Mike Stump <mikestump@comcast.net> wrote: > > On Nov 8, 2013, at 1:32 AM, Bin.Cheng <amker.cheng@gmail.com> wrote: > >> On Fri, Nov 8, 2013 at 5:03 PM, Mike Stump <mikestump@comcast.net> wrote: >>> On Nov 7, 2013, at 5:13 PM, Mingjie Xing <mingjie.xing@gmail.com> wrote: >>>> Well, it is my understanding that the warning should be emitted for a >>>> volatile variable only if it is not accessed. Initialization means >>>> accessing, even though it is not used anywhere. >>> >>> Let me try. A warning is useful, if there is no way a conforming program can tell that the variable exists or not. So, the question is, how can you notice the variable? Answer, there is no way, so, there is no utility in having the variable. The warning is to tell the user to remove the dead variable. >> >> I am sort of lost. > > I can try again. Begin your sentence, the important utility of this construct is demonstrated by the following code: > > See if you can complete it. If not, then, then there is no utility. The warning says, there is no utility. This isn't a theoretic thing, it is an engineering thing. Yeah, and as opposed to the non-volatile case removing the volatile set-but-unused variable even reduces code size! Richard.
On Fri, Nov 8, 2013 at 5:39 PM, Mike Stump <mikestump@comcast.net> wrote: > > On Nov 8, 2013, at 1:32 AM, Bin.Cheng <amker.cheng@gmail.com> wrote: > >> On Fri, Nov 8, 2013 at 5:03 PM, Mike Stump <mikestump@comcast.net> wrote: >>> On Nov 7, 2013, at 5:13 PM, Mingjie Xing <mingjie.xing@gmail.com> wrote: >>>> Well, it is my understanding that the warning should be emitted for a >>>> volatile variable only if it is not accessed. Initialization means >>>> accessing, even though it is not used anywhere. >>> >>> Let me try. A warning is useful, if there is no way a conforming program can tell that the variable exists or not. So, the question is, how can you notice the variable? Answer, there is no way, so, there is no utility in having the variable. The warning is to tell the user to remove the dead variable. >> >> I am sort of lost. > > I can try again. Begin your sentence, the important utility of this construct is demonstrated by the following code: > > See if you can complete it. If not, then, then there is no utility. The warning says, there is no utility. This isn't a theoretic thing, it is an engineering thing. Thanks for elaborating. The warning message is actually for no-use of variable, and it has few things to do with whether it's accessed or not. Thanks, bin
Oops, if it is not a bug, please close the report http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57258 Thanks, Mingjie 2013/11/8 Richard Biener <richard.guenther@gmail.com>: > On Fri, Nov 8, 2013 at 10:39 AM, Mike Stump <mikestump@comcast.net> wrote: >> >> On Nov 8, 2013, at 1:32 AM, Bin.Cheng <amker.cheng@gmail.com> wrote: >> >>> On Fri, Nov 8, 2013 at 5:03 PM, Mike Stump <mikestump@comcast.net> wrote: >>>> On Nov 7, 2013, at 5:13 PM, Mingjie Xing <mingjie.xing@gmail.com> wrote: >>>>> Well, it is my understanding that the warning should be emitted for a >>>>> volatile variable only if it is not accessed. Initialization means >>>>> accessing, even though it is not used anywhere. >>>> >>>> Let me try. A warning is useful, if there is no way a conforming program can tell that the variable exists or not. So, the question is, how can you notice the variable? Answer, there is no way, so, there is no utility in having the variable. The warning is to tell the user to remove the dead variable. >>> >>> I am sort of lost. >> >> I can try again. Begin your sentence, the important utility of this construct is demonstrated by the following code: >> >> See if you can complete it. If not, then, then there is no utility. The warning says, there is no utility. This isn't a theoretic thing, it is an engineering thing. > > Yeah, and as opposed to the non-volatile case removing the volatile > set-but-unused variable even reduces code size! > > Richard.
On Nov 8, 2013, at 2:25 AM, Bin.Cheng <amker.cheng@gmail.com> wrote: > Thanks for elaborating. The warning message is actually for no-use of > variable, and it has few things to do with whether it's accessed or > not. I disagree. If you examine why the warning was put in, you realize it was put in so that lazy programmers can find dead things. Why do they want to do that, to remove the dead things. This construct is a dead thing. If you disagree, you'd have to say why the warning was produced.
On Nov 8, 2013, at 2:35 AM, Mingjie Xing <mingjie.xing@gmail.com> wrote: > Oops, if it is not a bug, please close the report > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57258 Well, I've stated my position. I can be swayed by a good argument, if someone has one. I'd give people a chance to weigh in if they can think of one. A global variable with linkage is completely different, as the linker can grab it, dlsym can grab it, and who knows… while a cleaver (Ian type of cleaver) person could grab an automatic variable, the difficulty is enormous and doing that would be exceedingly odd. I don't know of any coding style where people do that sort of thing. Reflection comes the closest; dynamic bindings (a la lisp) I guess comes second; shudder. I'd make them mark it with the used attribute, since they're really weird.
Index: c/c-decl.c =================================================================== --- c/c-decl.c (revision 204285) +++ c/c-decl.c (working copy) @@ -1187,11 +1187,17 @@ pop_scope (void) && scope != external_scope) { if (!TREE_USED (p)) - warning (OPT_Wunused_variable, "unused variable %q+D", p); + { + if (!TREE_THIS_VOLATILE (p) || !DECL_INITIAL (p)) + warning (OPT_Wunused_variable, "unused variable %q+D", p); + } else if (DECL_CONTEXT (p) == current_function_decl) - warning_at (DECL_SOURCE_LOCATION (p), - OPT_Wunused_but_set_variable, - "variable %qD set but not used", p); + { + if (!TREE_THIS_VOLATILE (p)) + warning_at (DECL_SOURCE_LOCATION (p), + OPT_Wunused_but_set_variable, + "variable %qD set but not used", p); + } } if (b->inner_comp)