PowerBroker2 / SafeString

This SafeString library is designed for beginners to be a safe, robust and debuggable replacement for string processing in Arduino. Note, this is NOT my work, I am simply hosting it for easy access. The original code belongs to Forward Computing and Control Pty. Ltd.
https://www.forward.com.au/pfod/ArduinoProgramming/SafeString/index.html
38 stars 12 forks source link

Fix warnings about `F` redefinition on ESP32 (and elsewhere?) #68

Closed egnor closed 6 months ago

egnor commented 1 year ago

Compiling SafeString on many architectures generates a lot of warnings about redefining the F macro.

From the code:

//#ifndef F  // Arduino IDE 2+ adds namespace around this macro so need to define it here also
// ESP32 defines a macro F(s) (s), so will get a redefinition warning which can be ignored
#define F(string_literal) (reinterpret_cast<const __FlashStringHelper *>(PSTR(string_literal)))
//#endif

IMVHO, a comment saying "this warning can be ignored" is not a great solution-- many of us like our compilation to be warning-clean as a matter of good practice, and having to somehow remember that this (repeated) warning is OK makes it a lot easier for other warnings to slip through the cracks.

I don't understand the comment "Arduino IDE 2+ adds namespace around this macro". The macro is part of the Arduino runtime, not part of the IDE, macros can't be namespaced in any version of C++, I don't see any namespaces involved in the AVR runtime's definition of F(...), and I can't find that do-nothing definition for ESP32 or any other platform?? Probably I misunderstand something??

I'm not actually sure why this (re)definition has to be there at all?

PowerBroker2 commented 1 year ago

@drmpf

egnor commented 1 year ago

(By the way, I'm happy to make a PR if desired, just let me know what solution is preferred!)

drmpf commented 1 year ago

The Arduino SAMD Boads install defines class __FlashStringHelper; inside the arduino namespace in String.h No warnings when compiling for one of those boards on Default compiler warnings with V4.1.28

Looks like if the SafeStringNameSpaceStart.h etc files have suitable #define checks there is no issue removing the F(string) define.

So I will remove

//#ifndef F  // Arduino IDE 2+ adds namespace around this macro so need to define it here also
// ESP32 defines a macro F(s) (s), so will get a redefinition warning which can be ignored
#define F(string_literal) (reinterpret_cast<const __FlashStringHelper *>(PSTR(string_literal)))
//#endif

in the next release and see if anyone complains about things breaking on one of many boards I don't have on hand to test against. In the mean time you can just try deleting those lines yourself to check for your own boards.