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

HAVE_CXX11_ATOMIC macro should be defined somewhere in headerfiles #667

Closed
xq114 opened this issue Jun 7, 2021 · 1 comment · Fixed by #671
Closed

HAVE_CXX11_ATOMIC macro should be defined somewhere in headerfiles #667

xq114 opened this issue Jun 7, 2021 · 1 comment · Fixed by #671
Assignees
Labels
Milestone

Comments

@xq114
Copy link

xq114 commented Jun 7, 2021

glog/src/glog/logging.h.in

Lines 102 to 106 in c1499f6

#ifdef HAVE_CXX11_ATOMIC
#include <atomic>
#elif defined(OS_WINDOWS)
#include <Windows.h>
#endif

Here HAVE_CXX11_ATOMIC is defined in config.h which is generated and used in the build stage, while the macro is missing when the header is included by some other projects. Since the user holds no information about whether or not the macro is defined in the build stage, I think it's necessary to enable the feature by checking @ac_cv_have_cxx11_atomic@ rather then by checking macro definition. At least, the macro should be defined somewhere such as export.h.

Moreover, it has caused incorrect inclusion of windows.h due to that OS_WINDOWS is a commonly defined macro. It's really a torture when debugging for it.

@sergiud sergiud added the bug label Jun 9, 2021
@sergiud
Copy link
Collaborator

sergiud commented Jun 9, 2021

Missing HAVE_CXX11_ATOMIC definition is indeed an error. However, I'm not sure I understand this part:

Moreover, it has caused incorrect inclusion of windows.h due to that OS_WINDOWS is a commonly defined macro. It's really a torture when debugging for it.

What do you mean by "commonly defined"? Is there an OS_WINDOWS definition with a completely different semantic? Can you provide an example?

sergiud added a commit to sergiud/glog that referenced this issue Jun 15, 2021
@sergiud sergiud linked a pull request Jun 15, 2021 that will close this issue
@sergiud sergiud self-assigned this Jun 15, 2021
@sergiud sergiud added this to the 0.6 milestone Jun 15, 2021
sergiud added a commit to sergiud/glog that referenced this issue Jun 15, 2021
sergiud added a commit to sergiud/glog that referenced this issue Jun 15, 2021
sergiud added a commit that referenced this issue Jun 16, 2021
cmake: export `<atomic>` availability (fixes #667)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants