max() macro does not play well with std::max()

[jgfoster]

If I include <Arduino.h> before <vector>, then I get an error in stl_vector.h:1645 where the code refers to std::max().

	const size_type __len = size() + std::max(size(), __n);

This appears to be due to a macro definition of max() in arduino_ci-0.3.0/cpp/arduino/AvrMath.h causing the code to be expanded to:

	const size_type __len = size() + std::((size())>(__n)?(size()):(__n));

This produces error: expected unqualified-id before ‘(’ token (which makes sense).

My research suggests that this sort of use of macros should be avoided.

Unit testing BaseClass.cpp with g++... 

Last command:  $ g++ -std=c++0x -o /home/runner/work/LiquidCrystal/LiquidCrystal/unittest_BaseClass.cpp.bin -DARDUINO=100 -g -O1 -fno-omit-frame-pointer -fno-optimize-sibling-calls -fsanitize=address -D__AVR_ATmega2560__ -I/home/runner/.rubies/ruby-2.6.6/lib/ruby/gems/2.6.0/gems/arduino_ci-0.3.0/cpp/arduino -I/home/runner/.rubies/ruby-2.6.6/lib/ruby/gems/2.6.0/gems/arduino_ci-0.3.0/cpp/unittest -I/home/runner/work/LiquidCrystal/LiquidCrystal/src /home/runner/.rubies/ruby-2.6.6/lib/ruby/gems/2.6.0/gems/arduino_ci-0.3.0/cpp/arduino/Arduino.cpp /home/runner/.rubies/ruby-2.6.6/lib/ruby/gems/2.6.0/gems/arduino_ci-0.3.0/cpp/arduino/Godmode.cpp /home/runner/.rubies/ruby-2.6.6/lib/ruby/gems/2.6.0/gems/arduino_ci-0.3.0/cpp/arduino/stdlib.cpp /home/runner/.rubies/ruby-2.6.6/lib/ruby/gems/2.6.0/gems/arduino_ci-0.3.0/cpp/unittest/ArduinoUnitTests.cpp /home/runner/work/LiquidCrystal/LiquidCrystal/src/LiquidCrystal.cpp /home/runner/work/LiquidCrystal/LiquidCrystal/src/LiquidCrystal_CI.cpp /home/runner/work/LiquidCrystal/LiquidCrystal/test/BaseClass.cpp

In file included from /home/runner/.rubies/ruby-2.6.6/lib/ruby/gems/2.6.0/gems/arduino_ci-0.3.0/cpp/arduino/WString.h:8:0,
                 from /home/runner/.rubies/ruby-2.6.6/lib/ruby/gems/2.6.0/gems/arduino_ci-0.3.0/cpp/arduino/Arduino.h:13,
                 from /home/runner/work/LiquidCrystal/LiquidCrystal/src/LiquidCrystal.h:4,
                 from /home/runner/work/LiquidCrystal/LiquidCrystal/src/LiquidCrystal_CI.h:2,
                 from /home/runner/work/LiquidCrystal/LiquidCrystal/src/LiquidCrystal_CI.cpp:1:
/usr/include/c++/7/bits/stl_vector.h: In member function ‘std::vector<_Tp, _Alloc>::size_type std::vector<_Tp, _Alloc>::_M_check_len(std::vector<_Tp, _Alloc>::size_type, const char*) const’:
/home/runner/.rubies/ruby-2.6.6/lib/ruby/gems/2.6.0/gems/arduino_ci-0.3.0/cpp/arduino/AvrMath.h:20:18: error: expected unqualified-id before ‘(’ token
 #define max(a,b) ((a)>(b)?(a):(b))
                  ^
/usr/include/c++/7/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/runner/.rubies/ruby-2.6.6/lib/ruby/gems/2.6.0/gems/arduino_ci-0.3.0/cpp/arduino/AvrMath.h:20:18: error: expected unqualified-id before ‘(’ token
 #define max(a,b) ((a)>(b)?(a):(b))
                  ^
...Unit testing BaseClass.cpp with g++                                         ✗

But I need to do the includes in this order. If I include them in the other order then I get an error that <vector> isn't available in the Arduino compile (I have a check for MOCK_PINS_COUNT to leave out non-test code in the Arduino compiles).

The workaround is to #undef max (see line 6).

[matthijskooijman]

An alternative would be to use a template function rather than a macro, at least in C++. For C, this won't work and you'll still need to macro, but in C (lacking namespaces, overloading and methods) this is not so likely to be problematic.

This was already applied in the ArduinoCore-API repo, so that's probably easy to just copy: https://github.com/arduino/ArduinoCore-API/blob/45e4e5aba6ad1a5b67026b686099cc0fce437cdc/api/Common.h#L120-L147

Actually, this is partly a duplicate of #133, and I just realized I already provided a fix for this in #144. Maybe you could test that and see if it helps?

[jgfoster]

Using the alternative macro gives a similar error. Essentially it translates

std::max(a, b)

into

std::({typeof(a) _a = (a); typeof(b) _b = (b); _a > _b ? _a : _b;})

Which still gives error: expected unqualified-id before ‘(’ token.

[matthijskooijman]

But the code in my PR applies the alternative macro only for C code, for c++ code it uses a template function?

[jgfoster]

Yes, I see that now. I've updated my code using this approach and it now passes the tests. I'll close this as a duplicate of #133.

[jgfoster]

I said that I updated my code but this keeps happening and I don't remember what I changed. Well, it is in LiquidCrystal_CI.h.