Bug 61534 - Wlogical-op should not warn when either operand comes from macro expansion
Summary: Wlogical-op should not warn when either operand comes from macro expansion
Status: NEW
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
: 53131 (view as bug list)
Depends on: 43486
Blocks: 69602 Wall-Wextra 63357
  Show dependency treegraph
 
Reported: 2014-06-17 10:25 UTC by Manuel López-Ibáñez
Modified: 2024-01-20 17:01 UTC (History)
6 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2014-06-25 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Manuel López-Ibáñez 2014-06-17 10:25:10 UTC
#define DEFINED (x != 0)

int foo(int x)
{
  if (!DEFINED && x != 0) return 0;
  return 1;
}

manuel@gcc10:~$ ~/test1/210581/build/gcc/cc1 -Wlogical-op test.c
 foo
test.c: In function ‘foo’:
test.c:5:16: warning: logical ‘and’ of mutually exclusive tests is always false [-Wlogical-op]
   if (!DEFINED && x != 0) return 0;
                ^

Clang does not warn. This was the reason -Wlogical-op was moved out of -Wextra (PR40172).
Comment 1 Marek Polacek 2014-06-25 17:53:35 UTC
This should be easy with track macro expansion on, we'd just have

--- a/gcc/input.h
+++ b/gcc/input.h
@@ -60,6 +60,8 @@ extern location_t input_location;
 
 #define in_system_header_at(LOC) \
   ((linemap_location_in_system_header_p (line_table, LOC)))
+#define from_macro_expansion_at(LOC) \
+  ((linemap_location_from_macro_expansion_p (line_table, LOC)))
 
 void dump_line_table_statistics (void);
 
and then we could use from_macro_expansion_at and don't warn if it's true.  But the problem is with -ftrack-macro-expansion=0, since from_macro_expansion_at wouldn't work :(.
Comment 2 Manuel López-Ibáñez 2014-06-25 18:09:09 UTC
(In reply to Marek Polacek from comment #1)
> and then we could use from_macro_expansion_at and don't warn if it's true. 
> But the problem is with -ftrack-macro-expansion=0, since
> from_macro_expansion_at wouldn't work :(.

I think warning in that case (returning false in the macro) is fair. One cannot expect to turn off features and get the same quality. Is ftrack-macro-expansion=0 used when bootstrapping gcc?
Comment 3 Marek Polacek 2014-06-25 18:21:51 UTC
I don't think so, -ftrack-macro-expansion=2 is on by default and I don't see -ftrack-macro-expansion=0 anywhere in the log of bootstrap.  So maybe my approach would be viable after all (and I could fix PR61081 the same way then).
Comment 4 Marek Polacek 2014-06-25 19:44:05 UTC
BTW, clang doesn't warn even when neither operand comes from a macro expansion; and clang version 3.4 doesn't know -Wlogical-op warning option (so I tried -Wall -Wextra -Weverything, but still no warning).  Yes, I don't have mainline clang, but I don't see that option in clang code base.
Comment 5 Manuel López-Ibáñez 2014-11-02 00:57:29 UTC
If we take gcc/testsuite/gcc.dg/pr40172-3.c (which is XFAIL)

extern int xxx;
#define XXX xxx
int test (void)
{
  if (!XXX && xxx)
    return 4;
  else
    return 0;
}


The big hurdle is that !XXX becomes XXX == 0, but it has the location of "!", which is not virtual. If we look at the argument of the expression, then XXX is actually a var_decl, whose location corresponds to the declaration and not the use, and it is not virtual either. This is PR43486.
Comment 6 Marek Polacek 2015-04-23 09:49:36 UTC
*** Bug 53131 has been marked as a duplicate of this bug. ***
Comment 7 Marek Polacek 2015-04-24 11:50:24 UTC
Author: mpolacek
Date: Fri Apr 24 11:49:52 2015
New Revision: 222406

URL: https://gcc.gnu.org/viewcvs?rev=222406&root=gcc&view=rev
Log:
	PR c/61534
	* input.h (from_macro_expansion_at): Define.

	* c-common.c (warn_logical_operator): Bail if either operand comes
	from a macro expansion.

	* c-c++-common/pr61534-1.c: New test.

Added:
    trunk/gcc/testsuite/c-c++-common/pr61534-1.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/c-family/ChangeLog
    trunk/gcc/c-family/c-common.c
    trunk/gcc/input.h
    trunk/gcc/testsuite/ChangeLog
Comment 8 Manuel López-Ibáñez 2015-11-19 19:54:44 UTC
The last patch did not fix the original testcase nor gcc/testsuite/gcc.dg/pr40172-3.c
Comment 9 Marek Polacek 2015-11-19 20:03:06 UTC
So that's why this PR is still open.
Comment 10 Manuel López-Ibáñez 2015-11-19 20:04:22 UTC
(In reply to Marek Polacek from comment #9)
> So that's why this PR is still open.

Sure, sorry, I should have been clearer. It was mostly a note to myself so I do not need to re-check next time I look at this PR.
Comment 11 Marek Polacek 2015-11-19 20:06:09 UTC
Np.  It's certainly something I'd love to see fixed :/.  Hopefully the next stage1.
Comment 12 Manuel López-Ibáñez 2018-10-19 20:40:33 UTC
This prevent enabling -Wlogical-op with -Wall or -Wextra.
Comment 13 Eric Gallager 2019-07-19 04:34:41 UTC
(In reply to Marek Polacek from comment #11)
> Np.  It's certainly something I'd love to see fixed :/.  Hopefully the next
> stage1.

It's a new stage1