Skip to content
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

Deprecate PI #250

Merged
merged 1 commit into from
Jun 24, 2024
Merged

Deprecate PI #250

merged 1 commit into from
Jun 24, 2024

Conversation

chiphogg
Copy link
Contributor

If we have anything called PI --- even if it is properly encapsulated
inside a C++ namespace! --- then users will be unable to use our library
if they also depend on any library that defines a macro called PI,
in any namespace (because macros do not care about namespaces).

Of course, defining a macro named PI would be an execrable and
amateurish violation of long-accepted best practices for C++ software
engineering. However, at least one library does this: the Arduino core
library. Therefore, if we want to support it --- which we most
certainly do --- then we will have to yield.

The plan going forward is to release this deprecation as part of 0.3.5,
and then fully delete the variable as part of 0.4.0. In the meantime,
users can still get Au to work; they just need to make sure that they
include Au after any files that --- lamentably --- define a macro
PI.

We have other "short all-caps" identifiers, ONE and ZERO. We could
get rid of ONE with no trouble at all; mag<1>() is a fine
replacement. Getting rid of ZERO would be extremely painful, though.
The motivation to define a ZERO macro is far less than for PI, so I
am planning to simply hope that no library of note has done this, and
expecting to tell users of any such library that they are simply out of
luck.

Fixes #246.

If we have anything called `PI` --- even if it is properly encapsulated
inside a C++ namespace! --- then users will be unable to use our library
if they also depend on _any library_ that defines a macro called `PI`,
in _any_ namespace (because macros _do not care_ about namespaces).

Of course, defining a macro named `PI` would be an execrable and
amateurish violation of long-accepted best practices for C++ software
engineering.  However, at least one library does this: the Arduino core
library.  Therefore, if we want to support it --- which we most
certainly do --- then we will have to yield.

The plan going forward is to release this deprecation as part of 0.3.5,
and then fully delete the variable as part of 0.4.0.  In the meantime,
users can still get Au to work; they just need to make sure that they
include Au _after_ any files that --- _lamentably_ --- define a macro
`PI`.

We have other "short all-caps" identifiers, `ONE` and `ZERO`.  We could
get rid of `ONE` with no trouble at all; `mag<1>()` is a fine
replacement.  Getting rid of `ZERO` would be extremely painful, though.
The motivation to define a `ZERO` macro is far less than for `PI`, so I
am planning to simply hope that no library of note has done this, and
expecting to tell users of any such library that they are simply out of
luck.

Fixes #246.
@chiphogg chiphogg added the release notes: 🗑️ lib (deprecation) PR deprecating library functionality label Jun 23, 2024
@chiphogg chiphogg requested a review from geoffviola June 23, 2024 20:34
Copy link
Contributor

@geoffviola geoffviola left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable. We can explore other options in the future.

@chiphogg chiphogg merged commit 50de6e9 into main Jun 24, 2024
10 checks passed
@chiphogg chiphogg deleted the chiphogg/deprecate-pi#246 branch June 24, 2024 14:47
chiphogg added a commit that referenced this pull request Aug 12, 2024
It conflicts with the `pascal` macro in `<Windows.h>`.  Yes, they really
did define a lowercase-named macro in `<Windows.h>`.

As with `PI` (#247), the `#ifndef` should immediately unblock most
users, but it's not a long-term solution, because it depends on
order-of-includes.  Therefore, we also deprecate `pascal` (as with `PI`
in #250).  I think a good rule of thumb is that deprecated constructs
should be deprecated for at least one full minor release cycle, so we'll
plan to delete it (along with `PI`) as part of 0.5.0.

Fixes #284.
@chiphogg chiphogg mentioned this pull request Aug 12, 2024
chiphogg added a commit that referenced this pull request Aug 12, 2024
It conflicts with the `pascal` macro in `<Windows.h>`.  Yes, they really
did define a lowercase-named macro in `<Windows.h>`.

As with `PI` (#247), the `#ifndef` should immediately unblock most
users, but it's not a long-term solution, because it depends on
order-of-includes.  Therefore, we also deprecate `pascal` (as with `PI`
in #250).  I think a good rule of thumb is that deprecated constructs
should be deprecated for at least one full minor release cycle, so we'll
plan to delete it (along with `PI`) as part of 0.5.0.

Fixes #284.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release notes: 🗑️ lib (deprecation) PR deprecating library functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

naming conflict in Arduino causes compilation errors
2 participants