search

alandefreitas / matplotplusplus

Matplot++: A C++ Graphics Library for Data Visualization 📊🗾
https://alandefreitas.github.io/matplotplusplus/
MIT License
4.29k stars 333 forks source link

Clang compilation issue with legend freestanding functions #293

Open WildRackoon opened 2 years ago

WildRackoon commented 2 years ago

Bug category

Platform

Environment Details: Ubuntu 22.04.1 LTS CMake Ninja Clang version 14.0.0-1ubuntu1 x86_64 c++17

Describe the bug I have the same issue as #15 but in reverse: Removing the MSVC anonymous namespace hack makes it work for Clang 14

Compilation Log

[43/1093] Building CXX object thirdparty/matplotplusplus/examples/line_plot/loglog/CMakeFiles/example_loglog_5.dir/loglog_5.cpp.o
FAILED: thirdparty/matplotplusplus/examples/line_plot/loglog/CMakeFiles/example_loglog_5.dir/loglog_5.cpp.o 
/usr/bin/c++ -DMATPLOT_BUILD_HIGH_RESOLUTION_WORLD_MAP -DNOMINMAX -I/home/rackoon/dev/plot/thirdparty/matplotplusplus/source -O2 -O3 -DNDEBUG -std=c++17 -MD -MT thirdparty/matplotplusplus/examples/line_plot/loglog/CMakeFiles/example_loglog_5.dir/loglog_5.cpp.o -MF thirdparty/matplotplusplus/examples/line_plot/loglog/CMakeFiles/example_loglog_5.dir/loglog_5.cpp.o.d -o thirdparty/matplotplusplus/examples/line_plot/loglog/CMakeFiles/example_loglog_5.dir/loglog_5.cpp.o -c /home/rackoon/dev/plot/thirdparty/matplotplusplus/examples/line_plot/loglog/loglog_5.cpp
/home/rackoon/dev/plot/thirdparty/matplotplusplus/examples/line_plot/loglog/loglog_5.cpp:15:5: error: reference to 'legend' is ambiguous
    legend("Signal 1", "Signal 2")
    ^
