Reviewing compiler options that ignore warnings

[acvictor]

Ignoring some compiler warnings can introduce significant correctness, reliability, and security risks. I recently spent days debugging a weird crash with someone from my team at Microsoft who figured out that the cause was a bool function that was not returning anything although the crash log seemed to indicate an issue with memory freeing. I've since checked in one PR to remove -Wno-return-type and have another one up for -Wno-stringop-overflow, -Wno-stringop-overread. We should methodically review the compiler flags for Velox and Gluten and attempt to reduce the number of disabled warnings. I would like to take a stab at some other 'serious' ones on the list. For GNU the entire set includes -

set(KNOWN_COMPILER_SPECIFIC_WARNINGS
        "-Wno-implicit-fallthrough \
         -Wno-empty-body \
         -Wno-comment \
         -Wno-class-memaccess \
         -Wno-int-in-bool-context \
         -Wno-redundant-move \
         -Wno-array-bounds \
         -Wno-maybe-uninitialized \
         -Wno-unused-result \
         -Wno-format-overflow \
         -Wno-strict-aliasing \
         -Wno-type-limits \
         -Wno-stringop-overflow \
         -Wno-stringop-overread")

@mbasmanova what do you think?

[mbasmanova]

@acvictor I agree. It would be nice to enable compiler to highlight code problems as much as possible.

[acvictor]

Thank you @mbasmanova! I'll take this up.

[majetideepak]

Yes, let's fix the code!

[pedroerp]

Thank you @acvictor for looking into this.

For historical context, I suppose we ended up adding these flags because of the dependencies we compile (we used to pass the exact same compiler flags to compile dependencies and vendored code), but this is better controlled nowadays. Thanks for the clean up.

Cc: @assignUser @majetideepak @kgpai

[assignUser]

👍 #7389 (comment)