www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.bugs - [Issue 10480] New: Warning against wrong usage of incorrect operator for bits set test

reply d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10480

           Summary: Warning against wrong usage of incorrect operator for
                    bits set test
           Product: D
           Version: D2
          Platform: All
        OS/Version: All
            Status: NEW
          Keywords: diagnostic
          Severity: enhancement
          Priority: P2
         Component: DMD
        AssignedTo: nobody puremagic.com
        ReportedBy: bearophile_hugs eml.cc


--- Comment #0 from bearophile_hugs eml.cc 2013-06-26 11:05:24 PDT ---
The D compiler 2.064alpha gives no warnings here:


bool someFunction() { return true; }
uint getFlags() { return uint.max; }
void main() {
    uint kFlagValue = 2u ^^ 14;
    if (someFunction() || getFlags() | kFlagValue) {}
}




enum INPUT_VALUE = 2;
void f(uint flags) {
    if (flags | INPUT_VALUE) {}
}


The warning given by Visual Studio 2012:
http://msdn.microsoft.com/en-us/library/f921xb29.aspx

warning C6316: Incorrect operator:  tested expression is constant and non-zero.
 Use bitwise-and to determine whether bits are set.<


More info:
http://randomascii.wordpress.com/2013/06/24/two-years-and-thousands-of-bugs-of-/

Perhaps it's a good idea to add this warning to D/dmd.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jun 26 2013
next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10480


Walter Bright <bugzilla digitalmars.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bugzilla digitalmars.com


--- Comment #1 from Walter Bright <bugzilla digitalmars.com> 2013-06-26
15:44:25 PDT ---
I'm unhappy with making this an error or a warning. It's perfectly reasonable
to write code that tests something that is (at compile time) always true or
false. This can happen:

1. in generic code

2. in reasonable attempts to avoid versioning

3. D code idiomatically does quite a bit more at compile time than other
languages do. This change would make that more difficult.

4. in temporary debugging code, like:

    if (0 && condition) ...

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jun 26 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10480



--- Comment #2 from bearophile_hugs eml.cc 2013-06-26 17:20:12 PDT ---
(In reply to comment #1)
 I'm unhappy with making this an error or a warning. It's perfectly reasonable
 to write code that tests something that is (at compile time) always true or
 false. This can happen:
 
 1. in generic code
 
 2. in reasonable attempts to avoid versioning
 
 3. D code idiomatically does quite a bit more at compile time than other
 languages do. This change would make that more difficult.
 
 4. in temporary debugging code, like:
 
     if (0 && condition) ...
Thank you for your answer. I have opened this issue because: - I think a bit of static analysis from the compiler goes a long way avoiding common simple programmer mistakes. - I think being aware of a problem is important even when I don't know how to solve it, so having this enhancement request in Bugzilla is important to remember it. - In both the article that has spurred this ER and in the warning in the Visual Studio, some intelligent persons think it's a common bug and a good to have warning for C++. I have now gained some experience on how D is designed and how you want it to be designed (nearly no warnings, avoid false positives as much as possible, help generic code and compile time coding), so I understand such problems. If you want I will close down this issue. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Jun 26 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10480


bearophile_hugs eml.cc changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |WONTFIX


--- Comment #3 from bearophile_hugs eml.cc 2013-06-26 17:23:29 PDT ---
Walter, from:
http://forum.dlang.org/post/kqfr0k$52o$1 digitalmars.com

 We've discussed this one before. I oppose it, on grounds I
 added to the enhancement request.
So let's close this down, wontfix. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Jun 26 2013
prev sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10480


yebblies <yebblies gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
                 CC|                            |yebblies gmail.com
         Resolution|WONTFIX                     |


--- Comment #4 from yebblies <yebblies gmail.com> 2013-06-27 22:39:58 EST ---
I'm not seeing the problem with this here...

I agree with Walter, this should be allowed, it happens all the time.
if (0 && anything)

Same with this:
if (1 || anything)

But this...?
if (1 | anything)

At best, they meant to write ||, and this is a typo.
At worst, it was supposed to be an && and this is a bug.

I think an error here would be very unlikely to be a false positive.

I can't think of a single use for a logical or expression as a condition.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jun 27 2013