Fix the build failure with C++20 standard (apache#302)

BewareMyPower · web-flow · commit 0e63af9b5fee · 2023-07-17T10:10:37.000+08:00
### 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
diff --git a/.github/workflows/ci-pr-validation.yaml b/.github/workflows/ci-pr-validation.yaml
@@ -90,11 +90,35 @@ jobs:
         run: make check-format
 
       - name: Build
-        run: make -j8
+        run: |
+          # Build the libraries first to avoid possible link failures
+          cmake --build . -j8 --target pulsarShared pulsarStatic
+          cmake --build . -j8
 
       - name: Run unit tests
         run: RETRY_FAILED=3 ./run-unit-tests.sh
 
+  cpp20-build:
+    name: Build with the C++20 standard
+    runs-on: ubuntu-22.04
+    timeout-minutes: 60
+
+    steps:
+      - name: checkout
+        uses: actions/checkout@v3
+      - name: Install deps
+        run: |
+          sudo apt-get update -y
+          sudo apt-get install -y libcurl4-openssl-dev libssl-dev \
+            protobuf-compiler libprotobuf-dev libboost-dev \
+            libboost-dev libboost-program-options-dev \
+            libzstd-dev libsnappy-dev libgmock-dev libgtest-dev
+      - name: CMake
+        run: cmake -B build -DBUILD_PERF_TOOLS=ON -DCMAKE_CXX_STANDARD=20
+      - name: Build
+        run: |
+          cmake --build build -j8 --target pulsarShared pulsarStatic
+          cmake --build build -j8
 
   cpp-build-windows:
     timeout-minutes: 120
@@ -281,7 +305,7 @@ jobs:
   check-completion:
     name: Check Completion
     runs-on: ubuntu-latest
-    needs: [wireshark-dissector-build, unit-tests, cpp-build-windows, package, cpp-build-macos]
+    needs: [wireshark-dissector-build, unit-tests, cpp20-build, cpp-build-windows, package, cpp-build-macos]
 
     steps:
       - run: true
diff --git a/lib/ObjectPool.h b/lib/ObjectPool.h
@@ -223,8 +223,8 @@ class ObjectPool {
     }
 
    private:
-    ObjectPool<Type, MaxSize>(const ObjectPool<Type, MaxSize>&);
-    ObjectPool<Type, MaxSize>& operator=(const ObjectPool<Type, MaxSize>&);
+    ObjectPool(const ObjectPool<Type, MaxSize>&);
+    ObjectPool& operator=(const ObjectPool<Type, MaxSize>&);
 };
 }  // namespace pulsar
 #endif /* LIB_OBJECTPOOL_H_ */
diff --git a/lib/ProducerImpl.cc b/lib/ProducerImpl.cc
@@ -36,6 +36,7 @@
 #include "OpSendMsg.h"
 #include "ProducerConfigurationImpl.h"
 #include "PulsarApi.pb.h"
+#include "Semaphore.h"
 #include "TimeUtils.h"
 #include "TopicName.h"
 #include "stats/ProducerStatsDisabled.h"
diff --git a/lib/ProducerImpl.h b/lib/ProducerImpl.h
@@ -31,7 +31,6 @@
 #include "PendingFailures.h"
 #include "PeriodicTask.h"
 #include "ProducerImplBase.h"
