Bug 62182 - New warning wished: operator== and "equality comparison result unused [-Wunused-comparison]"/-Wunsed-value
Summary: New warning wished: operator== and "equality comparison result unused [-Wunus...
Status: RESOLVED DUPLICATE of bug 53598
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 5.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic
Depends on:
Blocks:
 
Reported: 2014-08-19 09:40 UTC by Tobias Burnus
Modified: 2017-07-20 20:10 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2015-02-28 00:00:00


Attachments
unused-comparison warning (992 bytes, patch)
2015-04-15 22:40 UTC, Arnaud Bienner
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tobias Burnus 2014-08-19 09:40:08 UTC
For a simple:
  int i;
  i == 5;

GCC diagnoses (but warning is enabled not by default):
  warning: statement with no effect [-Wunused-value]
   i == 7;

CLANG does the same (warning enabled by default):
  warning: equality comparison result unused [-Wunused-comparison]
  i == 7;
  ~~^~~~
  note: use '=' to turn this equality comparison into an assignment
  i == 7;
    ^~
    =


However, in our code C++ code, we used an operator==. In that case, GCC doesn't warn while Clang does (by default):

foo.cc:6:7: warning: equality comparison result unused [-Wunused-comparison]
  str == "bar";
  ~~~~^~~~~~~~
foo.cc:6:7: note: use '=' to turn this equality comparison into an assignment
  str == "bar";
      ^~
      =


C++ test case:

#include <stdio.h>
#include <string>

int main() {
  std::string str("init");
  str == "bar";
  printf("%s\n", str.c_str());
  return 0;
}
Comment 1 Manuel López-Ibáñez 2015-02-28 11:59:43 UTC
I'm going to confirm this because we obviously want it.
Comment 2 Arnaud Bienner 2015-04-13 16:38:35 UTC
Related bug: bug 53598.
Comment 3 Arnaud Bienner 2015-04-15 22:40:22 UTC
Created attachment 35324 [details]
unused-comparison warning

I also believe it can be useful to have "unused comparison" warning (i.e. something more specific than current "unused value" warning, because they are likely to be typo.
Having a dedicated warning will allow people who want to activate this warning specifically and/or to turn it into an error.

So I started to have a look at this and I would like to have some feedback from someone more experienced.
It's my first patch to gcc, so it is probably not perfect.

One thing that doesn't work is turning on this warning using -Wunused-comparison parameter. But surprisingly, turning it off with -Wno-unused-comparison (when -Wunused or -Wall is used) works. Not sure what I'm missing here.

The patch would just be a first step: the next step would be to also raise this warning in the case of a "==" operator overloading in C++ (which seems to be the case that doesn't raise a warning currently). Not sure yet how to do this.
Comment 4 Manuel López-Ibáñez 2015-04-16 00:43:17 UTC
(In reply to Arnaud Bienner from comment #3)
> Created attachment 35324 [details]
> unused-comparison warning

You need testcases, and to run the testsuite. See point 4 at:
https://gcc.gnu.org/wiki/GettingStarted#Basics:_Contributing_to_GCC_in_10_easy_steps

> One thing that doesn't work is turning on this warning using
> -Wunused-comparison parameter. But surprisingly, turning it off with
> -Wno-unused-comparison (when -Wunused or -Wall is used) works. Not sure what
> I'm missing here.

That is very weird. I don't see anything wrong in your patch about this.

Nonetheless, please add the warning to c-family/c.opt not to common.opt since it is a C/C++ warning.

Also watch out for the formatting (too long lines, incorrect indentation, etc.) See point 6 in the link above.

> The patch would just be a first step: the next step would be to also raise
> this warning in the case of a "==" operator overloading in C++ (which seems
> to be the case that doesn't raise a warning currently). Not sure yet how to
> do this.

It is more than ok to do one patch per step. Try to get the first patch right and committed, then worry about the next.
Comment 5 Marek Polacek 2015-04-16 06:05:14 UTC
(In reply to Arnaud Bienner from comment #3)
> One thing that doesn't work is turning on this warning using
> -Wunused-comparison parameter. But surprisingly, turning it off with
> -Wno-unused-comparison (when -Wunused or -Wall is used) works. Not sure what
> I'm missing here.

Because the emit_side_effect_warnings calls are guarded by warn_unused_value.
Comment 6 Eric Gallager 2016-12-07 16:38:38 UTC
I think this is a duplicate of bug 53598
Comment 7 Eric Gallager 2017-07-20 20:10:02 UTC
(In reply to Eric Gallager from comment #6)
> I think this is a duplicate of bug 53598

Closing as a duplicate of it

*** This bug has been marked as a duplicate of bug 53598 ***