Print

Please login or register.
1 Hour 1 Day 1 Week 1 Month Forever
Login with username, password and session length

[Kay12]

Given enough eyeballs, all bugs are shallow.
Linus Torvalds

I'm referring to the infamous bug that sometimes causes women's rights so plummet to Arch-Conservative. Once I noticed it always occurs after the news story about the clinic murder, the problem was hunted down with relative ease.

In majorevent.cpp, line 63

Code: [Select]

if(law[LAW_WOMEN]=-2)should be

Code: [Select]

if(law[LAW_WOMEN]==-2)and the conservative bug breathes heavily, coughing up blood... then is quiet.

[praguepride]

I have no idea what you're saying before and after the code segment, but yeah, I understand what you're talking about with the code.

For those who don't, the first line is saying that Women Law will be set to -2 (arch-conservative) instead of asking IF Women Law is -2 (arch-conservative).

I don't know if the first line is actually resetting the variable, but it will always be true because that statement can never be false.

[Ari Rahikkala]

Hm, this sort of bug has come up before and caused some annoyance. Figured it's worth the trouble to try and make sure it won't come up again, so I just committed a revision that cleans up all warnings found by g++ -Wparentheses (plus one unrelated warning caused by g++ being concerned about for loop scoping rules). I don't know if there's an equivalent flag in MSVC, but at least it'll make it easy for me to hunt these guys down later on...

[Kay12]

As far as I know, it does reset the variable and return true always. After adding another equals sign, women's rights should no longer jump to C+ upon the Clinic murder event.

[Innominate]

See now this is what implicit cast warnings are for. Assignment returns a LHS-reference, so "law[LAW_WOMEN]=-2" will return a reference to a variable with value -2. The if statement forces a cast to a bool (X != 0) and then evaluates true every time. This is why you should always compile in the strictest mode possible. gcc -Wconversion should, I think, pick up this error, requiring an explicit (bool) before compiling. In MSVC it will give a warning when you assign in a conditional expression (only when set to the highest warning level however).

Typos like this can bring down entire projects; good catch!

[praguepride]

If you think strict casting is a good thing, you haven't coded large projects before.

Casting on the fly is a godsend for most programs

[Innominate]

If you think strict casting is a good thing, you haven't coded large projects before.

Casting on the fly is a godsend for most programs

I definitely can't speak from experience (the most I've ever coded in one place was a 2D physics engine that I never even finished), but I would have thought that being able to catch bugs that arise because loss of data when implicitly casting before you even compile would be worth having to explicitly specify casts between certain types.

I'm not knocking implicit casting in general; casting between different int types, or from float to double, for example, is almost always harmless. But implicitly casting to bool is frequently the result of an error, and in any case is more easily understood - if you really mean "if(X = Y)" rather than "if(X == Y)", it's much less likely another programmer will fix your 'mistake' if you write "if( (X = Y) != 0)" or "if(bool(X = Y))". Implicit casting is great when you don't really care about the data types, or when you want to make a section of code more readable, but it can readily make a mess of things if you don't intend to use it.

Don't even get me started on implicit casting when typedefs and #defines come into the picture. It's all too easy to break code that relies on implicit casts when you change the typedef. Since implicit casts have no speed benefit (the compiler rewrites them as explicit before linking, IIRC), I avoid them like the plague. But more experienced coders might have good reasons to disagree.

[Ari Rahikkala]

gcc -Wconversion should, I think, pick up this error, requiring an explicit (bool) before compiling.

Aaaaactually it won't, if and buddies are perfectly happy getting an argument of type int and interpreting it by Iverson's convention, and -Wconversion won't warn about that (in fact it seems that most that it warns about in the current LCS codebase is integer widening conversions). 'tis -Wparentheses that will suggest you to put parens around an assignment used as a truth value, which makes little sense to me but does at least tell you when you've used an assignment as a truth value.

[praguepride]

I remember a java project I was working on and it was a complete pain to convert from integer to decimal. You'd think "oh, that should be easy" but it was through unix and apparently that doesn't allow you to easy cast variables, and it didn't even have functions to convert from one to another. I ended up having to throw it into a string and then back into a decimal.

Stupid, I know. People will be like "Hey...that shouldn't be right..."

Trust me, I know...I still don't understand it but the error was clean as day

"program was expecting a decimal and got an integer value instead"

*facepalm*