-#include "Semaphore.h"
 
 namespace pulsar {
 
@@ -53,6 +52,7 @@ class PulsarFriend;
 
 class Producer;
 class MemoryLimitController;
+class Semaphore;
 class TopicName;
 struct OpSendMsg;
 
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
@@ -56,7 +56,7 @@ file(GLOB TEST_SOURCES *.cc c/*.cc)
 
 add_executable(pulsar-tests ${TEST_SOURCES} ${PROTO_SOURCES})
 
-target_include_directories(pulsar-tests PRIVATE ${PROJECT_SOURCE_DIR}/lib ${AUTOGEN_DIR}/lib)
+target_include_directories(pulsar-tests PRIVATE ${AUTOGEN_DIR}/lib)
 
 target_link_libraries(pulsar-tests ${CLIENT_LIBS} pulsarStatic $<$<CONFIG:Debug>:${GMOCKD_LIBRARY_PATH}> $<$<CONFIG:Debug>:${GTESTD_LIBRARY_PATH}> $<$<NOT:$<CONFIG:Debug>>:${GMOCK_LIBRARY_PATH}> $<$<NOT:$<CONFIG:Debug>>:${GTEST_LIBRARY_PATH}>)
 
diff --git a/tests/InterceptorsTest.cc b/tests/InterceptorsTest.cc
@@ -25,7 +25,7 @@
 #include <utility>
 
 #include "HttpHelper.h"
-#include "Latch.h"
+#include "lib/Latch.h"
 #include "lib/LogUtils.h"
 
 DECLARE_LOG_OBJECT()
diff --git a/tests/KeyValueImplTest.cc b/tests/KeyValueImplTest.cc
@@ -16,9 +16,10 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-#include <KeyValueImpl.h>
 #include <gtest/gtest.h>
 
+#include "lib/KeyValueImpl.h"
+
 using namespace pulsar;
 
 TEST(KeyValueTest, testEncodeAndDeCode) {
diff --git a/tests/LookupServiceTest.cc b/tests/LookupServiceTest.cc
@@ -16,10 +16,6 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-#include <BinaryProtoLookupService.h>
-#include <Future.h>
-#include <HTTPLookupService.h>
-#include <Utils.h>
 #include <gtest/gtest.h>
 #include <pulsar/Authentication.h>
 #include <pulsar/Client.h>
@@ -31,11 +27,15 @@
 
 #include "HttpHelper.h"
 #include "PulsarFriend.h"
+#include "lib/BinaryProtoLookupService.h"
 #include "lib/ClientConnection.h"
 #include "lib/ConnectionPool.h"
+#include "lib/Future.h"
+#include "lib/HTTPLookupService.h"
 #include "lib/LogUtils.h"
 #include "lib/RetryableLookupService.h"
 #include "lib/TimeUtils.h"
+#include "lib/Utils.h"
 
 DECLARE_LOG_OBJECT()
 
diff --git a/tests/MessageChunkingTest.cc b/tests/MessageChunkingTest.cc
@@ -23,9 +23,9 @@
 #include <ctime>
 #include <random>
 
-#include "ChunkMessageIdImpl.h"
 #include "PulsarFriend.h"
 #include "WaitUtils.h"
+#include "lib/ChunkMessageIdImpl.h"
 #include "lib/LogUtils.h"
 
 DECLARE_LOG_OBJECT()
diff --git a/tests/MessageTest.cc b/tests/MessageTest.cc
@@ -22,7 +22,7 @@
 
 #include <string>
 
-#include "MessageImpl.h"
+#include "lib/MessageImpl.h"
 
 using namespace pulsar;
 TEST(MessageTest, testMessageContents) {
diff --git a/tests/SchemaTest.cc b/tests/SchemaTest.cc
@@ -20,7 +20,7 @@
 #include <pulsar/Client.h>
 
 #include "PulsarFriend.h"
-#include "SharedBuffer.h"
+#include "lib/SharedBuffer.h"
 
 using namespace pulsar;
 
diff --git a/tests/TableViewTest.cc b/tests/TableViewTest.cc
@@ -23,10 +23,10 @@
 #include <future>
 
 #include "HttpHelper.h"
-#include "LogUtils.h"
 #include "PulsarFriend.h"
-#include "TopicName.h"
 #include "WaitUtils.h"
+#include "lib/LogUtils.h"
+#include "lib/TopicName.h"
 
 using namespace pulsar;
 
diff --git a/tests/c/c_SeekTest.cc b/tests/c/c_SeekTest.cc
@@ -17,12 +17,13 @@
  * under the License.
  */
 
-#include <TimeUtils.h>
 #include <gtest/gtest.h>
 #include <pulsar/c/client.h>
 
 #include <future>
 
+#include "lib/TimeUtils.h"
+
 struct seek_ctx {
     std::promise<pulsar_result> *promise;
 };
