Orx code linting - clang compiler option Orx supported

The goal would be to come up with a clang compiler orx supported option set allowing -Wextra -Wall compilation.

Follows a summary of the flags reviewed by iarwain at this point:
https://github.com/orx/orx/pull/43

Clang Orx diagnostic flags status
    https://clang.llvm.org/docs/DiagnosticsReference.html

summary of first reviewed batch

-Wno-sign-compare
-Wno-unused-parameter
-Wno-c++98-compat-pedantic

- disable
+ enable

    [-Wbad-function-cast]
 -  [-Wc11-extensions]
 -  [-Wc++98-compat-pedantic]
    [-Wcast-align]
 +  [-Wcast-qual]
    [-Wcomma]
    [-Wconditional-uninitialized]
    [-Wconversion]
    [-Wdeprecated]
    [-Wdisabled-macro-expansion]
 +  [-Wdocumentation]
    [-Wdocumentation-deprecated-sync]
    [-Wdocumentation-unknown-command]
 -  [-Wdouble-promotion]
    [-Wextra-semi]
    [-Wfloat-conversion]
 -  [-Wfloat-equal]
 -  [-Wformat-nonliteral]
    [-Wglobal-constructors]
    [-Wgnu-anonymous-struct]
    [-Wgnu-empty-initializer]
 -  [-Wgnu-zero-variadic-macro-arguments]
    [-Wimplicit-fallthrough]
    [-Wimplicit-function-declaration]
    [-Wmissing-prototypes]
    [-Wmissing-variable-declarations]
    [-Wnested-anon-types]
    [-Wnull-pointer-arithmetic]
    [-Wold-style-cast]
 -  [-Wpadded]
 -  [-Wpedantic]
 -  [-Wreserved-id-macro]
    [-Wshadow]
    [-Wshadow-field-in-constructor]
 +  [-Wshorten-64-to-32]
    [-Wsign-compare]
 +  [-Wsign-conversion]
 -  [-Wstrict-prototypes]
 -  [-Wswitch-enum]
 +  [-Wundef]
    [-Wunknown-warning-option]
    [-Wunused-macros]
    [-Wunused-parameter]
    [-Wvla]
    [-Wweak-vtables]
    [-Wzero-as-null-pointer-constant]
    [-Wzero-length-array]

