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

Update codebase migration progress from C++11 to C++17/20 #579

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

GermanAizek
Copy link
Contributor

@KOLANICH, @alex19EP review my pull request and feedback me.

@KOLANICH
Copy link
Contributor

-return std::shared_ptr<language>(new some_language(*this)); //*this is specialized language info for that lang
+return std::make_shared<language>(*this);

looks like it creates the objects of superclass instead of objects of specialized classes. IDK if it is OK, but IMHO if it is, then the specialized classes should be eliminated in a separate PR.

@GermanAizek GermanAizek force-pushed the master branch 2 times, most recently from 5cda4a0 to a56c8fc Compare May 25, 2022 12:13
@GermanAizek
Copy link
Contributor Author

Windows build:

src\core\config.cpp(51): error C2429: language feature 'structured bindings' requires compiler flag '/std:c++17'

Linux build:

src/include/core/dtree.hpp:255:21: note: 'std::make_unique' is only available from C++14 onwards

@KOLANICH, I thought there was support for new standard, I saw a mention here:

RHVoice/CMakeLists.txt

Lines 32 to 38 in 219f846

if(${CMAKE_VERSION} VERSION_GREATER "3.12")
set(CMAKE_CXX_STANDARD 20)
else()
set(CMAKE_CXX_STANDARD 17)
endif()

@alex19EP
Copy link
Member

Windows build:

src\core\config.cpp(51): error C2429: language feature 'structured bindings' requires compiler flag '/std:c++17'

Linux build:

src/include/core/dtree.hpp:255:21: note: 'std::make_unique' is only available from C++14 onwards

@KOLANICH, I thought there was support for new standard, I saw a mention here:

RHVoice/CMakeLists.txt

Lines 32 to 38 in 219f846

if(${CMAKE_VERSION} VERSION_GREATER "3.12")
set(CMAKE_CXX_STANDARD 20)
else()
set(CMAKE_CXX_STANDARD 17)
endif()

hello. in cmake yes. but hour CI uses scons.

since we're not using all the features of c++11 yet, I'm not sure if switching to a newer standard is a good idea.

@GermanAizek
Copy link
Contributor Author

@alex19EP, if you set up a CI for new standard, I can do a code modernize?

@alex19EP
Copy link
Member

as I already said

since we're not using all the features of c++11 yet, I'm not sure if switching to a newer standard is a good idea.

if you want to work on it, you can do some research on cpp11 features. offhand, it seems to me that we can abandon the self-made thread class ...

@Olga-Yakovleva
Copy link
Member

Olga-Yakovleva commented Oct 11, 2022 via email

@alex19EP
Copy link
Member

will Debian
stable be OK with this?

lets ask @sthibaul

@sthibaul
Copy link

debian's default gcc will be at least gcc-12

@sthibaul
Copy link

debian's current stable gcc is gcc-10

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

Successfully merging this pull request may close these issues.

None yet

5 participants