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

C++ API, DEBUG symbol conflict #4151

Closed
adujardin opened this issue Nov 6, 2023 · 3 comments · Fixed by #4152
Closed

C++ API, DEBUG symbol conflict #4151

adujardin opened this issue Nov 6, 2023 · 3 comments · Fixed by #4152
Labels
🪳 bug Something isn't working 🌊 C++ API C or C++ logging API
Milestone

Comments

@adujardin
Copy link

Describe the bug
This is technically not a bug but when trying to integrate rerun C++ API into a pretty big C/C++ codebase (with third-party dependencies) the compilation fails because some file already defined "DEBUG" as a macro. While a questionable choice, it's unfortunately pretty common in C code. The compilation fails at

static const TextLogLevel DEBUG;

with an error like:

text_log_level.hpp:54:26: error: declaration does not declare anything [-fpermissive]
   54 |             static const TextLogLevel DEBUG;
      |                          ^~~~~~~~~~~~

One quick and dirty way to fix or at least to confirm this, is something like this is in the same header:

#ifdef DEBUG
#undef DEBUG
#endif

Another common solution is to not use "DEBUG" and instead DBG, _DBG, etc

Desktop:

  • OS: Ubuntu GCC 9.4, although it's not really relevant here

Rerun version
0.10

Additional context

@adujardin adujardin added 👀 needs triage This issue needs to be triaged by the Rerun team 🪳 bug Something isn't working labels Nov 6, 2023
@Wumpf Wumpf added 🌊 C++ API C or C++ logging API and removed 👀 needs triage This issue needs to be triaged by the Rerun team labels Nov 6, 2023
@Wumpf Wumpf added this to the 0.11 C++ polish milestone Nov 6, 2023
@Wumpf
Copy link
Member

Wumpf commented Nov 6, 2023

Good catch. DEBUG as name is an identifier is very unfortunate indeed, we'll have to put something else in there.
DBG and _DBG are defines I've seen in the past as well, so we'll have to do something else, probably some post/prefix 🤔

@emilk
Copy link
Member

emilk commented Nov 6, 2023

Adding #undef DEBUG implies #include <rerun.h> would un-define DEBUG. That means it may break user code that relies on that macro may break.

Other variations of DEBUG such as DEBUG_ DBG etc are also likely to be macros.

An alternative solution is to use PascalCase for constants instead, as those are much less likely to be macros, i.e. change this to static const TextLogLevel Debug; and TextLogLevel::Debug.

Another benefit of the PascalCase convention is that it is closer to Rust.

@emilk emilk added 🏎️ Quick Issue Can be fixed in a few hours or less and removed 🏎️ Quick Issue Can be fixed in a few hours or less labels Nov 6, 2023
@Wumpf
Copy link
Member

Wumpf commented Nov 6, 2023

Fixed by #4152

@Wumpf Wumpf closed this as completed Nov 6, 2023
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 or C++ logging API
Projects
None yet
3 participants