Move platform preprocessor definitions to cmake

[antoinevg]

This PR moves the platform definitions (e.g. IS_PRALINE, IS_HACKRF_ONE etc.) into cmake.

The specific issues it addresses are:

• The include order of platform_detect.h matters whenever #ifdef IS_<some_board> is used. If it is not included first it can result in compile errors that are not immediately obvious.
• This causes false negatives with include-what-you-use resulting in a sprinkling of IWYU pragma: keep across the sources.

These have caused some friction for me recently.

The downsides are:

• Moving things into cmake.
• It uses add_compile_definitions() which is only available in cmake >= 3.12. Though it could be implemented using add_definitions() which would just look uglier.
• switch (detected_platform()) can no longer evaluate that a given check is exhaustive. That said, we don't have too many of those left.
• The optional introduction of IS_UNIVERSAL (see 3rd commit). An argument can be made that this helps to annotate portions of the code that are affected when building a universal binary but it's been nice to not have it so far.

The main advantage I see is that it generally makes the usage of platform_detect.h a little less fraught.

Would love to hear feedback!

[martinling]

If we go ahead with this, I suggest we just bump the firmware CMake requirement to 3.12 to support it. I've pushed a commit with that change to this branch. Note that we can leave the host code requirement at 3.10.

My rationale for choosing 3.10 as our last bump was here, and there's no particular requirement to avoid bumping it a little further, particularly for firmware builds.

CMake 3.12 was released in 2018. Debian buster from 2019 is beyond even the oldoldstable horizon now, and shipped with 3.13.

[antoinevg]

This is getting somewhat cursed.

2252036 is a pretty verbose solution but it does express intent clearly. Unfortunately, this does not work on iwyu==0.23 as, unlike iwyu==0.26, it appears unable to detect usage defined in a pre-processor macro leading to false positives for removals.

I've now pushed an alternate approach with 9f3182b which simply passes --keep platform_detect.h to pacify iwyu until we have 0.26 running in CI.

So the situation now is that we should probably choose one of the following:

1. Stick with the first two commits, and live with the fact that it's not always going to be obvious to external contributors when they need to mark a #include "platform_detect.h" with // IWYU pragma: keep and when not.
2. Temporarily let go of the ability to detect spurious includes for platform_detect.h in CI. (We still get compiler errors if it's not included when it should be so that's still good.)

I think the overall improvement in ergonomics is worth putting up with the small loss of coverage we get with 2. but I'd love to hear other suggestions!

[martinling]

I've now pushed an alternate approach with 9f3182b which simply passes --keep platform_detect.h to pacify iwyu until we have 0.26 running in CI.

I like this solution. I'm fine with that leaving the possibility of spurious includes of platform_detect.h.

[mossmann]

This looks great but needs a rebase. I like the --keep solution.

[martinling]

It occured to me that an alternative to moving these defines to CMake, could be to make platform_detect.h be automatically included everywhere, by passing -include platform_detect.h as part of the compiler flags.

[antoinevg]

It occured to me that an alternative to moving these defines to CMake, could be to make platform_detect.h be automatically included everywhere, by passing -include platform_detect.h as part of the compiler flags.

Tempting :-)

The main reason I'd rather not do that is my bias towards keeping things as explicit as practical and keeping that include in the sources for now serves as a useful marker that a given translation unit has runtime platform checks present.

[antoinevg]

This looks great but needs a rebase. I like the --keep solution.

Rebased and squashed 👍