-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Fix a few #ifdef vs. #if issues #66898
Conversation
Tagging subscribers to this area: @tommcdon Issue DetailsAlso upstreamed a fix for libunwind libunwind/libunwind#342 (but we are not using ptrace so it can wait until the next libunwind update). cc @akoeplinger, @jkotas
|
Ah, actually we are compiling |
@@ -42,9 +42,9 @@ SET_DEFAULT_DEBUG_CHANNEL(DEBUG); // some headers have code with asserts, so do | |||
#include <unistd.h> | |||
#if HAVE_PROCFS_CTL | |||
#include <unistd.h> | |||
#elif HAVE_TTRACE // HAVE_PROCFS_CTL | |||
#elif defined(HAVE_TTRACE) // HAVE_PROCFS_CTL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason why we use cmakedefine/defined here over the existing check? The other cases out of libunwind make sense as they were always defined and they are "HAVE" remarks. Is this because libunwind uses this pattern more often?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, correct.
It is actually related to config.h.in change, we are using same define (pr: #cmakedefine HAVE_TTRACE
, main: #cmakedefine01 HAVE_TTRACE
) for libunwind and debug.cpp. So I converged it to libunwind's way.
When we use #cmakedefine01 FOO
, it generates #define FOO 1
(when feature is detected) or #define FOO 0
(when detection fails), so defined(FOO)
is always true and it is a logical error if we use that. With #cmakedefine FOO
, it generates #define FOO
or nothing when detection fails. In that case #if FOO
issues a warning Wundef
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just wanted to know why use the define divergent when everything uses the logical one. If it's for libunwind consistency that's all good.
Also upstreamed a fix for libunwind libunwind/libunwind#342 (but we are not using ptrace so it can wait until the next libunwind update).
Also upstreamed a fix for libunwind libunwind/libunwind#342 (but we are not using ptrace so it can wait until the next libunwind update).
cc @akoeplinger, @jkotas