Skip to content

Fix the build failure with C++20 standard #302

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

Merged
merged 1 commit into from
Jul 17, 2023

Conversation

BewareMyPower
Copy link
Contributor

@BewareMyPower BewareMyPower commented Jul 13, 2023

Motivation

When building the project with the -DCMAKE_CXX_STANDARD=20 option and GCC 11.3, it failed. There are two main reasons.

One is the ObjectPool.h, see http://eel.is/c++draft/diff.cpp17.class#2

In short, see the code below:

template <typename T>
struct A {
  // A<T>() {}  // error: simple-template-id not allowed for constructor
  A() {}        // OK, injected-class-name used
};

The other reason is deeply hidden and OS-specific. When building the target for the unit test, the lib/ directory is added into the include directories. So for #include "Semaphore.h", the Semaphore.h header will be looked up first in the lib/ directory. However, C++20 introduced a <semaphore> header, which finds the POSIX semaphore header semaphore.h in the system path.

For example, the include order in ubuntu:22.04 arm64 container is:

  • $PROJECT_DIR/lib/ (where Semaphore.h is)
  • ...
  • /usr/lib/gcc/aarch64-linux-gnu/11/include (where semaphore.h is)

The C++ header might be case insensitive so the lib/Semaphore.h will be included by the <semaphore> header, which is implicitly included by <thread>. Our own Semaphore.h does not have the POSIX semaphore struct definitions so the build failed.

Modifications

  • Fix the semantics error in ObjectPool.h
  • Remove the lib/ directory from the included directories of the unit test and include lib/xxx.h for header in lib/ directory.
  • Add a workflow to verify now it can be built with C++20

### Motivation

When building the project with the `-DCMAKE_CXX_STANDARD=20` option and
GCC 11.3, it failed. There are two main reasons.

One is the `ObjectPool.h`, see http://eel.is/c++draft/diff.cpp17.class#2

In short, see the code below:

```c++
template <typename T>
struct A {
  // A<T>() {}  // error: simple-template-id not allowed for constructor
  A() {}        // OK, injected-class-name used
};
```

The other reason is deeply hidden and OS-specific. When building the
target for the unit test, the `lib/` directory is added into the include
directories. So for `#include "Semaphore.h"`, the `Semaphore.h` header
will be looked up first in the `lib/` directory. However, C++20
introduced a `<semaphore>` header, which finds the POSIX semaphore
header `semaphore.h` in the system path.

For example, the include order in `ubuntu:22.04` arm64 container is:
- `$PROJECT_DIR/lib/` (where `Semaphore.h` is)
- ...
- `/usr/lib/gcc/aarch64-linux-gnu/11/include` (where `semaphore.h` is)

The C++ header is case insensitive so the `lib/Semaphore.h` will be
included by the `<semaphore>` header, which is implicitly included by
`<thread>`. Our own `Semaphore.h` does not have the POSIX semaphore
struct definitions so the build failed.

### Modifications

- Fix the semantics error in `ObjectPool.h`
- Remove the `lib/` directory from the included directories of the unit
  test and include `lib/xxx.h` for header in `lib/` directory.
- Add a workflow to verify now it can be built with C++20
@BewareMyPower BewareMyPower added this to the 3.3.0 milestone Jul 13, 2023
@BewareMyPower BewareMyPower self-assigned this Jul 13, 2023
@BewareMyPower BewareMyPower merged commit 0e63af9 into apache:main Jul 17, 2023
@BewareMyPower BewareMyPower deleted the bewaremypower/fix-cpp-20 branch July 17, 2023 02:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants