Bug 23608 - constant propagation (CCP) would improve -Wsign-compare
Summary: constant propagation (CCP) would improve -Wsign-compare
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 4.0.2
: P3 enhancement
Target Milestone: ---
Assignee: Paolo Carlini
URL:
Keywords:
Depends on:
Blocks: 38470
  Show dependency treegraph
 
Reported: 2005-08-29 02:30 UTC by Bruno Martínez
Modified: 2013-05-20 13:22 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2005-12-07 04:29:10


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Bruno Martínez 2005-08-29 02:30:10 UTC
Compiling

#define FIVE 5

int main()
{
    int i = 5;
    int const ic = 5;

    i < 5u;
    ic < 5u;
    FIVE < 5u; // line 10
}


with -Wsign-compare produces the output

pp.cpp:8: warning: comparison between signed and unsigned integer expressions
pp.cpp:9: warning: comparison between signed and unsigned integer expressions


MSVC only warns on line 8.  G++'s current behaviour is unfortunate because it 
favors #defines over consts.

Regards,
Bruno Martínez
Comment 1 Andrew Pinski 2005-08-29 02:34:18 UTC
ic by the C++ standard does not need to propagate its value into ic < 5u so I think the warning is 
warranted.
Comment 2 Bruno Martínez 2005-08-29 22:24:02 UTC
Could you point me to the section in the standard?  I thought warning were a 
quality of implementation matter.
Comment 3 Andrew Pinski 2005-08-29 23:01:01 UTC
Subject: Re:  -Wsign-compare and const propagation


On Aug 29, 2005, at 6:24 PM, br1 at internet dot com dot uy wrote:

>
> ------- Additional Comments From br1 at internet dot com dot uy  
> 2005-08-29 22:24 -------
> Could you point me to the section in the standard?  I thought warning 
> were a
> quality of implementation matter.

No you misunderstood me.  What I was trying to say the const propagation
does not need to happen.  Since that is the case the warning is still 
valid.

-- Pinski

Comment 4 Wolfgang Bangerth 2005-08-29 23:10:36 UTC
What Andrew is trying to say is that in C, even variables that are marked  
'const' can be modified. Thus, the compiler can't (naively, i.e. without  
using flow analysis) determine that 'i' or 'ic' have the value 5, and  
therefore warn that there is a comparison between signed and unsigned.  
However, in the last line, the compiler knows the value that is being   
compared to 5u and sees that nothing bad can happen here. It therefore  
does not warn.  
 
In C++, the situation is different, since const variables can't be changed. 
The compiler could therefore be able to determine that the second comparison 
does not warrant a warning. This notwithstanding, the compiler doesn't do  
this right now. 
 
As annoying as this is, I don't consider this a bug. It may be classified 
as an enhancement, though, if someone wants to make this warning contigent 
upon the availability of forward propagation of values. General consensus 
has been, however, not to do this. 
 
W. 
Comment 5 Bruno Martínez 2005-08-29 23:23:31 UTC
I don't know much about const in C, just C++.  I just wanted to have this 
request entered in your database, as a possible enhancement, of course.

Regards,
Bruno
Comment 6 Gabriel Dos Reis 2005-08-30 00:09:44 UTC
Subject: Re:  -Wsign-compare and const propagation

"bangerth at dealii dot org" <gcc-bugzilla@gcc.gnu.org> writes:

| As annoying as this is, I don't consider this a bug. It may be classified 
| as an enhancement, though, if someone wants to make this warning contigent 
| upon the availability of forward propagation of values. General consensus 
| has been, however, not to do this. 

I agree with Wolfgang's clear analysis.

-- Gaby
Comment 7 Manuel López-Ibáñez 2010-02-24 14:16:27 UTC
A simpler version of PR 38470. An idea could be to do some cheap and quick ccp in the front-end like clang does. The alternative is to move this to the middle-end, which is rather difficult.
Comment 8 Paolo Carlini 2013-05-20 11:08:37 UTC
This seems fixed in 4.8: we warn only for line #8. Manu could you please double check we can close this? (thanks a lot again)
Comment 9 Paolo Carlini 2013-05-20 11:20:33 UTC
(Nit: the location is off by 2 chars, I think I can easily fix this separately: build_new_op_1 isn't propagating loc to cp_build_binary_op)
Comment 10 Paolo Carlini 2013-05-20 13:22:58 UTC
Location fixed for 4.9.0.

If we have reasons to believe the bogus warning is now suppressed only be chance, let's re-open the bug (or move the discussion to 38470?)