[Bug c++/42810] Enumeration with sequential values has its for-loop exit condition optimized out.

[tony3 at GarlandConsulting dot us]

------- Comment #12 from tony3 at GarlandConsulting dot us  2010-01-27 19:01 
-------
Here's a modified version of the original code which is intended to show just
how arbitrary the compiler optimization is from a programmer's perspective.
Here are two loops: one exits as expected, the other does not.  The only
difference between the two loops is the use of an automatic variable in one of
the two inline functions which test the exist condition.  Is this really how
you want gcc to operate?  Is it truly acceptable to the general programming
population that such a subtle difference in a supporting function can lead to a
forever loop in one case and not in another?

Both of the range testing functions perform the comparison with all their
arguments promoted to int. Yet one never returns false.

// Compile and run: g++ -O2 test.cpp && ./a.out
#include <stdio.h>

enum ENUM_VALUE
{
    ENUM_VALUE_FIRST = 1,
    ENUM_VALUE_A,
    ENUM_VALUE_B,
    ENUM_VALUE_C,
    ENUM_VALUE_D,
    ENUM_VALUE_E,
    ENUM_VALUE_LAST
};

inline bool enumInRangeWorks(const ENUM_VALUE& eVal)
{
    int val = eVal;
    return (
        (static_cast<int>(ENUM_VALUE_FIRST) <= val) &&
        (val <= static_cast<int>(ENUM_VALUE_LAST))
    );
}

inline bool enumInRangeFails(const ENUM_VALUE& eVal)
{
    return (
        (static_cast<int>(ENUM_VALUE_FIRST) <= static_cast<int>(eVal)) &&
        (static_cast<int>(eVal) <= static_cast<int>(ENUM_VALUE_LAST))
    );
}

main()
{
    printf("Compiled on %s %s\n", __DATE__, __TIME__);

    // This loop works correctly. The extra counter is not needed.
    unsigned int count = 0;
    for (
        ENUM_VALUE id = ENUM_VALUE_FIRST; 
        enumInRangeWorks(id); 
        id = static_cast<ENUM_VALUE>(id + 1)
    )
    {
        printf("id=%d\n", id);

        // Should never need to exit this way.
        if (10 == count++) break;
    }
    printf("ENUMERATION COMPARISON ");
    printf("%s\n", count < 10? "WORKED" : "FAILED");

    // This loop fails to exit. Without the special case additional
    // counter it results in a forever loop.
    count = 0;
    for (
        ENUM_VALUE id = ENUM_VALUE_FIRST; 
        enumInRangeFails(id); 
        id = static_cast<ENUM_VALUE>(id +1)
    )
    {
        printf("id=%d\n", id);

        // Should never need to exit this way.
        if (10 == count++) break;
    }
    printf("ENUMERATION COMPARISON ");
    printf("%s\n", count < 10? "WORKED" : "FAILED");
}

I'm sure the response is likely to be "the version that happens to work is
invalid and therefore this is a moot point."  That may be, but it does
illustrate how misleading gcc is when it allows code like this to compile
without warning and then behaves radically differently for minor variations.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42810