-
Notifications
You must be signed in to change notification settings - Fork 641
qualify isnan() with std namespace #565
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
base: master
Are you sure you want to change the base?
Conversation
When a program that depends on implot includes <implot.h>, we have this sequence of includes: <implot.h> -> <implot_internal.h> -> <imgui_internal.h> -> <math.h> Inside <implot_internal.h>'s source code, there's an unqualified call to isnan(). By doing so, it relies on <imgui_internal.h> having isnan() in the global namespace, which <math.h> is guaranteed to do. However, it's possible that implot's user included <cmath> first, which is not guaranteed to put isnan() in the global namespace, and may prevent a later <math.h> from putting isnan() in the global namespace. On NetBSD 10.0 i386, <cmath> doesn't put isnan() in the global namespace and prevents <math.h> from doing so, causing a compile error. Qualify the call to fix it.
The NetBSD port will only work after epezent/implot#565 is merged and Dolphin updates to a version with the fix. In the meantime, you can apply the fix yourself to a Dolphin checkout.
The NetBSD port will only work after epezent/implot#565 is merged and Dolphin updates to a version with the fix. In the meantime, you can apply the fix yourself to a Dolphin checkout.
|
@guijan is this still an issue with the latest ImGui? I thought |
|
I can confirm that this still happens with master. I just came across this issue while trying to cross-compile from x64 to ARM64 on Linux while using glibc in particular. |
|
Oh... I've moved the issue to high priority :) Are you able to reproduce the issue by modifying the |
|
I've narrowed it down quite a lot, so much so that I can place the reproduction program here: #include <cmath>
#include "implot.h"
#include "implot_internal.h"
int main() { return 0; }Here is what happens when I try to compile this program using glibc and gcc/g++ from x64 to ARM64: Log OutputIt seems including cmath before implot_internal.h causes the issue to show up. This issue does not happen when compiling without a sysroot and cross-compiler, though. |
| static inline int ImPosMod(int l, int r) { return (l % r + r) % r; } | ||
| // Returns true if val is NAN | ||
| static inline bool ImNan(double val) { return isnan(val); } | ||
| static inline bool ImNan(double val) { return std::isnan(val); } |
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.
Looks like this problem is compiler-specific. Can you change it to the below? Then we can merge it.
| static inline bool ImNan(double val) { return std::isnan(val); } | |
| static inline bool ImNan(double val) { return (val != val); } |
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.
return (val != val);? Doesn't this always evaluates to false?
Yep, that looks pretty narrow to me... |
See the commit message for details. I've tested this change on NetBSD only.
Found while porting dolphin-emu/dolphin to NetBSD: