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.

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

Establish a C++ version for the project #375

Open
riojax opened this issue Jul 18, 2023 · 12 comments
Open

Establish a C++ version for the project #375

riojax opened this issue Jul 18, 2023 · 12 comments

Comments

@riojax
Copy link
Contributor

riojax commented Jul 18, 2023

It's a good idea to set a specific C++ version and follow it to avoid future problems, for example, someone pushing C++20 code that breaks all MSVC 2019 builds.

Personally I propose to follow C++11 that is very compatible between a lot compilers/versions.

@dimitry-ishenko
Copy link
Contributor

dimitry-ishenko commented Jul 18, 2023

That's already been done:

orbiter/CMakeLists.txt

Lines 9 to 11 in 09baae4

set(CMAKE_CXX_STANDARD 20)
set(CMAKE_CXX_STANDARD_REQUIRED ON)
set(CMAKE_CXX_EXTENSIONS OFF)

Personally I propose to follow C++11

Not sure why you would establish a version that's over 10 years old though. All respectable compilers (GCC/Clang/MSVC) support c++20.

And with c++17 & 20 we can replace a lot of windows-specific code with cross-platform code from the standard library.

@riojax
Copy link
Contributor Author

riojax commented Jul 21, 2023

You pushed those lines in CMakeLists.txt and also pushed some C++20 code, but there are not a public discuss about to switch to C++20, for this I opened this issue to do it properly, in an open and public manner giving voice to anyone.

About to use C++20 I am totally against it because it is not fully supported on MSVC 2019 and the implemented part have serious unfixed STL issues.

P.S. I wrote (and write) tons of C++03 portable to every arch/OS without major problems.

@dimitry-ishenko
Copy link
Contributor

You pushed those lines in CMakeLists.txt and also pushed some C++20 code

Yes I did 😝

but there are not a public discuss about to switch to C++20, for this I opened this issue to do it properly, in an open and public manner giving voice to anyone.

I brought it up in an issue here and in the forums.

About to use C++20 I am totally against it because it is not fully supported on MSVC 2019 and the implemented part have serious unfixed STL issues.

Upgrade your compiler. Believe it or not, but MSVC is currently the one compiler that has a full support for C++20:

https://en.cppreference.com/w/cpp/compiler_support#cpp20

Others are not far behind though.

P.S. I wrote (and write) tons of C++03 portable to every arch/OS without major problems.

Why not C++98?... 😉 It's 2023... maybe it's time to upgrade?

@dimitry-ishenko
Copy link
Contributor

On a serious note:

  • c++11 gives us cross-platform thread support
  • c++17 - file system support, optional, tuples and decomposition
  • c++20 - formatting library, ranges, starts_with() and ends_with() for strings (important! 😅 )

And these are just a few things that came to my mind. Why deny yourself all these features just because you have an older compiler?

@riojax
Copy link
Contributor Author

riojax commented Jul 21, 2023

but there are not a public discuss about to switch to C++20, for this I opened this issue to do it properly, in an open and public manner giving voice to anyone.

I brought it up in an issue here and in the forums.

I don't have forum access, and here... I think that I didn't see it, sorry. Is this the issue?

Upgrade your compiler. Believe it or not, but MSVC is currently the one compiler that has a full support for C++20:

https://en.cppreference.com/w/cpp/compiler_support#cpp20

Others are not far behind though.

Ok, this can be a problem then to port it to Linux or BSD (also the MSVC 2019 support is incomplete and bugged)
Also, what is the real advantage using C++20 that worth to suffer incompatibilities with other compilers and drop some OS support?

Why not C++98?... 😉 It's 2023... maybe it's time to upgrade?

Nah, latest != better

And these are just a few things that came to my mind. Why deny yourself all these features just because you have an older compiler?

I only see real advantages on C++11. Later versions don't give me advantages and drop support for a lot OS/compilers adding STL complexity and worse performance

@n7275
Copy link
Contributor

n7275 commented Jul 21, 2023

I don't have forum access, and here... I think that I didn't see it, sorry. Is this the issue?

The forum and discord are still quite active. It is probably worth joining for the discussions that happen there.

@dimitry-ishenko
Copy link
Contributor

Ok, this can be a problem then to port it to Linux or BSD (also the MSVC 2019 support is incomplete and bugged)
Also, what is the real advantage using C++20 that worth to suffer incompatibilities with other compilers and drop some OS support?

Actually, portability is exactly why I think we should push for later standards. I'm a long-time Linux user and I would like to see Orbiter running natively on Linux.

Currently, Orbiter code is full of windows-specific code because at the time it was written, there was no standard for such things as file/directory manipulation for example. But, with C++17 we can now convert it into portable code that will run on other platforms.

C++20 gives us portable threads with cancellation (jthread) and semaphores.

If there is a specific issue you are running into, I'm happy to help figure it out.

@riojax
Copy link
Contributor Author

riojax commented Jul 22, 2023

Actually, portability is exactly why I think we should push for later standards. I'm a long-time Linux user and I would like to see Orbiter running natively on Linux.
Currently, Orbiter code is full of windows-specific code because at the time it was written, there was no standard for such things as file/directory manipulation for example. But, with C++17 we can now convert it into portable code that will run on other platforms.

I'm also a long term Linux user, and also want to have it native, but I don't want to sacrifice other OS in the way.
Think that MSVC 2019 dropped WinXP/ReactOS support and MSVC 2022 dropped Vista/7 and that's a no-go for me, to me the ideal compromise between functionalities and support is C++11 but it's true that C++17 is also a very good standard with shared mutex, filesystem, etc. Maybe C++17 is a better compromise for start than C++20 and if for something is needed to upgrade take this step. Anyway this is only my humble opinion and of course it can be wrong.

P.S. https://learn.microsoft.com/en-us/cpp/overview/visual-cpp-language-conformance?view=msvc-170

@jarmonik
Copy link
Contributor

I can reset this to C++17. Setting a required standard to highers existing is usually not a good idea since not everybody have the latest tools. Right now the code compiles on C++14 and might do well on C++11 haven't tested that.

@GLS-SSV
Copy link
Contributor

GLS-SSV commented Jul 24, 2023

In my humble opinion (and not because I'm using it), C++17 is a good choice between a more established standard (which has less features) and a "cutting-edge" standard, which brings support issues, bugs, etc.
If linux support does require a more recent standard, then maybe put the whole linux compatibility work in a branch (with whatever standard) and leave the master in C++17. Only down the road when the merge happens (and it should take a while to get everything working in linux), would everybody need to move forward.

@GLS-SSV
Copy link
Contributor

GLS-SSV commented Jan 5, 2024

Probably related to this "standards issue": compiling a project in VS2017 that uses "VesselAPI.h" and "drawapi.h" files, I need to add #include <algorithm> to those files, because otherwise std::max and std::min aren't found.
Should this include be added permenantly, or those calls changed, or something else?

@jarmonik
Copy link
Contributor

jarmonik commented Jan 5, 2024

Probably related to this "standards issue": compiling a project in VS2017 that uses "VesselAPI.h" and "drawapi.h" files, I need to add #include <algorithm> to those files, because otherwise std::max and std::min aren't found. Should this include be added permenantly, or those calls changed, or something else?

Will be fixed along "Terrain collision" PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants