arduino / Arduino

Arduino IDE 1.x
https://www.arduino.cc/en/software
Other
14.12k stars 7k forks source link

Use GCC binary constants format #3561

Open mikaelpatel opened 9 years ago

mikaelpatel commented 9 years ago

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.

cousteaulecommandant commented 7 years ago

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 commented 7 years ago

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.

per1234 commented 3 years ago

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:

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

cousteaulecommandant commented 3 years ago

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.)