Skip to content

DeFossilizeTheImpatient

Jump to bottom
Matthias Melcher edited this page Feb 27, 2024 · 3 revisions

The Impatient

After many hours of research on refactoring over the last months, I know I should set my goals, analyze what I have, find a path, and I will do that. Promised. But tonight my first fun task is to enable all compiler warnings with

target_compile_options(fluid-lib PRIVATE -std=c++11 -Wall -Wextra -pedantic -Werror -Wno-missing-field-initializers)

To my surprise, I got very few warnings, easily fixable in five minutes. So now for the big one: -Weverything. Yeah, it's as I expected, I get hundreds of warnings everywhere, including the header files of the library itself. I can't change those headers yet, but I can suspend the über-warnings of all warnings while including files:

#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Weverything"
#include <FL/Fl.H>
#pragma clang diagnostic pop

Some of the warnings are not useful, but the sheer number of times that I should use nullptr instead of NULL or where I use C style casting is impressive! Those are easy fixes, but I have to be careful, not to just search and replace. Every warning must be verified and a note must be added if I find anything suspicious or want to do changes later.

A few observations

  • the code uses 0 and NULL interchangeably where it should be nullptr now. I wonder how many times a 0 was mistaken for an integer and not a pointer
  • the old code free((void*)p) completely obscured that p was declared const. Changing to static_cast<void*>(p) revealed the issue
  • the same code emphasizes the even bigger issue of using malloc and free, throwing away all type safety
  • which in turn revealed that we were happily writing to const char* member variables all the time in at least one class

I will revert my -Weverything changes for now, but yeah, the decision to systematically go through the code was a good one.

Enough played for today.

PS: it was too easy, so I just had to do it. I added the line set(CMAKE_CXX_CLANG_TIDY "clang-tidy;-checks=*") to the top level CMakeLists.txt, and I got a huge wall of suggestion on how to improve the code. It actually crashed clang-tidy before I even got to the FLUID code. - Yes, I know that I can set and clear individual analysis modules. This was just or fun, and fun it was!

Clone this wiki locally