Arduino icon indicating copy to clipboard operation
Arduino copied to clipboard

Use GCC binary constants format

Open mikaelpatel opened this issue 9 years ago • 4 comments

https://gcc.gnu.org/onlinedocs/gcc/Binary-constants.html

This would allow a simpler solution to binary number constants and the file binary.h https://github.com/arduino/Arduino/blob/master/hardware/arduino/avr/cores/arduino/binary.h becomes unnecessary.

Motivation: Standard, less complex, portability, reduce error risk (unsigned/signed/number size), reduce source code size.

Drawbacks: Large ripple effect on code that uses the Arduino "binary" format.

mikaelpatel avatar Jul 21 '15 16:07 mikaelpatel

In general, I think this is a good idea; I'd suggest changing all documentation and existing code to 0b style. However I'd leave the binary.h header for backwards compatibility.


Motivation: Standard, less complex, portability, reduce error risk (unsigned/signed/number size), reduce source code size.

~Not quite standard; it's a GCC extension.~ standard in C++14 ~Only portable to platforms using GCC (or other compilers supporting this extension, e.g. Clang).~ or C++14 Less complex indeed (why do all "numeric constants" start with a digit except binary? why do these look like variable names?). Also, B- style is limited to 8 bits, whereas 0b- can be arbitrarily long (up to 64 bits, I guess).

Drawbacks: Large ripple effect on code that uses the Arduino "binary" format.

I think the binary.h header should be kept just in case.

If anything, Arduino could add an __attribute__ ((__deprecated__)) to these old macros to trigger warnings if used (and warnings are enabled). ~New GCC versions allow putting this attribute to enum constants (not macros), like~

enum {
  B0 __attribute__ ((__deprecated__ ("use 0b0 instead"))) = 0,
  B1 __attribute__ ((__deprecated__ ("use 0b1 instead"))) = 1,
  ...
}

~but I'm not sure if the current GCC version used by Arduino does.~ In any case, I'm not even sure it's a good idea to make this header even deprecated since LOTS of code use it.

cousteaulecommandant avatar Nov 28 '16 16:11 cousteaulecommandant

Not quite standard; it's a GCC extension.

I stand corrected; it seems to be standard in C++14. (Arduino uses C++11, but GCC allows that too.)

New GCC versions allow putting this attribute to enum constants (not macros)

...but it seems the current GCC used in Arduino doesn't, so forget it.

cousteaulecommandant avatar Dec 31 '16 17:12 cousteaulecommandant

I did a survey of all the official Arduino firmware repositories (platforms, libraries, sketches), as well as the Arduino Language Reference and the Tutorials documentation content (so the most significant missing part is the Libraries reference pages) and found the only incidences of use of the Binary.h macros here:

  • https://github.com/arduino-libraries/SigFox/blob/d5bad3801ddcd6582e9a512152ff8ade16ae1fc7/src/SigFox.cpp#L380
  • https://github.com/arduino-libraries/RobotIRremote
    • This has been archived, so there is no possibility of changing it
  • https://www.arduino.cc/en/Reference/PortManipulation
  • ~~https://www.arduino.cc/en/Tutorial/ShftIn13~~
  • ~~https://www.arduino.cc/en/Tutorial/ShftIn22~~
  • ~~https://www.arduino.cc/reference/en/language/structure/bitwise-operators/bitwiseand/~~ (fixed by https://github.com/arduino/reference-en/pull/793)
  • ~~https://www.arduino.cc/reference/en/language/structure/bitwise-operators/bitwiseor/~~ (fixed by https://github.com/arduino/reference-en/pull/793)
  • ~~https://www.arduino.cc/reference/en/language/structure/bitwise-operators/bitwisexor/~~ (fixed by https://github.com/arduino/reference-en/pull/793)
  • ~~https://www.arduino.cc/reference/en/language/structure/compound-operators/compoundbitwiseand/~~ (fixed by https://github.com/arduino/reference-en/pull/793)
  • ~~https://www.arduino.cc/reference/en/language/structure/compound-operators/compoundbitwiseor/~~ (fixed by https://github.com/arduino/reference-en/pull/793)
  • ~~https://www.arduino.cc/reference/en/language/structure/compound-operators/compoundbitwisexor/~~ (fixed by https://github.com/arduino/reference-en/pull/793)
  • ~~https://www.arduino.cc/reference/en/language/variables/constants/integerconstants/~~ (fixed by https://github.com/arduino/reference-en/pull/793)

I also note that steps are being taken to deprecate these macros: https://github.com/arduino/ArduinoCore-API/commit/8bcc446b918f8579a39d493a830668a14e6cf93e

per1234 avatar Oct 18 '20 18:10 per1234

Yep, those should be changed as well. Both the B0b and removing the 8-bit limit. The language reference is in https://github.com/arduino/reference-en/ (for example, the file for integer constants is in https://github.com/arduino/reference-en/blob/master/Language/Variables/Constants/integerConstants.adoc). The tutorials, I have no idea but they should be easy to find as well. ~~If you don't have time I can try to change this myself. Should I send a single PR for all the files, or one per file?~~ → EDIT: Done! (For the reference only; the tutorials and libraries still need to be changed.)

(As a side note, integerconstants has also a couple of conceptual errors re: the type of integer constants if you don't specify L, but I think it's better to change those in a separate PR.)

cousteaulecommandant avatar Oct 25 '20 17:10 cousteaulecommandant