epezent / implot

Immediate Mode Plotting
MIT License
4.78k stars 531 forks source link

Fix build failure on pre C++11 compilers #322

Closed RT2Code closed 2 years ago

RT2Code commented 2 years ago

The latest version of implot cannot build on pre C++11 compilers because it uses delegated constructors and consecutive right angle brackets with no space between them. I don't know if targeting pre C++11 compilers is in implot goals, but if it is, this PR will fix that.

epezent commented 2 years ago

@RT222 , I see this stems from a vcpkg update. I want to of course make it possible for implot to be used with vcpkg, but supporting pre C++11 compilers is not a burden I'm ready to take on. For future vcpkg updates, I would suggest we ignore or turn off that check if possible. I'm not really familiar with vcpkg so I'll defer to you.

PS -- Thanks for keeping us updated on vcpkg. I'm sure there are several folks who appreciate your efforts!

RT2Code commented 2 years ago

My pleasure, though its you who deserve thanks for the work you put into this awesome library.

I totally understand that you don't wish to support pre C++11 compilers. I work with C++20 on my projects and I think it is regrettable that in some places we still have to waste our time trying to support some almost two decade old standards. Anyway, you're not supposed to make your library compatible with vcpkg, it's the other way around.

I stumbled into this issue through the vcpkg CI, and it only happened on the osx platform because it's using a pre C++11 compiler. As I don't code for osx, I have no idea why is that, so I'm going to ask the vcpkg team how we should handle the future versions of implot to support (or not) this platform.

marcizhu commented 2 years ago

macOS developer here 🙋🏻‍♂️

While I have never used vcpkg and thus I don't know exactly how their CI works, I do know for certain that the macOS compiler (Apple clang) defaults to C++98 (just like if you call g++ without -std=c++11) and the provided log file seems to imply they don't explicitly set the C++ version. I guess this could be as easy as adding a -std=c++11 to the compiler command line on the OSX CI build.

RT2Code commented 2 years ago

Thanks for the explanation. You're right, osx supports new C++ standards, the opposite would have been very surprising actually. vcpkg doesn't care about this anyway, so you are right, the fix was trivial : I just had to specify the desired c++ version in the cmake file.

All is good and all platforms will be supported by the implot vcpkg port in the future, no matter what C++ standard it is targeting. 😃

epezent commented 2 years ago

Nice catch, @marcizhu!