Bug 44677 - Warn for variables incremented but not used (+=, ++)
Summary: Warn for variables incremented but not used (+=, ++)
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: c (show other bugs)
Version: 4.5.0
: P3 enhancement
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic
: 69723 70092 78964 100184 102909 108156 (view as bug list)
Depends on:
Blocks: Wunused
  Show dependency treegraph
 
Reported: 2010-06-26 00:53 UTC by Joseph S. Myers
Modified: 2023-07-18 22:59 UTC (History)
15 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2020-05-19 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph S. Myers 2010-06-26 00:53:13 UTC
void
f (void)
{
  int i;
  i = 0;
  i++;
}

seems like something a natural generalization of -Wunused-but-set-variable
should warn for; the value of i is only used as part of an increment of i,
so the definition and all uses of this variable could be safely removed.

For a real example of this, see the variable lang_n_infiles in
gcc.c:process_command (I'm testing a patch that removes that variable
along with other changes).
Comment 1 Andrew Pinski 2010-06-28 00:34:58 UTC
Confirmed.
Comment 2 Manuel López-Ibáñez 2016-02-09 01:17:38 UTC
*** Bug 69723 has been marked as a duplicate of this bug. ***
Comment 3 Manuel López-Ibáñez 2016-03-05 22:41:25 UTC
*** Bug 70092 has been marked as a duplicate of this bug. ***
Comment 4 Martin Sebor 2017-01-04 01:31:43 UTC
*** Bug 78964 has been marked as a duplicate of this bug. ***
Comment 5 Jakub Jelinek 2017-01-04 08:04:48 UTC
As soon as consider more complicated testcases than just pre/post increment, this is going to be very hard to define and especially implement.

The way the warning works right now is that all rvalue uses of a variable are marked as "read" and the warning then warns about variables that don't have the "read" bit set, but have the "used" bit set (i.e. they aren't fully unused).
This is especially because the FEs used to fold expressions aggressively very early (and still do, especially the C FE), so not all expressions (even use in sizeof etc. counts) survive long enough where we actually would know what the lhs is corresponding to rhs.  Also, we do not warn if there are any side-effects in between the rvalue use of the var and the store to it.  So, if we do want to warn about m = m + 1; we still shouldn't warn for m = foo (m) + 1; or m = (n = m) + 1; etc.  Handling the pre/post increment by 1 might be easiest, just remember whether the var is not "read" before processing it and reset the "read" bit if so afterwards, but as soon as you run into more complex expressions that is going to be harder and harder.
Comment 6 Martin Sebor 2017-01-04 18:54:18 UTC
I haven't thought through the implementation challenges but defining the extended -Wunused-but-set-variabl rule that's being suggested here seems straightforward: every write to an object must be followed by another access to it (either read or write).  If not, it's diagnosed.

This seems analogous to the -Wuninitialized checker/rule that might be stated as: the first read of an object must be preceded by a write to it.
Comment 7 Jakub Jelinek 2017-01-04 19:00:56 UTC
(In reply to Martin Sebor from comment #6)
> I haven't thought through the implementation challenges but defining the
> extended -Wunused-but-set-variabl rule that's being suggested here seems
> straightforward: every write to an object must be followed by another access
> to it (either read or write).  If not, it's diagnosed.

That is not straightforward at all.  The FE doesn't have IL on which it can analyze write accesses being followed by something (no cfg, no SSA form), and in the middle end it is way too late for such a warning (because as soon as you optimize away something, you could optimize away those reads and warning just because the optimizers managed to optimize away some use are not really helpful).
Comment 8 Martin Sebor 2019-09-29 18:54:35 UTC
I would expect handling -Wunused-but-set-variable during Gimplification to make detecting these sorts of things possible at least in the basic cases.
Comment 9 Jakub Jelinek 2019-09-29 18:58:31 UTC
(In reply to Martin Sebor from comment #8)
> I would expect handling -Wunused-but-set-variable during Gimplification to
> make detecting these sorts of things possible at least in the basic cases.

That is way too late.
Comment 10 Martin Sebor 2020-05-19 17:23:03 UTC
See pr95217 for other cases to handle (-Wunused-but-set-parameter).  For the test case there, Clang's static analyzer diagnoses two out of the four cases:

$ cat pr95217.c && clang -S -Wall -Wextra --analyze pr95217.c
void f0 (int *p)
{
  p = 0;           // -Wunused-but-set-parameter (expected)
}

void f1 (int *p)
{
  p += 1;          // missing warning
}

void f2 (int *p)
{
  p = p + 1;       // missing warning
}

void f3 (int *p)
{
  ++p;             // missing warning
}

pr95217.c:8:3: warning: Value stored to 'p' is never read
  p += 1;          // missing warning
  ^    ~
pr95217.c:13:3: warning: Value stored to 'p' is never read
  p = p + 1;       // missing warning
  ^   ~~~~~
2 warnings generated.
Comment 11 Joseph S. Myers 2021-04-21 17:41:24 UTC
*** Bug 100184 has been marked as a duplicate of this bug. ***
Comment 12 Andrew Pinski 2021-10-24 06:09:57 UTC
*** Bug 102909 has been marked as a duplicate of this bug. ***
Comment 13 Paul Eggert 2022-04-09 00:48:33 UTC
I ran into this issue today on Emacs master, with both += and |=. Clang correctly diagnosed unused local variables, but GCC did not.

It would be nice if GCC could do at least as well as Clang does.

Here are references to patches I installed into Emacs to fix these issues that Clang found but GCC did not:

https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=d9bffa1f3b121085fd8f954eb9446a4a5241c062
https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=68bc1446855c86b96d5bc22f819e63358ab250ac
Comment 14 David C. Manuelda 2022-09-19 10:03:22 UTC
It is worth to notice that this bug propagates to C++ whenever an object uses an int that increments but is never used, like the following example:

class Test{
        public:
                Test() { i = 0; i++; }
        private:
                int i;
};

int main(int, char**) {
        Test unused;

        return 0;
}
Comment 15 Andrew Pinski 2022-12-17 17:58:15 UTC
*** Bug 108156 has been marked as a duplicate of this bug. ***
Comment 16 Andrew Pinski 2022-12-17 18:04:23 UTC
"set but not used" (just for future searches)
Comment 17 Vincent Lefèvre 2023-07-18 22:59:45 UTC
(In reply to Martin Sebor from comment #10)
> $ cat pr95217.c && clang -S -Wall -Wextra --analyze pr95217.c
[...]
> pr95217.c:8:3: warning: Value stored to 'p' is never read
>   p += 1;          // missing warning
>   ^    ~
> pr95217.c:13:3: warning: Value stored to 'p' is never read
>   p = p + 1;       // missing warning
>   ^   ~~~~~
> 2 warnings generated.

Clang (15 and above) with -Wunused-but-set-variable now detects the issue on the "p++" and "p += 1" forms (ditto with other combined assignment operators), but not on "p = p + 1".

Such forms (p++, etc.) are common, so that detecting an unused variable is very useful.

Like Paul did for Emacs (comment 13), I've just fixed two issues in GNU MPFR (one cosmetic about a useless loop variable and one important in the testsuite), found with Clang 16. The references:
https://gitlab.inria.fr/mpfr/mpfr/-/commit/4c110cf4773b3c07de2a33acbee591cedb083c80
https://gitlab.inria.fr/mpfr/mpfr/-/commit/b34d867fa41934d12d0d4dbaaa0242d6d3eb32c7

For the second MPFR issue, there was an "err++" for each error found by the function in the testsuite, but err was not tested at the end, so that potential errors were never reported.