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

enum SortingStatus::Unsorted conflicts with define in X.h #7846

Closed
rgolovanov opened this issue Oct 21, 2024 · 2 comments · Fixed by #7855
Closed

enum SortingStatus::Unsorted conflicts with define in X.h #7846

rgolovanov opened this issue Oct 21, 2024 · 2 comments · Fixed by #7855
Assignees
Labels
🪳 bug Something isn't working 🌊 C++ API C/C++ API specific

Comments

@rgolovanov
Copy link
Contributor

Describe the bug
The attempt to compile C++ project with X11 leads to an error

To Reproduce
Steps to reproduce the behavior:
Just run compilation of the project which includes <rerun.hpp>

[build] In file included from /usr/include/X11/Xlib.h:44,
[build]                  from /<...>/include/vulkan/vulkan.h:58,
[build] /<...>/include/rerun/time_column.hpp:27:9: error: expected identifier before numeric constant
[build]    27 |         Unsorted = 2,

Expected behavior
No compiler errors

Screenshots
image

Desktop (please complete the following information):

  • OS: Linux 5e5b334a2327 5.15.153.1-microsoft-standard-WSL2 #1 SMP Fri Mar 29 23:14:13 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux

Rerun version
0.19.0

@rgolovanov rgolovanov added 👀 needs triage This issue needs to be triaged by the Rerun team 🪳 bug Something isn't working labels Oct 21, 2024
@Wumpf Wumpf added this to the Next patch release milestone Oct 21, 2024
@Wumpf Wumpf added 🌊 C++ API C/C++ API specific and removed 👀 needs triage This issue needs to be triaged by the Rerun team labels Oct 21, 2024
@emilk
Copy link
Member

emilk commented Oct 21, 2024

We can't protect against all weird macros that other libraries might set. But we should at least try to avoid the most common problems, and X11 is pretty common.

Solution alternatives

I think B is the right call.

A) do nothing

Users will have to carefully order their #include:s, and/or add #undef Unsorted to their code.

Status quo.

This is a problem in two alternatives:

A) whenever a user includes X.h BEFORE rerun.hpp
B) whenever a user includes both rerun.hpp and X.h and then wants to use rerun::SortingStatus::Unsorted

Case A is too common.

B) Add #undef Unsorted to time_column.hpp

This will work until someone does these things in this exact order:

  • #include <rerun.hpp>
  • #include <X.h>
  • Want to use rerun::SortingStatus::Unsorted

…but that is a pretty narrow problem now, and has a clear workaround (add #undef Unsorted to your user code).

C) Rename Unsorted

We can call it Unsorted_ in C++, but keep the Unsorted name in other APIs. Or call it Nonsorted everywhere.
But now it feels like we're negotiating with terrorists. What other words are we not allowed to use? 😠

@emilk emilk self-assigned this Oct 21, 2024
@emilk
Copy link
Member

emilk commented Oct 21, 2024

I wonder if we should just codegen #undef Identifier for every identifier that appears in each of our header files…

Image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🪳 bug Something isn't working 🌊 C++ API C/C++ API specific
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants