Bug 18624 - GCC does not detect local variable set but never used
GCC does not detect local variable set but never used
Status: RESOLVED FIXED
Product: gcc
Classification: Unclassified
Component: c
3.4.3
: P3 enhancement
: 4.6.0
Assigned To: Not yet assigned to anyone
: diagnostic
: 16517 20459 24611 31795 36390 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2004-11-23 08:34 UTC by David Binderman
Modified: 2011-08-24 19:26 UTC (History)
13 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2009-08-04 10:52:25


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description David Binderman 2004-11-23 08:34:16 UTC
Hello there,

For the following source code, g++ 343 appears unable to detect that
local variable b is set but not used.

void g( int );

void f( int a)
{
	int b;

	if (a < 0)
	{
		b = 1;
	}
	else
	{
		b = 2;
	}
	g( a);
}

[dcb@localhost src]$ ~/gnu/gcc343/results/bin/g++ -g -O2 -Wall -W -c unused.cc
[dcb@localhost src]$

Here is Intel C++ 8.1 doing what I want.

unused.cc(6): remark #593: variable "b" was set but never used
        int b;
            ^
Comment 1 Andrew Pinski 2004-11-23 13:06:46 UTC
Yes this would be slightly useful but one has to be care full of what is warned about.
Comment 2 Andrew Pinski 2004-12-06 05:02:30 UTC
Confirmed.
Comment 3 Andrew Pinski 2005-03-07 03:41:06 UTC
This should be done in the front-ends.
Comment 4 Giovanni Bajo 2005-03-07 08:55:06 UTC
(In reply to comment #3)
> This should be done in the front-ends.

Why? How?
Comment 5 Andrew Pinski 2005-03-13 17:31:15 UTC
*** Bug 20459 has been marked as a duplicate of this bug. ***
Comment 6 Kazu Hirata 2005-03-13 17:32:58 UTC
PR 20459 has an experimental patch (with some false positives).
Comment 7 Andrew Pinski 2005-11-01 14:22:15 UTC
*** Bug 24611 has been marked as a duplicate of this bug. ***
Comment 8 Alexander J. Herrmann 2005-11-02 11:29:55 UTC
I guessed somebody found it before but searching the db I couldn'n find it. Anyway shouldn't we make it than dependend or blocking Bug #tree-optimization/21513
As the funktion while C correct will never make it to the compiler using the -Werror switch together with optimization when this Bug is fixed :)
int problem_funktion(int a)
{
   int b;
   if (__builtin_expect(((a > 0) && ((b = 5) != 0)),
1))
   {
      return(a*b);
   }
   return(a);
} 
Comment 9 Andrew Pinski 2007-05-03 16:52:35 UTC
*** Bug 31795 has been marked as a duplicate of this bug. ***
Comment 10 Kaveh Ghazi 2008-06-26 20:52:10 UTC
*** Bug 16517 has been marked as a duplicate of this bug. ***
Comment 11 Bernhard Reutner-Fischer 2008-06-27 13:04:22 UTC
should we

