Cleanup of Clang-Tidy warnings in Applewin

[maxolasersquad]

This resolves some warning provided by Clang-Tidy. I only targeted Applwin.(c/h) in this commit. I have tested the code compiles and could play Cave of Time.

Primarily changes are

• bools now use true and false instead of 1 and 0.
• Use the cpp time in place of the c time.h.
• Replace use of NULL with nullptr.

There are other changes than this. This should definitely be inspected by someone with more cpp experience than me who can evaluate and test the likely impacts of this in more detail. Until then I'm leaving this as a draft for review. If this looks good I'll take the time to update the other files.

[akhepcat]

for the bool section, that would be moving the wrong way - at least as i'm parsing how you've written it out. You don't want integer literals in place of logical boolean values.

These are the clang-tidy options that impact boolean checking:

bugprone-bool-pointer-implicit-conversion
readability-implicit-bool-conversion
readability-implicit-bool-cast
readability-simplify-boolean-expr
modernize-use-bool-literals

make sure that your clang-tidy is enabling them (i think you can get away with just "modernize..." for most of this, though "bugprone..." might be good as well)

[maxolasersquad]

@akhepcat

for the bool section, that would be moving the wrong way

It is better to use 1 and 0 over true and false?

Running with the options you suggested, it still recommends the opposite.

$ clang-tidy src/Applewin.cpp -checks=bugprone-bool-pointer-implicit-conversion,readability-implicit-bool-conversion,readability-implicit-bool-cast,readability-simplify-boolean-expr,modernize-use-bool-literals

...
/home/maxolasersquad/linapple/src/Applewin.cpp:109:16: warning: converting integer literal to bool, use bool literal instead [modernize-use-bool-literals]
bool restart = 0;
...

[akhepcat]

No, i think we're in agreement-

I just noted that the way you wrote it initially was confusing to me without an explicit "should be" (should be using true/false instead of...)

but for clarity, yes, it should be boolean values and not integer literals.

[maxolasersquad]

Ah, I see. Thanks.

[maxolasersquad]

I updated my original comment for clarity.

[maxolasersquad]

I added a .clang-tidy and hope to get feedback. It's basically the default file on Ubuntu, with the provided options added. It would be awesome to one day add a clang-tidy check to make sure all new code conforms to whatever standards we agree on. First we need to agree on standards and update the code to pass.

[akhepcat]

Since it's not too impacting to just add the option in the Makefile, like...

ctidy:
        clang-tidy $(SRCDIR)/*.$(SRCEXT) -- $(CFLAGS) $(INC) 2>&1 | tee clang-tidy.log

why not add it as a separate target with no dependency linkage, so that it can be worked manually by folk who are interested, but doesn't impact the build process?

(currently generates 2842 warnings!)

[akhepcat]

after playing around with clang-tidy, and doing some cleanups, I've got things down to:

• 67 warnings and 3 errors without .clang-tidy, or
• 327 warnings and 3 errors with .clang-tidy

I did end up removing "readability-implicit-bool-conversion" from .clang-tidy, as it conflicts with modernize-use-bool-literals

https://github.com/akhepcat/linapple has my current approach. there's... a lot of fixes in place.

[maxolasersquad]

@akhepcat, I added the new target in Makefile and also removed readability-implicit-bool-conversion as you pointed out.