Not to be compared to a number



  • So this one is of my own making. We had an error in another part of the system and I was wondering why the API allowed adding parts that were not available for the widget. Somehow any part key would be accepted despite this line:

    if (!widget.availableParts.indexOf(part) == -1) throw new Error("Part "+part+" not available");
    

    Variable part contains a string key, and widget.availableParts is a list of string keys. Should be simple, right? I was staring at this line for a minute and could not explain the behavior. Only when I started toying with it I discovered the mistake. Maybe I should get some more sleep.

    The diff is illuminating:

    - if (!widget.availableParts[part]) throw new Error("Part "+part+" not available");
    + if (!widget.availableParts.indexOf(part) == -1) throw new Error("Part "+part+" not available");
    

    So the interface changed from a dictionary to a straight list of keys. When I updated the code I forgot to remove the bang 😦 A few choice test cases would have caught this error when I made it over a year ago. One can dream, no?


  • I survived the hour long Uno hand

    And that's why--

    @gleemonk said:

    A few choice test cases would have caught this error

    Damn, :hanzo:'d.



  • What is the precedence here? Does the ! apply to the int or the bool expression? Because if it's the bool then it should have thrown an Error for any part that's in the list, no?



  • The use of both ! and == without parenthesis immediately stood out to me as being wrong. You definitely need sleep.



  • @Yamikuronue said:

    And that's why--

    @gleemonk said:

    A few choice test cases would have caught this error

    Damn, :hanzo:'d.

    Oh please do go on. I'll forward the complaints to the one responsible for this lack of tests.
    Well, that would be me.


  • I survived the hour long Uno hand

    Oh ,I was just going to say that's why you write extensive, 100% coverage unit tests, even when you think they're stupid, even when the intent of the code should be obvious...



  • Thank you! I hope I get this through to the developer 😀



  • @marczellm said:

    What is the precedence here? Does the ! apply to the int or the bool expression? Because if it's the bool then it should have thrown an Error for any part that's in the list, no?

    Negation comes first. It turns the returned index into a boolean which will be unequal to -1, so the error never gets thrown. That was the weird thing, because (I reasoned) if I'd gotten the test wrong, it should've rejected the good ones.

    @LB_ said:

    The use of both ! and == without parenthesis immediately stood out to me as being wrong. You definitely need sleep.

    That bang just didn't register with me 😞


  • Banned

    @marczellm said:

    What is the precedence here?

    Unary operators typically have higher priority than any binary operator.

    TRWTF is that $LANGUAGE allows for logical negation of int and equality comparison of bool and int.



  • @Gaska said:

    TRWTF is that $LANGUAGE allows for logical negation of int and equality comparison of bool and int.

    So C got it wrong 😃



  • @Gaska said:

    TRWTF is that $LANGUAGE allows for logical negation of int and equality comparison of bool and int.

    That, and the fact that $LANGUAGE has data structures that are routinely used as sets but don't have a membership-test method.



  • Yes. The problem is that C's "type system" is not rich enough to discriminate between booleans and integers, or, indeed, any other value type. "Using values as booleans" should be a high-level warning. The same goes for C++.

    A boolean is not a number.


  • Banned

    @gleemonk said:

    So C got it wrong 😃

    Yes.

    @Planar said:

    That, and the fact that $LANGUAGE has data structures that are routinely used as sets but don't have a membership-test method.

    Data structures are part of a library, not a language per se (unless it's some built-in thing like associative arrays in most dynamic languages). Although if it's in stdlib, then yes, you can blame $LANGUAGE.

    @tufty said:

    The problem is that C's "type system" is not rich enough to discriminate between booleans and integers, or, indeed, any other value type.

    Ancient C - yes. But from C99 onwards, there's actual boolean type in the language.



  • @Gaska said:

    Ancient C - yes. But from C99 onwards, there's actual boolean type in the language.

    Does this fail to compile with type errors? Fail at runtime with type errors?
    [code]#include <stdbool.h>
    int main (int argc, char ** argv) {
    // Start off sensibly
    bool a = true;
    // This is neither true nor false.
    bool b = 42;
    // Here we use a signed integer as a boolean
    if (argc) {
    // Here we do the converse
    return a;
    } else {
    // And here we take a nearly-a-boolean and add an integer to it.
    return b + 10;
    }
    }[/code]
    Nope, or at least not under clang, gcc, or any of the other c99-capable compilers I've run it through. It quite simply compiles and runs without a warning, treating int as bool and vice versa. Why is that? Well, here's a hint from the C99 spec (my emphasis)

    The type _Bool and the unsigned integer types that correspond to the standard signed integer types are the standard unsigned integer types.

    So, _Bool is an integer, although admittedly one which is only guaranteed by the spec to be able to hold the values true 1 and false 0. It might, dependent on compiler, be able to hold the value 42. For clang that's certainly the case. true and false are, of course, simply preprocessor #defines to 1 and 0 respectively.

    Having boolean as an integer type is no better than what we had before, which usually looked like this:
    [code]#define bool unsigned int
    #define true 1
    #define false 0[/code]



  • @tufty said:

    "Using values as booleans" should be a high-level warning. The same goes for C++.

    And adding an operator bool() to a class should be punished by stabbing the offender with a GAU-8.

    (caveat for C++11: unless you put an explicit modifier on it.)


  • Banned

    @tufty said:

    Does this fail to compile with type errors? Fail at runtime with type errors?

    How is it relevant? You're saying that treating bools like ints is possible because C's type system isn't rich enough to make it possible to do otherwise. I'm saying it's possible because C designers fucked up. Both of these explain why the code works.


  • Winner of the 2016 Presidential Election

    @Steve_The_Cynic said:

    And adding an operator bool() to a class should be punished by stabbing the offender with a GAU-8.

    So unique_ptr is Doing It Wrong™? 🚎

    Edit: Damn, it's actually explicit. That ruins the joke. TIL.



  • @tufty said:

    Yes. The problem is that C's "type system" is not rich enough to discriminate between booleans and integers, or, indeed, any other value type. "Using values as booleans" should be a high-level warning. The same goes for C++.

    And it is. I got such warning from gcc just yesterday or the day before (the code was !0 == something and it should have been ~0 == something (~0 == -1); yes, it was a long standing bug, so apparently the warning is new(-ish)).


  • Banned

    @asdf said:

    So unique_ptr is Doing It Wrongâ„¢?

    Yes. Non-nullable unique_ptr, on the other hand...


  • Banned

    @Bulb said:

    should have been ~0

    No, it should have been -1.



  • I was responding to your "no you're wrong only ancient C doesn't have a boolean type". Having a boolean type that is absolutely indistinguishable from other value types is worthless, you are better off not having one at all; c99's bool "type" is like a prophylactic with a hole in it - it gives the impression of safety, but actually brings nothing to the table.



  • @Gaska said:

    No, it should have been -1.

    Some teammates write it as ~0 when the value it is to be compared against is unsigned, because in unsigned it is not really -1, but it is still bitwise negation of 0. I don't care which way it's written.


  • Banned

    @tufty said:

    Having a boolean type that is absolutely indistinguishable from other value types is worthless

    It is distinguishable. There is separate built-in type for booleans. All the infrastructure is there. The only problems are that:

    1. automatic conversions nullify all gains;
      364356356132345432234705290346593264752394651863295489615289645328465142695623984597162984560196324583206519764325062039746501926459650162907645032696502134650263456203645062137064509236405690316450316490562093465063654329864651682535486234095937654939543046521036591243059032465610974650239459283465961904569036452304651903465934265234659732640972365423645. operators still return int.

    Both of these are very easy changes to language spec. In fact, the former is easier to do than what they ultimately went with.


  • Banned

    @Bulb said:

    Some teammates write it as ~0 when the value it is to be compared against is unsigned

    I'm pretty sure you have undefined behavior here, since signed ints have unspecified bit representation, and literal 0 is signed int. ~0u, on the other hand, would be okay. Even better, some const unsigned int uint_max = 0xFFFFFFFF that I'm pretty sure is defined in stdlib already.



  • @Gaska said:

    ~0u

    Now that's pedantic literal use.



  • @Bulb said:

    (~0 == -1)

    Oh for the days when ones-complement architectures weren't mostly historical. On them, 0 == ~0 is true.



  • @Gaska said:

    I'm pretty sure you have undefined behavior here, since signed ints have unspecified bit representation, and literal 0 is signed int.

    Hm, true. I have never seen platform that would use anything else than two's complement, but it is not defined. And since C++ actually specifies, that -1 is converted to unsigned modulo generator of the unsigned type (2number of bits) and does not specify ~0 == -1… (The C++ library also always uses -1).

    @Gaska said:

    ~0u, on the other hand, would be okay

    Actually, that is another trap for the unwary. -1 converts to the maximum value of any unsigned type, because C++ specifies that extension is done before sign conversion. So one can use -1 whether the other side is unsigned, unsigned long, unsigned long long, uint64_t, size_t or whatever. ~0u, on the other hand…

    @Gaska said:

    Even better, some const unsigned int uint_max = 0xFFFFFFFF that I'm pretty sure is defined in stdlib already.

    Sure. std::numeric_limits<unsigned>::max(). It has, however, the same problem as the ~0u: if you refactor the other side to different type, it will silently fail. And C++ does not give warning for size mismatch. That is why I turned off the warning about sign mismatch. It encouraged doing the more fragile option.



  • @Steve_The_Cynic said:

    On them, 0 == ~0 is true.

    Nevertheless, even on them, C++ requires -1 == ~0u, meaning ~0u != ~0.



  • @asdf said:

    @Steve_The_Cynic said:
    And adding an operator bool() to a class should be punished by stabbing the offender with a GAU-8.

    So unique_ptr is Doing It Wrong™? 🚎

    Edit: Damn, it's actually explicit. That ruins the joke. TIL.


    Yes, it is (well, it isn't because is is explicit, but it would be). The preferred operator is operator const void *() const because that won't do weird, weird shit if you use the object as the key in a map. (Seriously, your map is liable to end up able to hold only two objects, one for all the cases where the operator bool returns true and one for all the cases where it returns false.)



  • @Bulb said:

    @Steve_The_Cynic said:
    On them, 0 == ~0 is true.

    Nevertheless, even on them, C++ requires -1 == ~0u, meaning ~0u != ~0.

    That's weird, because it would mean that ~0u would have to be (16-bit int here) 0xFFFE on one's complement, because the bit pattern of -1 is 0xFFFE. Or you have to have some funky-ass shit going on when you cast from signed to unsigned.



  • @Steve_The_Cynic said:

    The preferred operator is operator const void *() const because that won't do weird, weird shit if you use the object as the key in a map.

    In C++11, the explicit operator bool() is preferred because it does not do weird shit and does what it says.

    In C++03, most libraries used a member pointer.

    typedef void (unique_ptr::*unspecified_boolean_type)();
    void unspecified_boolean_true();
    public:
    operator unspecified_boolean_type() const;
    

    @Steve_The_Cynic said:

    That's weird, because it would mean that ~0u would have to be (16-bit int here) 0xFFFE on one's complement, because the bit pattern of -1 is 0xFFFE. Or you have to have some funky-ass shit going on when you cast from signed to unsigned.

    No. It means, that while -1 (in 16-bit) is 0xfffe, (unsigned)-1 is 0xffff. Because the conversion must be done by subtracting the value from unsigned zero modulo the generator.

    Oh, and because the other conversion is not specified, (signed)(unsigned)-1 can easily be 0.


  • Banned

    @Bulb said:

    C++ actually specifies, that -1 is converted to unsigned modulo generator of the unsigned type (2number of bits)

    Huh. TIL.

    @Bulb said:

    ~0u, on the other hand…

    I blame implicit conversions.

    @Bulb said:

    Sure. std::numeric_limits<unsigned>::max(). It has, however, the same problem as the ~0u: if you refactor the other side to different type, it will silently fail.

    C++11 gives you decltype. Another way is typedefing everything.



  • @Gaska said:

    Huh. TIL.

    I looked it up back then because of exactly this case. Compiler was throwing warnings for comparing -1 with unsigned, so we contemplated the -1u and fount that it silenced the warning, but broke the logic, because the thing in question was uint64_t. So I looked at how the conversions are done and concluded that -1 actually does the right thing in most cases and that we want that and nothing else and turned off the warning.

    @Gaska said:

    C++11 gives you decltype. Another way is typedefing everything.

    Yes. But it becomes unwieldy. It is shorter, and thus more readable, to just use -1 as end-of-whatever value everywhere than litter defines all over the place.



  • @Bulb said:

    In C++03, most libraries used a member pointer.

    [code]
    typedef void (unique_ptr::*unspecified_boolean_type)();
    void unspecified_boolean_true();
    public:
    operator unspecified_boolean_type() const;
    [/code]


    Goddam Dixorse suxxxx.

    That's painful, because pointer-to-member isn't necessarily even close to being just a pointer. There is significant hairiness required to correctly operate them when they might point to a virtual member function, and that means that they aren't necessarily just a single memory address. (I've seen the implication of sizes as large as 16 bytes on a 32-bit compile.)



  • Well, the assumption is that the compiler inlines the operator and optimizes passing the actual pointer away. That is the assumption with most template code anyway.


  • Banned

    @Bulb said:

    It is shorter, and thus more readable, to just use -1 as end-of-whatever value everywhere

    Shorter doesn't imply more readable (see: perl, IOCCC, minimized JS, etc.). And I guarantee you that assigning -1 to unsigned int will be confusing to every single new developer in your team.


  • FoxDev

    @Gaska said:

    And I guarantee you that assigning -1 to unsigned int will be confusing to every single new developer in your team.

    not to mention that unless i misread the C spec it's undefined behavior to assign a signed int to an unsigned int without an explicit cast, so you're in nasal demon territory.

    and even if you aren't in nasal demon territory not every system out there use twos compliment math. there are still some systems out there that use sign-magnitude.



  • @gleemonk said:

    That bang just didn't register with me 😞

    :giggity:



  • Being violated by your own js code is no laughing matter.



  • @Gaska said:

    And I guarantee you that assigning -1 to unsigned int will be confusing to every single new developer in your team.

    I guarantee you that it isn't. And it is what standard library does all over the place (eof, npos etc. are all -1, defined that way and a lot of code I've seen just uses -1 for them too).


  • Banned

    @Bulb said:

    And it is what standard library does all over the place

    C++ standard library is the most confusing codebase I've seen in my entire life, second only to Boost.



  • Why wasn't it originally this:

    if (!(part in widget.availableParts)) throw new Error("Part "+part+" not available");
    


  • @Gaska said:

    second only to Boost

    Boost is like the C++ standard library++, so I can see why that would be the case.


  • Banned

    More like ++standard_library++.


  • FoxDev

    @Gaska said:

    More like ++standard_library++.

    ..... i just ran across this code in production today.

    for (int i=0; i < 2*limit; ++i++) {
        //some code
        //about half wayu through
        short j = i/2;
        // more code using j this time, and some that uses j*2
    }
    

    i feel my reaction of turning around and unloading an entire clip of nerf darts at the developer who wrote that was entirely justified.


  • Java Dev

    Ew.


  • FoxDev

    @PleegWat said:

    Ew.

    yeah.

    i'm not sure what is worse... assigning j = i/2 and then using j*2 when i was still in scope and unchanged or that ++i++



  • @accalia said:

    i feel my reaction of turning around and unloading an entire clip of nerf darts at the developer who wrote that was entirely justified.

    From 1 foot away and into their eyes. There's no way a jury of your peers will convict you.


  • FoxDev

    @dcon said:

    From 1 foot away and into their eyes.

    about five feet away, but it was a full auto with a custom 50 dart clip



  • @accalia said:

    ```
    for (int i=0; i < 2limit; ++i++) {
    //some code
    //about half wayu through
    short j = i/2;
    // more code using j this time, and some that uses j
    2
    }

    
    Why isn't this on the front page?

Log in to reply