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

Explicitly specify c++ standard and reduce the use of 3rd party libraries #3198

Closed
junaire opened this issue Oct 25, 2021 · 3 comments
Closed
Assignees
Labels
community Source: who proposed the issue type/enhancement Type: make the code neat or more efficient
Milestone

Comments

@junaire
Copy link
Contributor

junaire commented Oct 25, 2021

Currently our code base uses a lot of c++17 core language features, like if constexprand etc, however we still use some 3rd party library components like boost::any, boost:variant, folly::optional... and we still have some backwards compatible codes like

template <typename T>
static constexpr auto is_copy_constructible_v = std::is_copy_constructible<T>::value;

These are weird, and I think we should:

  • Explicitly specify our c++ standdard in CMakeLists.txt
  • As the c++ standard library and 3rd party library all implement the same or similar features, we are supposed to use things in the STL.
  • Refactor our current code, use std::variant, std::any and std::optional, remove helper functions like is_copy_constructible_v and etc.

I think these work can make our code more clean and robust. However, feel free to close this if you guys think the main tasks now are implementing new features and bugs fixing ;-)

@Shylock-Hg
Copy link
Contributor

Shylock-Hg commented Oct 25, 2021

Thanks for your suggestion. I think it's reasonable.

  1. We had specify the c++ 17 in https://github.com/vesoft-inc/nebula/blob/master/cmake/nebula/GeneralCompilerConfig.cmake
  2. I think we should use STL first. But this maybe won't be done soon, or you could pull request directly.

@jiayuehua
Copy link
Contributor

@junaire thanks, your pr is updated and merged.

@junaire
Copy link
Contributor Author

junaire commented Mar 16, 2022

@junaire thanks, your pr is updated and merged.

i'm so grateful that you guys take my little pr so seriously and continue working on it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Source: who proposed the issue type/enhancement Type: make the code neat or more efficient
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants