Skip to content

Deprecate binary.h in favor of binary literals #7064

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

cousteaulecommandant
Copy link
Contributor

Replace macros in binary.h with a single enum.
Make all Bxxxx constants trigger a "deprecated" warning (if supported),
indicating to use 0bxxxx instead (supported since C++14 and GCC 4.3).
Addresses issue #3561

(It seems that __attribute__ ((__deprecated__)) on enum values is not supported until GCC 6.1; in this case the warning will not be displayed)

@facchinm facchinm added Architecture: AVR Applies only to the AVR microcontrollers (Uno, etc.) Component: Core Related to the code for the standard Arduino API c++14 Related to switching to use of the C++14 standard labels Jan 8, 2018
@cousteaulecommandant
Copy link
Contributor Author

Just to clarify. This header requires neither C++14 nor GCC 6 to work. Therefore, it can already be used, even before C++14 is adopted.
Also, even if C++14 is not enabled, 0b integer constants have been supported as a GCC extension since v4.3.

Additionally, the header performs some version checks in order to display an optional warning:

  • if C++14 is enabled, it will display a warning (using C++11's [[deprecated]] mechanism) saying to use C++14's 0b
  • if GCC version > 6, it will display a warning (using GCC 6's __attribute__ ((__deprecated__)) for enums, which doesn't work on enums on older GCC versions) saying to use GCC 4.3's 0b
  • if neither C++14 nor GCC>6 are present, it will display no warning message (either because it can't or because 0b cannot be used and therefore the warning would be a lie), but nevertheless it will still replace those evil macros with a nice and safe enum (that does not use 0b at all).

In any case, this header does not use 0b constants itself, only plain decimal ones; it is mainly a replacement of macros with an enum. The aforementioned check is only to determine whether or not to display the warning about Bxx being deprecated.

(In retrospect, maybe I should have added a comment line in the code before all those #ifs to clarify what they actually did; something like /* If the compiler supports 0b binary literals, it is preferable to use them instead -- warn the user (if possible) */)

Make all Bxxxx constants trigger a "deprecated" warning (if supported),
indicating to use 0bxxxx instead (supported since C++14 and GCC 4.3).
This warning will not be displayed for C++<14 (or C) and GCC<4.3.
@cousteaulecommandant
Copy link
Contributor Author

I have split this PR into two commits:

  1. Replace the macros with a single enum.
  2. Add the deprecated attribute to these constants in order to promote the use of 0bxx literals.

In my opinion, this PR could be pulled directly. However, if there are plans to slowly move away from macros in the Arduino library, and you don't think it is a good idea to mark Bxx constants as deprecated yet, I could make a separate PR with just the first commit.

PS: the second commit will have no effect with the default settings, since all warnings are suppressed.
PS2: Bxx constants are not used anywhere in this repository, so there should be no risk of getting a lot of warnings when compiling a sketch with warnings enabled if that sketch doesn't use these constants.

@facchinm facchinm self-assigned this May 29, 2018
@facchinm
Copy link
Member

Already applied in arduino/ArduinoCore-API@8bcc446 , closing here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Architecture: AVR Applies only to the AVR microcontrollers (Uno, etc.) c++14 Related to switching to use of the C++14 standard Component: Core Related to the code for the standard Arduino API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants