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

C++20 support - continue to use c++17 on files that use LLVM #1192

Merged
merged 2 commits into from
Jul 27, 2023

Conversation

spoonincode
Copy link
Member

LLVM headers prior to version 13.0 result in an error when compiling in c++20 mode.

Refactor EOS VM OC a wee bit to isolate LLVM headers to just a few .cpp files (and eliminate usage of "other stuff", like fc, from those same files), and compile those with c++17 still.

Compared to #1116, the positive with this change is that it retains compatibility with existing LLVM versions and doesn't get us in the business of patching system provided files which may cause some additional subtle headaches (do they need to be installed for libtester? do they cause an ODR violation?)

The negative is mainly that it's not clear how long lived this solution can be. For example, once we tackle #673, LLVM & EOS VM will be more tightly coupled. EOS VM doesn't use c++20 today, but it might in the future.

I don't particularly like using COMPILE_FLAGS "--std=gnu++17" but there is no source file property for CXX_STANDARD; there is a target property for CXX_STANDARD though, so a wee bit of clean up could possibly use that instead.

@greg7mdp
Copy link
Contributor

@spoonincode we need to merge either this PR or #1116 in order to switch to C++20. Let's decide and move forward.

@spoonincode spoonincode marked this pull request as ready for review July 26, 2023 19:06
@spoonincode spoonincode merged commit 6ab7c91 into main Jul 27, 2023
21 checks passed
@spoonincode spoonincode deleted the oc_llvm_c++17_only branch July 27, 2023 15:20
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

Successfully merging this pull request may close these issues.

c++20 support - Add fixed header file from llvm-11 distrib and use in build.
3 participants