- add a bit so that used_flags:2 and set TREE_USED() = 2 in expand_assignment(), expand_expr_real_1, adjust setting tree addressable(decl)=!!TREE_USED(decl), set it in gimplify_modify_expr_rhs() for vars that are not init-self, expand_one_var() etc.
or
- warn on has_zero_uses() in something like ccp?
Comment 12 David Binderman 2009-01-30 13:48:00 UTC
(In reply to comment #1)
> Yes this would be slightly useful but one has to be care full of what is 
> warned about.

Agreed. For a first cut, a simple straight forward job, without
considering the complex cases, could be the way forward.

For example, all bets would be off for any variable that
has it's address taken.

To get some data on how many times this "set but not used" problem
occurs in real code, I just had a go at compiling almost half the Suse Linux 
distribution source tree with Intel C/C++.

For source codes [a-k]*, there where 906 occurrences of the "set but
not used" warning from Intel C/C++. 

Given that Intel can't compile a lot of the GNU specific code,  
I estimate at least 2,000 and maybe 2,500 occurrences of this
problem in Suse Linux distribution.

Other distributions are different sizes, but at least this
gives us a single data point on how frequently the "set but not
used" problem occurs in practice.

Comment 13 Manuel López-Ibáñez 2009-01-30 16:11:21 UTC
(In reply to comment #12)
> 
> For source codes [a-k]*, there where 906 occurrences of the "set but
> not used" warning from Intel C/C++. 
> 

@dcb

I think nobody is discussing that we would want such warning (in some form or another) in GCC. We do not need more rationale or evidence supporting it. We need someone to write the patch. Just that.
Comment 14 David Binderman 2009-06-11 12:57:06 UTC
(In reply to comment #13)

> We need someone to write the patch. Just that.

I've got some spare time now, so I'd like to have a go.

I can see that used_flag bitfield can be expanded from 1 bit to 2 bits.
I can see that TREE_USED macro can be reworked to match. 

What concerns me is all the different places (currently 170) that the
text pattern "TREE_USED.*=" occurs.

I suspect a manual review of each of those 170 places would be 
prone to failure. Even without considering the various languages
in sub directories, there are still 84 uses of "TREE_USED.*=" left.

Considering solely the test program I originally posted, how
would I go about finding which subset of the 84 places get 
visited by the test program ?

gdb with 84 breakpoints doesn't sound feasible to me.
Comment 15 Manuel López-Ibáñez 2009-08-04 10:52:24 UTC
(In reply to comment #14)
> (In reply to comment #13)
> 
> > We need someone to write the patch. Just that.
> 
> I've got some spare time now, so I'd like to have a go.

If you want your code to be useful, fill a copyright assignment:

http://gcc.gnu.org/contribute.html#legal

> I can see that used_flag bitfield can be expanded from 1 bit to 2 bits.
> I can see that TREE_USED macro can be reworked to match. 

Reworked how?

> What concerns me is all the different places (currently 170) that the
> text pattern "TREE_USED.*=" occurs.
> 
> I suspect a manual review of each of those 170 places would be 
> prone to failure. Even without considering the various languages
> in sub directories, there are still 84 uses of "TREE_USED.*=" left.

Focus on the C front-end: the c-* files, fold* and tree.c

> gdb with 84 breakpoints doesn't sound feasible to me.

I really do not see any alternative. For your test program, you shouldn't hit all breakpoints, so the only problem is setting them. First try with the ones more likely and extend.
 
As I see this, you should just find those places where b is marked as used although it is only assigned. Then perhaps it will suffice to not mark it as used but as "set", perhaps this is done already, so one does not need to mark it as set either. Then in pop_scope in c-decl.c check for this following the existing check for Wunused_variable in order to give the proper warning message.

(BTW, this has nothing to do with Wuninitialized)
Comment 16 Jakub Jelinek 2009-11-24 11:36:44 UTC
*** Bug 36390 has been marked as a duplicate of this bug. ***
Comment 17 Jakub Jelinek 2009-11-24 11:38:28 UTC
http://gcc.gnu.org/ml/gcc-patches/2009-11/msg01338.html
is an example where the warning would be very useful, we create GC garbage uselessly...
Comment 19 Bernhard Reutner-Fischer 2009-11-25 18:15:52 UTC
(In reply to comment #18)
> http://gcc.gnu.org/ml/gcc-patches/2009-11/msg01392.html
> 

Wunused-var-1.c has these two tests:

void
f1 (void)
{
  int a;	/* { dg-warning "set but not used" } */
  int b;
  int c;
  c = 1;
  a = b = c;
}
Neither of them has side-effects nor is actually used, so warnings for all of them should be emitted.

void
f2 (int x)
{
  int a;	/* { dg-warning "set but not used" } */
  int b;
  int c;	/* { dg-warning "set but not used" } */
  c = (a = x, b = x);
}
Same here AFAICS, also the PARM_DECL is unused.
Comment 20 Jakub Jelinek 2009-11-25 18:28:32 UTC
That's on purpose.  b is actually used by assignment to a in both cases, c is used in the first case by assignment to b.  If the user decides to remove the a variable (resp. c in the second case) based on this warning, he'll get further warnings and can tweak it, but not counting references assigned to variables warned about is neither possible in this implementation (you don't know until the end whether something is going to be just set and not used) nor desirable (in some cases you don't want to fix code by removing the set but not used variable, but instead actually use it somewhere, and in that case you could be removing the other set but not used vars even when they are now actually used by some used variable).
Comment 21 David Binderman 2009-11-30 11:30:10 UTC
(In reply to comment #18)
> http://gcc.gnu.org/ml/gcc-patches/2009-11/msg01392.html

I tried out your patch on a recent Linux kernel and got
some possibly false positives

Code like

static inline pud_t __pud(pudval_t val)
{
        pudval_t ret;

        if (sizeof(pudval_t) > sizeof(long))
                ret = PVOP_CALLEE2(pudval_t, pv_mmu_ops.make_pud,
                                   val, (u64)val >> 32);
        else
                ret = PVOP_CALLEE1(pudval_t, pv_mmu_ops.make_pud,
                                   val);

        return (pud_t) { ret };
}

produces

arch/x86/include/asm/paravirt.h:613:11: warning: variable 'ret' set but not used

The return statement looks like unusual C syntax to me.
Some refinement of your patch may be possible.
Comment 22 Jakub Jelinek 2009-11-30 13:56:49 UTC
In that case you weren't using the latest version of the patch.
Please try http://gcc.gnu.org/ml/gcc-patches/2009-11/msg01582.html
instead, it should fix several important bugs and initializers of compound literals are certainly one of those.
Comment 23 Jakub Jelinek 2010-04-07 20:34:08 UTC
Subject: Bug 18624

Author: jakub
Date: Wed Apr  7 20:33:36 2010
New Revision: 158086

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=158086
Log:
	PR c/18624
	* tree.h (DECL_READ_P): Define.
	(struct tree_decl_common): Add decl_read_flag.
	* c-decl.c (pop_scope): If TREE_USED but !DECL_READ_P, issue
	a set but not used warning.
	(merge_decls): Merge DECL_READ_P flag.
	(finish_decl, build_compound_literal): Set DECL_READ_P flag.
	(finish_function): Issue -Wunused-but-set-parameter diagnostics.
	* c-common.c (handle_used_attribute, handle_unused_attribute):
	Likewise.
	* c-tree.h (default_function_array_read_conversion, mark_exp_read):
	New prototypes.
	* c-typeck.c (default_function_array_read_conversion, mark_exp_read):
	New functions.
	(default_conversion, c_process_expr_stmt): Call mark_exp_read.
	* c-parser.c (c_parser_initializer, c_parser_expr_no_commas,
	c_parser_binary_expression, c_parser_cast_expression,
	c_parser_expr_list, c_parser_omp_atomic, c_parser_omp_for_loop):
	Call default_function_array_read_conversion instead of
	default_function_array_conversion where needed.
	(c_parser_unary_expression, c_parser_conditional_expression,
	c_parser_postfix_expression_after_primary, c_parser_initelt):
	Likewise.  Call mark_exp_read where needed.
	(c_parser_statement_after_labels, c_parser_asm_operands,
	c_parser_typeof_specifier, c_parser_sizeof_expression,
	c_parser_alignof_expression, c_parser_initval): Call mark_exp_read
	where needed.
	* common.opt (Wunused-but-set-variable, Wunused-but-set-parameter):
	New.
	* toplev.c (warn_unused_but_set_variable): Default to warn_unused.
	(warn_unused_but_set_parameter): Default to warn_unused
	&& extra_warnings.
	* doc/invoke.texi: Document -Wunused-but-set-variable and
	-Wunused-but-set-parameter.

	* objc-act.c (finish_var_decl, objc_begin_catch_clause,
	really_start_method, get_super_receiver, handle_class_ref): Set
	DECL_READ_P in addition to TREE_USED.

	* gcc.dg/Wunused-var-1.c: New test.
	* gcc.dg/Wunused-var-2.c: New test.
	* gcc.dg/Wunused-var-3.c: New test.
	* gcc.dg/Wunused-var-4.c: New test.
	* gcc.dg/Wunused-var-5.c: New test.
	* gcc.dg/Wunused-var-6.c: New test.
	* gcc.dg/Wunused-parm-1.c: New test.

Added:
    trunk/gcc/testsuite/gcc.dg/Wunused-parm-1.c
    trunk/gcc/testsuite/gcc.dg/Wunused-var-1.c
    trunk/gcc/testsuite/gcc.dg/Wunused-var-2.c
    trunk/gcc/testsuite/gcc.dg/Wunused-var-3.c
    trunk/gcc/testsuite/gcc.dg/Wunused-var-4.c
    trunk/gcc/testsuite/gcc.dg/Wunused-var-5.c
    trunk/gcc/testsuite/gcc.dg/Wunused-var-6.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/c-common.c
    trunk/gcc/c-decl.c
    trunk/gcc/c-parser.c
    trunk/gcc/c-tree.h
    trunk/gcc/c-typeck.c
    trunk/gcc/common.opt
    trunk/gcc/doc/invoke.texi
    trunk/gcc/objc/ChangeLog
    trunk/gcc/objc/objc-act.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/toplev.c
    trunk/gcc/tree.h

Comment 24 Manuel López-Ibáñez 2010-04-15 19:32:04 UTC
@Jakub,

is this FIXED? Could you document this new option in GCC 4.6 changes.html?
Comment 25 Jakub Jelinek 2010-04-16 06:58:15 UTC
C++ still doesn't support this warning, so not yet.
Comment 26 Dodji Seketeli 2010-05-06 06:52:46 UTC
Subject: Bug 18624

Author: dodji
Date: Thu May  6 06:52:30 2010
New Revision: 159096

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=159096
Log:
C++ support for -Wunused-but-set-variable

gcc/cp/ChangeLog:
	PR 18624
	* cp-tree.h (mark_exp_read, mark_rvalue_use, mark_lvalue_use,
	mark_type_use): Declare ...
	* expr.c (mark_exp_read, mark_rvalue_use, mark_lvalue_use,
	mark_type_use): ... new fns.
	* typeck.c (cxx_sizeof_expr, cxx_alignof_expr): Call mark_type_use.
	(perform_integral_promotions): Call mark_rvalue_use.
	(cp_build_unary_op): Call mark_lvalue_use.
	(decay_conversion): Update comment. Call mark_lvalue.
	* decl.c (unused_but_set_errorcount): New variable.
	(poplevel): Issue -Wunused-but-set-variable diagnostics.
	(duplicate_decls): Merge DECL_READ_P flags.
	(start_cleanup_fn): Set DECL_READ_P flag.
	(finish_function): Issue -Wunused-but-set-parameter diagnostics.
	* tree.c (rvalue): Call mark_rvalue_use.
	* pt.c (convert_nontype_argument): Likewise.
	* semantics.c (finish_typeof, finish_decltype_type): Call
	mark_type_use.
	(finish_asm_stmt): Call mark_lvalue_use.
	(finish_expr_stmt): Call mark_exp_read.
	* call.c (convert_like_real) <ck_identity, ck_user>: Call
	mark_rvalue_use.
	(build_x_va_arg): Call mark_lvalue_use.
	(build_over_call): Call mark_type_use.
	* init.c (build_new, build_delete): Call mark_value_use.
	* rtti.c (build_typeid): Call mark_lvalue_use or mark_type_use.
	(build_dynamic_cast_1): call mark_lvalue_use or mark_rvalue_use.

gcc/testsuite/ChangeLog:
	PR 18624
	* g++.dg/warn/Wunused-7.C: Add dg-warning.
	* g++.dg/template/sfinae16.C: Likewise.
	* gcc.dg/Wunused-var-1.c: Moved to...
	* c-c++-common/Wunused-var-1.c: ...here. New test.
	* gcc.dg/Wunused-var-2.c: Moved to...
	* c-c++-common/Wunused-var-2.c: ...here. New test.
	* gcc.dg/Wunused-var-3.c: Moved to...
	* c-c++-common/Wunused-var-3.c: ...here. New test.
	* gcc.dg/Wunused-var-4.c: Moved to...
	* gcc.dg/Wunused-var-1.c: ... here.
	* gcc.dg/Wunused-var-5.c: Moved to...
	* c-c++-common/Wunused-var-4.c: ...here. New test.
	* gcc.dg/Wunused-var-7.c: Moved to...
	* c-c++-common/Wunused-var-5.c: ...here. New test.
	* gcc.dg/Wunused-var-6.c: Moved to...
	* gcc.dg/Wunused-var-2.c: ... here.
	* c-c++-common/Wunused-var-1.c: New test.
	* c-c++-common/Wunused-var-2.c: New test.
	* c-c++-common/Wunused-var-3.c: New test.
	* c-c++-common/Wunused-var-4.c: New test.
	* c-c++-common/Wunused-var-5.c: New test.
	* g++.dg/warn/Wunused-var-1.C: New test.
	* g++.dg/warn/Wunused-var-2.C: New test.
	* g++.dg/warn/Wunused-var-3.C: New test.
	* g++.dg/warn/Wunused-var-4.C: New test.
	* g++.dg/warn/Wunused-var-5.C: New test.
	* g++.dg/warn/Wunused-var-6.C: New test.
	* g++.dg/warn/Wunused-var-7.C: New test.
	* g++.dg/warn/Wunused-var-8.C: New test.
	* g++.dg/warn/Wunused-parm-1.C: New test.
	* g++.dg/warn/Wunused-parm-2.C: New test.
	* g++.dg/warn/Wunused-parm-3.C: New test.

Added:
    trunk/gcc/testsuite/c-c++-common/Wunused-var-1.c
      - copied, changed from r159095, trunk/gcc/testsuite/gcc.dg/Wunused-var-1.c
    trunk/gcc/testsuite/c-c++-common/Wunused-var-2.c
      - copied, changed from r159095, trunk/gcc/testsuite/gcc.dg/Wunused-var-2.c
    trunk/gcc/testsuite/c-c++-common/Wunused-var-3.c
      - copied, changed from r159095, trunk/gcc/testsuite/gcc.dg/Wunused-var-3.c
    trunk/gcc/testsuite/c-c++-common/Wunused-var-4.c
      - copied, changed from r159095, trunk/gcc/testsuite/gcc.dg/Wunused-var-7.c
    trunk/gcc/testsuite/c-c++-common/Wunused-var-5.c
      - copied, changed from r159095, trunk/gcc/testsuite/gcc.dg/Wunused-var-5.c
    trunk/gcc/testsuite/g++.dg/warn/Wunused-parm-1.C
    trunk/gcc/testsuite/g++.dg/warn/Wunused-parm-2.C
    trunk/gcc/testsuite/g++.dg/warn/Wunused-parm-3.C
    trunk/gcc/testsuite/g++.dg/warn/Wunused-var-1.C
    trunk/gcc/testsuite/g++.dg/warn/Wunused-var-2.C
    trunk/gcc/testsuite/g++.dg/warn/Wunused-var-3.C
    trunk/gcc/testsuite/g++.dg/warn/Wunused-var-4.C
    trunk/gcc/testsuite/g++.dg/warn/Wunused-var-5.C
      - copied, changed from r159095, trunk/gcc/testsuite/gcc.dg/Wunused-var-1.c
    trunk/gcc/testsuite/g++.dg/warn/Wunused-var-6.C
      - copied, changed from r159095, trunk/gcc/testsuite/gcc.dg/Wunused-var-2.c
    trunk/gcc/testsuite/g++.dg/warn/Wunused-var-7.C
      - copied, changed from r159095, trunk/gcc/testsuite/gcc.dg/Wunused-var-3.c
    trunk/gcc/testsuite/g++.dg/warn/Wunused-var-8.C
      - copied, changed from r159095, trunk/gcc/testsuite/gcc.dg/Wunused-var-5.c
Removed:
    trunk/gcc/testsuite/gcc.dg/Wunused-var-3.c
    trunk/gcc/testsuite/gcc.dg/Wunused-var-4.c
    trunk/gcc/testsuite/gcc.dg/Wunused-var-5.c
    trunk/gcc/testsuite/gcc.dg/Wunused-var-6.c
    trunk/gcc/testsuite/gcc.dg/Wunused-var-7.c
Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/call.c
    trunk/gcc/cp/cp-tree.h
    trunk/gcc/cp/decl.c
    trunk/gcc/cp/except.c
    trunk/gcc/cp/expr.c
    trunk/gcc/cp/init.c
    trunk/gcc/cp/pt.c
    trunk/gcc/cp/rtti.c
    trunk/gcc/cp/semantics.c
    trunk/gcc/cp/tree.c
    trunk/gcc/cp/typeck.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/g++.dg/template/sfinae16.C
    trunk/gcc/testsuite/g++.dg/warn/Wunused-7.C
    trunk/gcc/testsuite/gcc.dg/Wunused-var-1.c
    trunk/gcc/testsuite/gcc.dg/Wunused-var-2.c

Comment 27 Bernhard Reutner-Fischer 2011-08-24 17:14:19 UTC
FIXED? Seems that this is supported in c++ nowadays, thanks to dodji.
Comment 28 Jakub Jelinek 2011-08-24 17:21:21 UTC
Fixed.
Comment 29 alexander.herrmann 2011-08-24 19:21:47 UTC
Dear Jakub,
 I just did try the example code with gcc 4.5.2.
It plain c not c++. Some oldfashioned people still use c ;)
Regards,
Alexander J. Herrmann

2011/8/25 jakub at gcc dot gnu.org <gcc-bugzilla@gcc.gnu.org>:
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=18624
>
> Jakub Jelinek <jakub at gcc dot gnu.org> changed:
>
>           What    |Removed                     |Added
> ----------------------------------------------------------------------------
>             Status|NEW                         |RESOLVED
>                 CC|                            |jakub at gcc dot gnu.org
>         Resolution|                            |FIXED
>
> --- Comment #28 from Jakub Jelinek <jakub at gcc dot gnu.org> 2011-08-24 17:21:21 UTC ---
> Fixed.
>
> --
> Configure bugmail: http://gcc.gnu.org/bugzilla/userprefs.cgi?tab=email
> ------- You are receiving this mail because: -------
> You are on the CC list for the bug.
>
Comment 30 Andrew Pinski 2011-08-24 19:26:17 UTC
(In reply to comment #29)
> Dear Jakub,
>  I just did try the example code with gcc 4.5.2.
> It plain c not c++. Some oldfashioned people still use c ;)

It was added for 4.6.0.