Bug 10480 – Warning against wrong usage of incorrect operator for bits set test
Status
RESOLVED
Resolution
WONTFIX
Severity
enhancement
Priority
P2
Component
dmd
Product
D
Version
D2
Platform
All
OS
All
Creation time
2013-06-26T11:05:24Z
Last change time
2018-01-08T13:40:01Z
Keywords
diagnostic
Assigned to
No Owner
Creator
bearophile_hugs
Comments
Comment #0 by bearophile_hugs — 2013-06-26T11:05:24Z
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.
Comment #1 by bugzilla — 2013-06-26T15:44:25Z
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) ...
Comment #2 by bearophile_hugs — 2013-06-26T17:20:12Z
(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.
Comment #3 by bearophile_hugs — 2013-06-26T17:23:29Z
Walter, from:
http://forum.dlang.org/post/[email protected]
> We've discussed this one before. I oppose it, on grounds I
> added to the enhancement request.
So let's close this down, wontfix.
Comment #4 by yebblies — 2013-06-27T05:39:58Z
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.