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

Add Libcxx Compatability Mode to AnyBlob #3

Closed
wants to merge 0 commits into from

Conversation

wagjamin
Copy link
Contributor

@wagjamin wagjamin commented Nov 24, 2023

Contribution Overview

At the moment, AnyBlob can only link against the gnu standard library.
This commit adds a new CMake option ANYBLOB_LIBCXX_COMPAT that allows
building AnyBlob while linking against libcxx.

Detailed Changes

The main changes in this commit are:

  • Missing header includes. We don't bring in the same transitive headers
    when including the libcxx headers, as a result some string headers
    were missing in some places.
  • Remove ThroughputResolver when building with libcxx. This is the
    more invasive of the two changes. The ThroughputResolver depends on
    __gnu_pbds::tree, which is not spported in libcxx. When building in
    libcxx compatibility mode, we now remove this resolve completely. In
    this case, the io_uring_socket.cpp falls back onto the default
    resolver.
  • CI changes to test the new compatibility mode.

To ease reviewing, the PR is split into two commits. The first commit contains
the code changes to get AnyBlob to build with libcxx. The second commit
contains the CI changes.

Test Plan

  • We have verified that we can build all targets in libcxx compatibility
    mode.
  • We have verified that both unit and integration tests pass when building
    with clang-15 and the libcxx compatibility mode.
  • We have extended the CI to also build with clang-15 and libc++, running both unit and integration tests.

wagjamin added a commit to wagjamin/perfevent that referenced this pull request Nov 24, 2023
When building `PerfEvent.hpp` with `libcxx`, compilation fails. This is
because we are not including the `sstream` header.

This commit adds the missing include, allowing `PerfEvent.hpp` to build
with `libcxx`.

We ran into this issue while filing:
durner/AnyBlob#3
@wagjamin
Copy link
Contributor Author

I just filed viktorleis/perfevent#8 to make sure we can get around the nasty include ordering requirement for some of the tests.
Once this is merged I will update the PR to remove the special handling.

@@ -1,3 +1,7 @@
// Sadly, the include order matters here. PerfEvent depends on sstream but
// does not include the header.
#include <sstream>
Copy link
Owner

Choose a reason for hiding this comment

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

Let's fix that upstream.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Already filed the PR :-P

// Sadly, the include order matters here. PerfEvent depends on sstream but
// does not include the header.
#include <sstream>

Copy link
Owner

Choose a reason for hiding this comment

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

Let's fix that upstream.

@durner
Copy link
Owner

durner commented Nov 24, 2023

I just filed viktorleis/perfevent#8 to make sure we can get around the nasty include ordering requirement for some of the tests. Once this is merged I will update the PR to remove the special handling.

I just wanted to do the same. Thanks!

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.

2 participants