Skip to content

Do not expose public macros with common names#531

Closed
glebm wants to merge 1 commit into
devkitPro:masterfrom
glebm:no-macros
Closed

Do not expose public macros with common names#531
glebm wants to merge 1 commit into
devkitPro:masterfrom
glebm:no-macros

Conversation

@glebm

@glebm glebm commented Sep 11, 2023

Copy link
Copy Markdown
Contributor

The prevents compatibility issues with other libraries, such as the one seen in #530

Fixes #530

The prevents compatibility issues with other libraries, such as the one seen in devkitPro#530

Fixes devkitPro#530
@fincs

fincs commented Sep 11, 2023

Copy link
Copy Markdown
Member

While we understand the frustration and the rationale behind this PR, unfortunately we cannot accept it because this would entail a major breaking change, specifically the removal of the commonly used BIT() macro.

@fincs fincs closed this Sep 11, 2023
@glebm

glebm commented Sep 11, 2023

Copy link
Copy Markdown
Contributor Author

Well, my PR to libfmt also got closed fmtlib/fmt#3633 😅
@vitaut

@glebm

glebm commented Sep 12, 2023

Copy link
Copy Markdown
Contributor Author

@fincs Would you accept a PR that removes or prefixes macros other than BIT?

glebm added a commit to glebm/libctru that referenced this pull request Sep 12, 2023
The mitigates compatibility issues with other libraries, such as the one seen in devkitPro#530.

The `BIT` macro is kept because it is commonly used, so removing it
would be too breaking of a change (see
devkitPro#531 (comment))

Fixes devkitPro#530
glebm added a commit to glebm/libctru that referenced this pull request Sep 12, 2023
The mitigates compatibility issues with other libraries, such as the one seen in devkitPro#530.

The `BIT` macro is kept because it is commonly used, so renaming it
would be too breaking of a change (see
devkitPro#531 (comment))

Fixes devkitPro#530
@WinterMute

Copy link
Copy Markdown
Member

Well, my PR to libfmt also got closed fmtlib/fmt#3633 😅 @vitaut

Well, what you can do is just not include libctru headers in the same file as libfmt headers. You might want to point out to libfmt maintainers that "PACKED" is most commonly used for our precise use case & not theirs. See https://github.com/search?q=%22%23define+PACKED+%22&type=code for instance.

It seems rather late in the day to be making intrusive & breaking changes like this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Global macros clash with other things

3 participants