Comments

  • First warning batch already reviewed:

    Linux clang compile took 10 hours
    
    0 error(s), 45569 warning(s) (621 minute(s), 26 second(s))
    
    with the following options:
    
    -Wextra
    -ffast-math
    -msse2
    -fno-exceptions
    -Wno-unused-function
    -Wno-unused-but-set-variable
    $(ORXFLAGS)
    
    - summary ---
    
    coding standards
    
      Can specifying the supported compiler option set be considered?
    
        possibly give a short explain when support is not required?
    
    - option / warning / reference summary ---
    
    [-Wpedantic]
      warning: ISO C restricts enumerator values to range of 'int' (4294967295 is too large)
        https://stackoverflow.com/questions/38873404/how-to-avoid-pedantic-warnings-while-using-hexadecimal-in-enum
    
    [-Wreserved-id-macro]
       https://stackoverflow.com/questions/228783/what-are-the-rules-about-using-an-underscore-in-a-c-identifier
    
    [-Wstrict-prototypes]
       https://stackoverflow.com/questions/693788/is-it-better-to-use-c-void-arguments-void-foovoid-or-not-void-foo
    
    [-Wdocumentation]
      includes checking
        \param commands name parameters
           actually present in the function signature
        \return used only on functions that actually return a value
        etc.
    
    [-Wgnu-zero-variadic-macro-arguments]
      solution? #pragma clang diagnostic ignored "-Wgnu-zero-variadic-macro-arguments"
    
    [-Wsign-conversion]
    [-Wfloat-equal]
    [-Wcast-qual]
    [-Wshorten-64-to-32]
    [-Wfloat-equal]
    
    - warning detail ---
    
    includes the first warning instance encountered:
    
    [-Wreserved-id-macro]
    warning: macro name is a reserved identifier [-Wreserved-id-macro]
    ../../../include/base/orxDecl.h:47:11: warning: macro name is a reserved identifier [-Wreserved-id-macro]
      #define __orxPROFILER__
              ^
    
    [-Wundef]
    ../../../include/base/orxDecl.h:191:9: warning: 'TARGET_OS_IPHONE' is not defined, evaluates to 0 [-Wundef]
      #elif TARGET_OS_IPHONE
            ^
    [-Wpedantic]
    ../../../include/base/orxType.h:181:3: warning: ISO C restricts enumerator values to range of 'int' (4294967295 is too large) [-Wpedantic]
      orxSEEK_OFFSET_WHENCE_NONE = orxENUM_NONE
      ^                            ~~~~~~~~~~~~
    
    [-Wdocumentation]
    ../../../include/base/orxModule.h:116:83: warning: this function declaration is not a prototype [-Wstrict-prototypes]
    typedef orxSTATUS                         (orxFASTCALL *orxMODULE_INIT_FUNCTION)  ();
    
    ../../../include/plugin/orxPluginCore.h:96:5: warning: '@return' command used in a comment that is attached to a function returning void [-Wdocumentation]
     * @return nothing.
    
    ../../../include/memory/orxMemory.h:249:15: warning: parameter '_s32Size' not found in the function declaration [-Wdocumentation]
     * @param[in] _s32Size                Size to track, in bytes
    
    [-Wgnu-zero-variadic-macro-arguments]
    ../../../include/debug/orxDebug.h:311:153: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
            orxDEBUG_PRINT(orxDEBUG_LEVEL_ASSERT, orxANSI_KZ_COLOR_BG_RED orxANSI_KZ_COLOR_FG_WHITE orxANSI_KZ_COLOR_BLINK_ON "FAILED ASSERTION [" #TEST "]", ##__VA_ARGS__); \
    
    [-Wsign-conversion]
    ../../../include/math/orxMath.h:227:15: warning: implicit conversion changes signedness: 'int' to 'orxU32' (aka 'unsigned int') [-Wsign-conversion]
      u32Result = __builtin_popcount(_u32Value);
    
    [-Wfloat-equal]
    ../../../include/math/orxVector.h:248:19: warning: comparing floating point with == or != is unsafe [-Wfloat-equal]
      orxASSERT(_fOp2 != orxFLOAT_0);
                ~~~~~ ^  ~~~~~~~~~~
    ../../../include/debug/orxDebug.h:309:12: note: expanded from macro 'orxASSERT'
          if(!(TEST))                                                                                                                                                         \
               ^~~~
    
    [-Wpadded]
    ../../../include/core/orxSystem.h:95:5: warning: padding size of 'struct __orxSYSTEM_EVENT_PAYLOAD_t::(anonymous at ../../../include/core/orxSystem.h:95:5)' with 4 bytes to alignment boundary [-Wpadded]
        struct
        ^
    ../../../include/core/orxSystem.h:102:5: warning: padding size of 'struct __orxSYSTEM_EVENT_PAYLOAD_t::(anonymous at ../../../include/core/orxSystem.h:102:5)' with 4 bytes to alignment boundary [-Wpadded]
        struct
        ^
    
    [-Wcast-qual]
    ../../../include/utils/orxString.h:355:22: warning: cast from 'const char *' to 'unsigned char *' drops const qualifier [-Wcast-qual]
      pu8Byte = (orxU8 *)_zString;
    
    [-Wshorten-64-to-32]
    ../../../include/utils/orxString.h:795:53: warning: implicit conversion loses integer precision: 'size_t' (aka 'unsigned long') to 'int' [-Wshorten-64-to-32]
      *_ps32OutValue = (orxS32)strtol(_zString, &pcEnd, STRTO_CAST _u32Base);
                               ~~~~~~                   ^~~~~~~~~~~~~~~~~~~
    
    [-Wdouble-promotion]
    /orx/code/src/anim/orxAnim.c:964:169: warning: implicit conversion increases floating-point precision: 'orxFLOAT' (aka 'float') to 'double' [-Wdouble-promotion]
         orxDEBUG_PRINT(orxDEBUG_LEVEL_ANIM, "Can't add event <%s>: its timestamp [%g] needs to be strictly greater than previous event's one (<%s> @ [%g]).", _zEventName, _fTimeStamp, _pstAnim->astEventList[orxAnim_GetEventCount(_pstAnim) - 1].zName, _pstAnim->astEventList[orxAnim_GetEventCount(_pstAnim) - 1].fTimeStamp);
    
    [-Wfloat-equal]
    ../../../include/math/orxVector.h:248:19: warning: comparing floating point with == or != is unsafe [-Wfloat-equal]
      orxASSERT(_fOp2 != orxFLOAT_0);
                ~~~~~ ^  ~~~~~~~~~~
    
    // ** pragma disable candidate list ********************
    
    [-Wc11-extensions]
    ../../../include/math/orxVector.h:67:3: warning: anonymous unions are a C11 extension [-Wc11-extensions]
      union
    
    [-Wformat-nonliteral]
    ../../../include/utils/orxString.h:1535:37: warning: format string is not a string literal [-Wformat-nonliteral]
      s32Result = vsprintf(_zDstString, _zSrcString, stArgs);
                                        ^~~~~~~~~~~
        https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html
            format string not string literal so
              cannot be checked, unless the format function takes its format arguments as a va_list
    
    [-Wswitch-enum]
    ../../../include/object/orxStructure.h:214:10: warning: enumeration values 'orxSTRUCTURE_ID_NUMBER' and 'orxSTRUCTURE_ID_NONE' not explicitly handled in switch [-Wswitch-enum]
      switch(_eID)
    
    --------
    

    Thank you for running clang with all the warnings.

    Here's a preliminary list of warnings that I think are not really relevant to our use case:

    [-Wc11-extensions]
    [-Wdouble-promotion]
    [-Wfloat-equal]
    [-Wformat-nonliteral]
    [-Wgnu-zero-variadic-macro-arguments]
    [-Wpadded]
    [-Wpedantic]
    [-Wreserved-id-macro]
    [-Wstrict-prototypes]
    [-Wswitch-enum]
    

    If you have a list of the defects found with the other flags, I'd be happy to have a look.

  • Hi @_N_, thanks for continuing the discussion here.
    Sorry for the delay, currently dealing with people getting sick at home. I'll try to do the second pass this week.
    On your side, would it be possible to post all the occurrences of the warnings we agreed to keep?
    As I'm mostly working on Windows with Visual Studio, that would be quite helpful to me.
  • llvm warnings - summary - part 2
    
    orx library llvm compile with options
    
    -Wextra
    -ffast-math
    -msse2
    -fno-exceptions
    -Wno-unused-function
    -Wno-unused-but-set-variable
    $(ORXFLAGS)
    
    - summary ---
    
    turn off ---
        suggested option list to turn off
    
    review summary ---
        summary option list to review
    
    review example ---
        first warning instance with code
      (see next post)
    
    ---
    
    turn off ---
    
    -Wdeprecated
    -Wdisabled-macro-expansion
    
    review summary ---
    
    -Wbad-function-cast
    -Wcast-align
    -Wcast-qual
    -Wcomma
        any intent in using commas vs. semicolons?
    -Wconditional-uninitialized
    -Wconversion
    -Wdocumentation-deprecated-sync
    -Wdocumentation-unknown-command
    -Wextra-semi
    -Wfloat-conversion
    -Wglobal-constructors
        3 instances in the build
      possible performance issue, and
        since order of globals/statics initialization is undefined in C++,
          non-trivial global object might introduce in its constructor
             a dependency on another global object,
               leading to undefined behavior.
                  https://stackoverflow.com/questions/15708411/how-to-deal-with-global-constructor-warning-in-clang
    
      workaround / fix: instead of
          class A {
          public:
            // ...
            A();
          };
    
          A my_A; // triggers said warning
    
      use
          A&
          my_A()
          {
              static A a;
              return a;
          }
    
    -Wgnu-anonymous-struct
    -Wgnu-empty-initializer
    -Wgnu-zero-variadic-macro-arguments
    -Wimplicit-fallthrough
    -Wimplicit-function-declaration
    -Wmissing-prototypes
    -Wmissing-variable-declarations
    -Wnested-anon-types
    -Wnull-pointer-arithmetic
    -Wold-style-cast
    
  • review example ---
    
    -Wbad-function-cast
    /orx/code/src/anim/orxAnimSet.c:1414:55: warning: cast from function call of type 'void *' to non-matching type 'orxU64' (aka 'unsigned long long') [-Wbad-function-cast]
            u32DstAnim = ((orxU32) orxANIMSET_CAST_HELPER orxHashTable_Get(pstResult->pstIDTable, orxString_ToCRC(zDstAnim)) - 1);
    
    -Wcast-align
    /orx/code/src/core/orxClock.c:1193:14: warning: cast from 'orxU8 *' (aka 'unsigned char *') to 'orxCLOCK *' (aka 'struct __orxCLOCK_t *') increases required alignment from 1 to 8 [-Wcast-align]
      pstClock = orxSTRUCT_GET_FROM_FIELD(orxCLOCK, stClockInfo, _pstClockInfo);
    
    -Wcast-qual
    /orx/code/src/core/orxClock.c:1403:3: warning: cast from 'const struct __orxCLOCK_t *' to 'struct __orxSTRUCTURE_t *' drops const qualifier [-Wcast-qual]
      orxSTRUCTURE_ASSERT(_pstClock);
    
    -Wcomma
    /orx/code/src/core/orxConfig.c:2738:22: warning: possible misuse of comma operator here [-Wcomma]
            pcLineStart++, pc++;
                         ^
    /orx/code/src/core/orxConfig.c:2738:9: note: cast expression to void to silence warning
            pcLineStart++, pc++;
    
    ../../../../extern/stb_image/stb_image_write.h:288:26: warning: possible misuse of comma operator here [-Wcomma]
       arr[0] = a, arr[1] = b, arr[2] = c;
    
    -Wconditional-uninitialized
    /orx/code/src/anim/orxAnimSet.c:2040:33: warning: variable 'pfOriginDeltaX' may be uninitialized when used here [-Wconditional-uninitialized]
                vFrameOrigin.fX -= *pfOriginDeltaX;
                                    ^~~~~~~~~~~~~~
    /orx/code/src/anim/orxAnimSet.c:1816:63: note: initialize the variable 'pfOriginDeltaX' to silence this warning
    
    -Wconversion
    /orx/code/src/core/orxResource.c:747:68: warning: implicit conversion loses floating-point precision: 'double' to 'orxFLOAT' (aka 'float') [-Wconversion]
            orxClock_RemoveGlobalTimer(orxResource_NotifyUpdateChange, orxRESOURCE_KF_WATCH_NOTIFICATION_DELAY, _pContext);
            ~~~~~~~~~~~~~~~~~~~~~~~~~~                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    
    -Wdocumentation-deprecated-sync
    ../../../../extern/glfw-3/include/GLFW/glfw3.h:1437:6: warning: declaration is marked with '\deprecated' command but does not have a deprecation attribute [-Wdocumentation-deprecated-sync]
     *  @deprecated Scheduled for removal in version 4.0.
    
    -Wdocumentation-unknown-command
    ../../../../extern/glfw-3/include/GLFW/glfw3.h:1183:5: warning: unknown command tag name [-Wdocumentation-unknown-command]
     *  @glfw3 Added window handle parameter.
        ^~~~~~
    
    -Wextra-semi
    /orx/code/src/object/orxObject.c:5275:42: warning: extra ';' outside of a function [-Wextra-semi]
    orxOBJECT_MAKE_RECURSIVE(Enable, orxBOOL);
    
    -Wfloat-conversion
    ../../../include/../plugins/Display/GLFW/orxDisplay.c:5013:35: warning: implicit conversion turns floating-point number into integer: 'GLdouble' (aka 'double') to 'bool' [-Wfloat-conversion]
        sstDisplay.dLastOrthoBottom = (GLdouble)(sstDisplay.apstDestinationBitmapList[0] != orxNULL)
    
    -Wglobal-constructors
    In file included from /orx/code/src/plugin/orxPlugin_EmbeddedList.cpp:72:
    ../../../include/../plugins/Physics/LiquidFun/orxPhysics.cpp:93:25: warning: declaration requires a global constructor [-Wglobal-constructors]
      static const b2Color  stRayMissColor          = b2Color(0x00, 0xFF, 0x00);
                            ^                         ~~~~~~~~~~~~~~~~~~~~~~~~~
    ../../../include/../plugins/Physics/LiquidFun/orxPhysics.cpp:94:25: warning: declaration requires a global constructor [-Wglobal-constructors]
      static const b2Color  stRayBeforeHitColor     = b2Color(0xFF, 0xFF, 0x00);
                            ^                         ~~~~~~~~~~~~~~~~~~~~~~~~~
    ../../../include/../plugins/Physics/LiquidFun/orxPhysics.cpp:95:25: warning: declaration requires a global constructor [-Wglobal-constructors]
      static const b2Color  stRayAfterHitColor      = b2Color(0xFF, 0x00, 0x00);
                            ^                         ~~~~~~~~~~~~~~~~~~~~~~~~~
    
    -Wgnu-anonymous-struct
    ../../../include/display/orxDisplay.h:63:5: warning: anonymous structs are a GNU extension [-Wgnu-anonymous-struct]
        struct
        ^
    ../../../include/display/orxDisplay.h:63:5: warning: anonymous types declared in an anonymous union are an extension [-Wnested-anon-types]
    ../../../include/display/orxDisplay.h:265:5: warning: anonymous types declared in an anonymous union are an extension [-Wnested-anon-types]
        struct
        ^
    ../../../include/display/orxDisplay.h:279:5: warning: anonymous types declared in an anonymous union are an extension [-Wnested-anon-types]
        struct
    
    -Wgnu-empty-initializer
    /orx/code/src/anim/orxAnimSet.c:1495:30: warning: use of GNU empty initializer extension [-Wgnu-empty-initializer]
        orxCHAR acLinkName[64] = {};
    
    -Wimplicit-fallthrough
    ../../../../extern/stb_image/stb_image.h:5064:7: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough]
          case 15: if(is_rgb16) *is_rgb16 = 1;
    
    -Wimplicit-function-declaration
    /orx/code/src/debug/orxDebug.c:457:9: warning: implicit declaration of function 'asm' is invalid in C99 [-Wimplicit-function-declaration]
            asm("int $3");
    
    -Wmissing-prototypes
    /orx/code/src/anim/orxAnim.c:344:23: warning: no previous prototype for function 'orxAnim_SetName' [-Wmissing-prototypes]
    orxSTATUS orxFASTCALL orxAnim_SetName(orxANIM *_pstAnim, const orxSTRING _zName)
    
    -Wmissing-variable-declarations
    /orx/code/src/display/orxDisplay.c:114:1: warning: no previous extern declaration for non-static variable '_orxCoreFunctionPointer_orxDisplay_Init' [-Wmissing-variable-declarations]
    ../../../include/plugin/orxPluginCore.h:152:24: note: expanded from macro 'orxPLUGIN_DEFINE_CORE_FUNCTION'
      RETURN (orxFASTCALL *orxPLUGIN_CORE_FUNCTION_POINTER_NAME(FUNCTION_NAME))(__VA_ARGS__) = (RETURN(orxFASTCALL *)(__VA_ARGS__)) (&orxPLUGIN_DEFAULT_CORE_FUNCTION_NAME(FUNCTION_NAME))
    
    -Wnested-anon-types
    ../../../include/core/orxSystem.h:87:5: warning: anonymous types declared in an anonymous union are an extension [-Wnested-anon-types]
        struct
    
    -Wnull-pointer-arithmetic
    ../../../../extern/dlmalloc/malloc.c:3489:35: warning: arithmetic on a null pointer treated as a cast from integer to pointer is a GNU extension [-Wnull-pointer-arithmetic]
          size_t mfree = m->topsize + TOP_FOOT_SIZE;
                                      ^~~~~~~~~~~~~
    
    -Wold-style-cast
    ../../../include/base/orxType.h:192:59: warning: use of old-style cast [-Wold-style-cast]
    static const orxFLOAT             orxFLOAT_0            = orx2F(0.0f);
    ../../../include/base/orxType.h:161:36: note: expanded from macro 'orx2F'
        #define orx2F(V)              ((orxFLOAT)(V))
    
    instead of C-style cast use C++ cast?
    
    turn on ---
    -Wimplicit-function-declaration
       -std=gnu99
          fix - 'asm' is invalid in C99
    turn off ----
    
    -Wdeprecated
    ../../../../extern/LiquidFun-1.1.0/include/Box2D/Particle/b2Particle.h:124:19: warning: definition of implicit copy constructor for 'b2ParticleColor' is deprecated because it has a user-declared copy assignment operator [-Wdeprecated]
            b2ParticleColor& operator = (const b2ParticleColor &color)
    
    -Wdisabled-macro-expansion
    ../../../../extern/stb_image/stb_image.h:1399:50: warning: disabled expansion of recursive macro [-Wdisabled-macro-expansion]
       if (output == NULL) { STBI_FREE(data); return stbi__errpf("outofmem", "Out of memory"); }
    
  • All warnings without -Wextra -Wall (to minimize warning set size), using clang orx prospective compiler flag set:

    -fPIC
      https://stackoverflow.com/questions/26465305/position-independent-code-what-is-the-difference-at-compile-time
    
    -O2
    -m64
    
    -fno-rtti
      https://stackoverflow.com/questions/4486609/when-can-compiling-c-without-rtti-cause-problems
    
    -Wno-sign-compare
    -Wno-unused-parameter
    -Wno-c++98-compat-pedantic
    
    -Wno-c11-extensions
    -Wno-double-promotion
    -Wno-float-equal
    -Wno-format-nonliteral
    -Wno-gnu-zero-variadic-macro-arguments
    -Wno-padded
    -Wno-pedantic
    -Wno-reserved-id-macro
    -Wno-strict-prototypes
    -Wno-switch-enum
    
    -ffast-math
    -msse2
    -fno-exceptions
    -Wno-unused-function
    -Wno-unused-but-set-variable
    $(ORXFLAGS)
    

    llvm warnings - keep list.txt

  • edited December 2019

    Thanks for the post! However it seems this wasn't done with the most recent version as I can see some code divergence between your log and the current github version.
    Do you know how old was the codebase used?

  • _N__N_
    edited December 2019

    Indeed I used the 2019/09/29 build.

    Once I get the final list of keeper flags I can do a final run.

    isystem does not seem to have an effect on -Weverything on my machine at least.

    I use a set of grep sed filters to remove the noise.

    clang warnings - extern headers removed

          5 [-Wbad-function-cast]
         37 [-Wcast-align]
        544 [-Wcast-qual]
          1 [-Wcomma]
         25 [-Wconditional-uninitialized]
          4 [-Wconversion]
        282 [-Wdocumentation]
          2 [-Wfloat-conversion]
          2 [-Wimplicit-fallthrough]
          1 [-Wimplicit-function-declaration]
        783 [-Wmissing-prototypes]
        185 [-Wmissing-variable-declarations]
       2305 [-Wold-style-cast]
         18 [-Wshadow]
        176 [-Wshorten-64-to-32]
        558 [-Wsign-conversion]
         56 [-Wundef]
          1 [-Wunreachable-code]
         74 [-Wunused-macros]
          6 [-Wvla]
          1 [-Wweak-vtables]
        469 [-Wzero-as-null-pointer-constant]
    
Sign In or Register to comment.