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.

Sign up for GitHub

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

Jump to bottom

[TESTING] Switch compilation from C++17 to C++20 #4874

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

[TESTING] Switch compilation from C++17 to C++20 #4874

wants to merge 2 commits into from

Conversation

vlstill
Copy link
Contributor

@vlstill vlstill commented Aug 18, 2024 •
edited
Loading

Based on #4347. This applies a workaround that makes boost < 1.74 compatible with GCC >= 10 in the C++20 mode.

@fruffy @vlstill
Switch from C++17 to C++20.
8a59817 
Signed-off-by: fruffy <fruffy@nyu.edu>
@vlstill vlstill added run-sanitizer Use this tag to run a Clang+Sanitzers CI run. core Topics concerning the core segments of the compiler (frontend, midend, parser) run-ubuntu18 Use this tag to trigger a Ubuntu-18 CI run. run-static Use this tag to trigger static build CI run. labels Aug 18, 2024
@vlstill vlstill force-pushed the vstill/C++20-on-fruffy-c++20 branch 3 times, most recently from 5188d30 to 3e44ceb Compare August 18, 2024 17:59
@vlstill vlstill changed the title [TESTING] Enable C++20 [TESTING] Switch compilation from C++17 to C++20 Aug 18, 2024
@vlstill vlstill force-pushed the vstill/C++20-on-fruffy-c++20 branch 2 times, most recently from 4010ffb to c880182 Compare August 18, 2024 19:30
@vlstill
Work around boost::format requiring deprecated allocate
9590ab6 
Signed-off-by: Vladimir Still <git@vstill.eu>
@vlstill vlstill force-pushed the vstill/C++20-on-fruffy-c++20 branch from c880182 to 9590ab6 Compare August 18, 2024 19:40
@asl
Copy link
Contributor

asl commented Aug 19, 2024

Wow, very gross hack! :)

@vlstill
Copy link
Contributor Author

vlstill commented Aug 19, 2024

Yes. And a bit obscure too as it requires non-standard g++ but standard boost version on Ubuntu 20.04 (sure, there will be more platforms with this problem, but they are not officially supported). I still think we should make sure we can compile with GCC 10 on 20.04 though. BTW. clang can't handle the compilation at all on Ubuntu 20.04 unless GCC 10 is installed -- there is some compatibility problem with abseil which tries to use <compare>.

I'll need to double-check some parts though... the problem is it also modifies type of the string inside, which is no longer std::string, but a different instance of std::basic_string<...>. Which is why I had to change one of the helper functions' signatures. I'll double check we are not doing some string copies because of this and if so probably make this conditional to the case it is actually needed.

@fruffy fruffy added the breaking-change This change may break assumptions of compiler back ends. label Aug 19, 2024
@fruffy
Copy link
Collaborator

fruffy commented Aug 19, 2024

Clever solution. Nevertheless, it might be easier to just compile with a version of boost that is not broken?

@vlstill
Copy link
Contributor Author

vlstill commented Aug 19, 2024 •
edited
Loading

It is not broken, it is just old. At the time of its release, the method might have been deprecated, but that is all...

This issue seemed stuck to me... It all depends on what we want for Ubuntu 20.04.

  • Say we only support GCC 9, no clang.
    • We have to move sanitizers to 22.04 or newer.
    • If we want to bump GCC minimal version requirement for P4C we will require custom boost install on Ubuntu 20.04.
  • Support GCC 10+ on Ubuntu 20.04 with the old boost they have.
  • Get rid of the boost from platform dependency... I believe this path is stuck currently.

@fruffy
Copy link
Collaborator

fruffy commented Aug 19, 2024 •
edited
Loading

It looks like boost doesn't really support backporting patches, otherwise we could tried to fix up older versions of boost.

Get rid of the boost from platform dependency... I believe this path is stuck currently.

I can push #4663 forward, the current issue is the massive amount of includes added by this loop.

asl
asl reviewed Aug 19, 2024
View reviewed changes
frontends/p4/typeChecking/typeCheckTypes.cpp
@@ -331,13 +331,13 @@ static bool checkEnumValueInitializer(const IR::Type_Bits *type, const IR::Expre
std::string extraMsg;
if (!type->isSigned && constant->value < low) {
extraMsg =
str(boost::format(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one only does string interpolation. Maybe get rid of boost::format entirely here to reduce the footprint. Same below.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@asl asl asl left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
breaking-change This change may break assumptions of compiler back ends. core Topics concerning the core segments of the compiler (frontend, midend, parser) run-sanitizer Use this tag to run a Clang+Sanitzers CI run. run-static Use this tag to trigger static build CI run. run-ubuntu18 Use this tag to trigger a Ubuntu-18 CI run.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@vlstill @asl @fruffy