h2zero / n-able-Arduino

An arduino core for ARM based BLE devices supported by the NimBLE stack.
GNU Lesser General Public License v2.1
36 stars 15 forks source link

`min` and `max` definitions in `Arduino.h` cause compilation errors #12

Open pdcook opened 1 year ago

pdcook commented 1 year ago

Any project which uses std::unordered_map fails to compile. The error message is extraordinarily long, but starts as:

rom /home/pcook/.arduino15/packages/h2zero/tools/gcc-arm-none-eabi/9.3.1-1/arm-none-eabi/include/c++/9.3.1/bits/hashtable.h:35,
                 from /home/pcook/.arduino15/packages/h2zero/tools/gcc-arm-none-eabi/9.3.1-1/arm-none-eabi/include/c++/9.3.1/unordered_map:46,
/home/pcook/.arduino15/packages/h2zero/tools/gcc-arm-none-eabi/9.3.1-1/arm-none-eabi/include/c++/9.3.1/limits:1818:7: error: 'constexpr' static data member 'max' must have an initializer
 1818 |       max() _GLIBCXX_USE_NOEXCEPT { return __LDBL_MAX__; }
      |       ^~~
/home/pcook/.arduino15/packages/h2zero/tools/gcc-arm-none-eabi/9.3.1-1/arm-none-eabi/include/c++/9.3.1/limits:1818:7: error: expected ';' at end of member declaration
 1818 |       max() _GLIBCXX_USE_NOEXCEPT { return __LDBL_MAX__; }
      |       ^~~
      |          ;

I hope this is just something I'm doing wrong. An identical project using the non-n-able core compiles without issue.

h2zero commented 1 year ago

Interesting, thanks for the report. I'll look into this asap.

pdcook commented 1 year ago

@h2zero Please let me know if there are any tests or other things I should try. I'm convinced this is just a mistake on my end, since it seems to happen when using anything from std.

For reference, here is the complete compilation log (--verbose, --warnings all, and --clean): https://gist.github.com/pdcook/5f69fe45a18a8325c1365848edcdc54e

h2zero commented 1 year ago

Looks like there is a redefinition of min and max in the "drivers" library. Is this a private lib?

pdcook commented 1 year ago

Yes it is, but I just did a quick cat * | grep min and the same for max and I haven't redefined them, or even used them as a variable name. At worst they are part of a variable name, or in a comment. Very strange, and stranger still that this compiles fine on the default Arduino core.

pdcook commented 1 year ago

Oh boy, I think I've found it: https://arduinojson.org/v6/error/macro-min-passed-3-arguments-but-takes-just-2/

Looks like Arduino.h and std::min conflict. I'll see if I can fix this by changing the order of the #includes.

pdcook commented 1 year ago

Reordering does partially fix things, but still produces compilation errors from min and max in h2zero/hardware/arm-ble/0.1.1/cores/nRF5/Arduino.h. Specifically the whole error is:

/home/pcook/.arduino15/packages/h2zero/tools/gcc-arm-none-eabi/9.3.1-1/arm-none-eabi/include/c++/9.3.1/bits/stl_bvector.h: In member function 'std::vector<bool, _Alloc>::size_type std::vector<bool, _Alloc>::_M_check_len(std::vector<bool, _Alloc>::size_type, const char*) const':
/home/pcook/.arduino15/packages/h2zero/hardware/arm-ble/0.1.1/cores/nRF5/Arduino.h:83:18: error: expected unqualified-id before '(' token
   83 | #define max(a,b) ((a)>(b)?(a):(b))
      |                  ^

It looks like this is related to an issue in an older version of Arduino-samd, which I know this repo is loosely based off of. Is the core up to date with the latest SAMD core?

pdcook commented 1 year ago

I have confirmed that the issue is indeed in the Arduino.h definition of min and max. Commenting those lines out results in error-free compilation.

h2zero commented 1 year ago

Awesome, any thoughts about a permanent solution? Not sure if they did anything in the samd core to deal with this.

pdcook commented 1 year ago

@h2zero It's unclear to me how exactly they fixed it in the samd core. Something to do with the underlying arduino-api. But here a permanent (albeit a bit sloppy) fix would just be to use the same

#ifdef min
#undef min
#endif

that the samd core uses for abs. However, for whatever reason #ifdef min is false! As is #ifdef of any substring of min! So this doesn't work. Simply commenting out the definitions of min and max does, but that can't be a reasonable solution.

h2zero commented 1 year ago

@pdcook Sorry for the delay on this, I have created PR #15 which implements the fix from the arduinocore-api. If you could test your code with that PR and let me know if it works for you I would greatly appreciate it.