Avoiding compiler warnings when writing new code

Contents

Avoiding compiler warnings when writing new code

Purpose

Our goal is to have the Helix codebase compile with as few warnings as possible. However, different compilers on different platforms complain about different things. The purpose of this document is to describe several common code constructs which produce warnings. We also describe how to fix or avoid these warnings.

Common Warnings

Warning: Class initialization list order doesn't match declaration list

Example: Notice in the example below that m_ulBar1 is declared AFTER m_ulBar2, but is initialized BEFORE m_ulBar2.


       class CFoo
       {
           CFoo() :
              m_ulBar2(0),
              m_ulBar1(0)
           {
           };
           UINT32 m_ulBar1;
           UINT32 m_ulBar2;
       }

Solution: Make the initialization order match the declaration order.


       class CFoo
       {
           CFoo() :
              m_ulBar1(0),
              m_ulBar2(0)
           {
           };
           UINT32 m_ulBar1;
           UINT32 m_ulBar2;
       }

Warning: signed vs. unsigned comparisons

Example:


          UINT32 ulFoo;
          INT32  lBar;

          ...
          if (lBar < ulFoo)


Solution: Change the signed-ness of one of the variables or cast one of them.


          UINT32 ulFoo;
          INT32  lBar;

          ...
          if (lBar < ((INT32) ulFoo))


Be careful with casts however. If you cast a signed to an unsigned, make sure you handle the case when the signed value could be negative. If you cast an unsigned to a signed, make sure that the unsigned value will not get really big (>= MAX_INT32).

Warning: unused variables

Example: Sometimes this happens if we declare a variable and never use it. Often, this happens because we declare a variable and only use it in an HX_ASSERT(). So on debug builds we don't get the warning, and only get it on release builds, when the HX_ASSERT() goes away.


            HX_RESULT rv = (bar > 0 ? HXR_OK : HXR_FAIL);
            HX_ASSERT(SUCCEEDED(rv));

We would get an unused variable warning for rv in release builds.

Solution: If we really don't need a variable, remove it. If we do need the variable for the assert and that's really the only place we need it, then change it to:


            HX_RESULT rv = HXR_FAIL;
            if (bar > 0)
            {
                rv = HXR_OK;
            }
            HX_ASSERT(SUCCEEDED(rv));

Warning: enumeration value not handled in switch

Example:


       enum FooEnum
       {
          FooEnum1,
          FooEnum2,
          FooEnum3
       };

       ...
       FooEnum bar;
       switch (bar)
       {
           case FooEnum1:
              ...
              break;
           case FooEnum2:
              ...
              break;
       }

Some compilers will complain because for enumerated types they expect to see a "case" for each enumerated type.

Solution: You can: a) Add no-op "case" statements for each unused enumerated type; or b) consider using constants instead of enumerated types.

Warning: left hand operand of comma expression has no effect

Example:


    for(i=0;i < iNumCoords,pTok;++i)

Solution: this is pretty rarely used in our code, and makes code hard to understand, since most C/C++ developers are not that familiar with the comma operator. There is no one way to fix this - analyze the logic of the code and rewrite so as not to use the comma operator in a "for" condition like this.

Warning: suggest parentheses around assignment used as truth value

Example:


    if (foo = getbar())

Solution: Add parentheses:


             if ((foo = getbar()))

However, many developers find these constructs hard to read. Consider assigning foo outside the if statement, as in:


             foo = getbar()
             if (foo)

This is more readable.