/home/rackoon/dev/plot/thirdparty/matplotplusplus/source/matplot/core/legend.h:17:11: note: candidate found by name lookup is 'matplot::legend'
    class legend {
          ^
/home/rackoon/dev/plot/thirdparty/matplotplusplus/source/matplot/freestanding/axes_functions.h:236:19: note: candidate found by name lookup is 'matplot::legend'
    legend_handle legend(axes_handle ax, const std::vector<std::string> &names);
                  ^
/home/rackoon/dev/plot/thirdparty/matplotplusplus/source/matplot/freestanding/axes_functions.h:237:19: note: candidate found by name lookup is 'matplot::legend'
    legend_handle legend(const std::vector<std::string> &names);
                  ^
/home/rackoon/dev/plot/thirdparty/matplotplusplus/source/matplot/freestanding/axes_functions.h:239:19: note: candidate found by name lookup is 'matplot::legend'
    legend_handle legend(axes_handle ax, bool visible = true);
                  ^
/home/rackoon/dev/plot/thirdparty/matplotplusplus/source/matplot/freestanding/axes_functions.h:240:19: note: candidate found by name lookup is 'matplot::legend'
    legend_handle legend(bool visible = true);
                  ^
/home/rackoon/dev/plot/thirdparty/matplotplusplus/source/matplot/freestanding/axes_functions.h:242:19: note: candidate found by name lookup is 'matplot::legend'
    legend_handle legend(std::vector<axes_object_handle> objs,
                  ^
/home/rackoon/dev/plot/thirdparty/matplotplusplus/source/matplot/freestanding/axes_functions.h:248:23: note: candidate found by name lookup is 'matplot::(anonymous namespace)::legend'
        legend_handle legend(axes_handle ax, std::string_view name,
                      ^
/home/rackoon/dev/plot/thirdparty/matplotplusplus/source/matplot/freestanding/axes_functions.h:255:23: note: candidate found by name lookup is 'matplot::(anonymous namespace)::legend'
        legend_handle legend(std::string_view name,
                      ^
1 error generated.
[68/1093] Building CXX object thirdparty/matplotplusplus/source/matplot/CMakeFiles/matplot.dir/axes_objects/contours.cpp.o
ninja: build stopped: subcommand failed.

Recommendations Frankly, I can't figure a way to fix those compilation quirks in a proper manner for both compilers. However, I see the CMakeLists already has HACKS options

# MSVC hacks
option(BUILD_WITH_MSVC_HACKS "Accept utf-8 in MSVC by default." ON)
option(BUILD_WITH_UTF8 "Accept utf-8 in MSVC by default." ON)
option(BUILD_WITH_EXCEPTIONS "Add compiler flags to use exceptions." ON)

We could simply detect the macro #ifdef _MSC_VER to conditionnally enable this anonymous namespace ? We should also pinpoint the exact MSVC version in which it fails and act accordingly, it is probably fixed in newer ones ?

I can take care of this if you want.

alandefreitas commented 2 years ago

Interesting. Clang usually includes a message describing why each candidate was or wasn't rejected.

It would be good to know which overloads are really ambiguous and which is the expected overload before deciding the best solution.

In general, we don't even usually need _MSC_VER to go into CMakeLists.txt.

WildRackoon commented 2 years ago

Interesting. Clang usually includes a message describing why each candidate was or wasn't rejected.

Exactly, I am very puzzled by this, I feel like it does not try to instantiate the variadic template before failing to choose a candidate.

I have this for now: https://github.com/alandefreitas/matplotplusplus/compare/master...WildRackoon:matplotplusplus:fix-ambiguous-legend , but I am unsure if this is sufficient I have to test on MSVC.

Speaking of tests I am unsure how to run it, see #294.

alandefreitas commented 2 years ago

Exactly, I am very puzzled by this, I feel like it does not try to instantiate the variadic template before failing to choose a candidate.

Yes. It's important to know which are the ambiguous and expected overloads. We can do that by elimination, but clang usually has good messages to help with that.

This is probably a sensitive issue, since these were not ambiguous overloads before, so making guesses until we get it right is likely to break the API for other platforms.

Speaking of tests I am unsure how to run it, see https://github.com/alandefreitas/matplotplusplus/discussions/294.

Regarding the tests for new PRs, we just need the tests in the GHA workflow in /.github/workflows/build.yml. These scripts basically ensure all examples compile for each platform/compiler.

There are no real unit tests for the library because it's not easy to come up with unit tests for graphical applications. I had an idea based on image fingerprints at some point but it doesn't seem worth it at this point.

WildRackoon commented 2 years ago

Okay got it, it is alright visual testing is a hard issue.

I was also confused by the ctest command in the workflow, that must had been added by default because it does not run anything.

alandefreitas commented 2 years ago

Yes. Probably. I use ctest in all libraries but this one.

wahello commented 1 year ago

I encountered this problem on macos Ventura.

tcaer commented 1 year ago

@wahello were you able to fix it? I am experiencing this issue on Monterrey.

nourgaliev commented 1 year ago

I am having this problem on Monterey. It works with gcc, but failed with clang. Also, it failed debug build:

[ 2%] Building CXX object source/matplot/CMakeFiles/matplot.dir/util/world_cities.cpp.o [ 2%] Building CXX object source/matplot/CMakeFiles/matplot.dir/util/world_map_10m.cpp.o /Users/nourgaliev1/INSTALL/matplotplusplus/matplotplusplus/source/matplot/util/contourc.cpp:399:24: error: variable 'npoints' set but not used [-Werror,-Wunused-but-set-variable] size_t npoints = static_cast(line.size() + 1); ^ 1 error generated. gmake[2]: [source/matplot/CMakeFiles/matplot.dir/build.make:244: source/matplot/CMakeFiles/matplot.dir/util/contourc.cpp.o] Error 1 gmake[2]: Waiting for unfinished jobs....

It can be fixed by simply adding (void)npoints

But I think this should be fixed in distribution.