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.

Sign up for GitHub

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

Jump to bottom

qualify isnan() with std namespace #565

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

qualify isnan() with std namespace #565

wants to merge 1 commit into from

Conversation

guijan
Copy link

@guijan guijan commented Apr 20, 2024

See the commit message for details. I've tested this change on NetBSD only.

Found while porting dolphin-emu/dolphin to NetBSD:

[159/402] Building CXX object Externals/implot/CMakeFiles/implot.dir/implot/implot_items.cpp.o
samu: job failed: /usr/bin/c++ -DAUTOUPDATE=1 -DDATA_DIR=\"/usr/local/share/dolphin-emu/\" -DHAVE_EGL=1 -DHAVE_X11=1 -DHAVE_XRANDR=1 -DUSE_ANALYTICS=1 -DUSE_MEMORYWATCHER=1 -DUSE_PIPES=1 -D_A
RCH_32=1 -D_DEBUG -D_DEFAULT_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_M_GENERIC=1 -I/home/jan/Work/dolphin/Source/Core -I/home/jan/Work/dolphin/Externals/implot/implot -I/home/jan
/Work/dolphin/Externals/implot/imgui -I/home/jan/Work/dolphin/Externals/imgui -I/home/jan/Work/dolphin/Externals/fmt/fmt/include -g -std=c++2a -fdiagnostics-color -fno-strict-aliasing -fno-ex
ceptions -fvisibility-inlines-hidden -fvisibility=hidden -ggdb -w -MD -MT Externals/implot/CMakeFiles/implot.dir/implot/implot_items.cpp.o -MF Externals/implot/CMakeFiles/implot.dir/implot/im
plot_items.cpp.o.d -o Externals/implot/CMakeFiles/implot.dir/implot/implot_items.cpp.o -c /home/jan/Work/dolphin/Externals/implot/implot/implot_items.cpp
In file included from /home/jan/Work/dolphin/Externals/implot/implot/implot_items.cpp:27:
/home/jan/Work/dolphin/Externals/implot/implot/implot_internal.h: In function 'bool ImNan(double)':
/home/jan/Work/dolphin/Externals/implot/implot/implot_internal.h:123:47: error: 'isnan' was not declared in this scope; did you mean 'std::isnan'?
  123 | static inline bool ImNan(double val) { return isnan(val); }
      |                                               ^~~~~
      |                                               std::isnan
In file included from /home/jan/Work/dolphin/Externals/fmt/fmt/include/fmt/format.h:36,
                 from /home/jan/Work/dolphin/Source/Core/Common/Logging/Log.h:7,
                 from /home/jan/Work/dolphin/Source/Core/Common/Assert.h:8,
                 from /home/jan/Work/dolphin/Externals/imgui/imconfig.h:17,
                 from /home/jan/Work/dolphin/Externals/imgui/imgui.h:58,
                 from /home/jan/Work/dolphin/Externals/implot/implot/implot.h:48,
                 from /home/jan/Work/dolphin/Externals/implot/implot/implot_items.cpp:26:
/usr/include/g++/cmath:632:5: note: 'std::isnan' declared here
  632 |     isnan(_Tp __x)
      |     ^~~~~
samu: subcommand failed

@guijan
qualify isnan() with std namespace
ed9540d 
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.
guijan added a commit to guijan/dolphin that referenced this pull request Apr 22, 2024
@guijan
port to NetBSD
9c46f9d 
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 added a commit to guijan/dolphin that referenced this pull request Apr 22, 2024
@guijan
port to NetBSD
14ae234 
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
building
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@guijan @ocornut