refactor(IWYU): introduce IWYU and fix related issues (#1354)

.github/workflows/lint_and_test_cpp.yaml

@@ -57,6 +57,50 @@ jobs:
       - name: clang-format
         run: ./scripts/run-clang-format.py --clang-format-executable clang-format-3.9 -e ./src/shell/linenoise -e ./src/shell/sds -e ./thirdparty -r .
 
+  iwyu:
+    name: IWYU
+    needs: cpp_clang_format_linter
+    runs-on: ubuntu-latest
+    container:
+      image: apache/pegasus:thirdparties-bin-ubuntu2204-${{ github.base_ref }}
+    steps:
+      - uses: actions/checkout@v3
+      - uses: dorny/paths-filter@v2
+        id: changes
+        with:
+          filters: |
+            thirdparty:
+              - '.github/workflows/thirdparty-regular-push.yml'
+              - 'docker/thirdparties-src/**'
+              - 'docker/thirdparties-bin/**'
+              - 'thirdparty/**'
+      - name: Unpack prebuilt third-parties
+        if: steps.changes.outputs.thirdparty == 'false'
+        run: unzip /root/thirdparties-bin.zip -d ./thirdparty
+      - name: Rebuild third-parties
+        if: steps.changes.outputs.thirdparty == 'true'
+        working-directory: thirdparty
+        # Build thirdparties and leave some necessary libraries and source
+        run: |
+          mkdir build
+          cmake -DCMAKE_BUILD_TYPE=Release -DROCKSDB_PORTABLE=ON -B build/
+          cmake --build build/ -j $(nproc)
+          rm -rf build/Build build/Download/[a-y]* build/Source/[a-g]* build/Source/[i-q]* build/Source/[s-z]*
+          ../scripts/download_hadoop.sh hadoop-bin
+          ../scripts/download_zk.sh zookeeper-bin
+      - name: Build IWYU binary
+        run: |
+          mkdir iwyu && cd iwyu && git clone https://github.com/include-what-you-use/include-what-you-use.git
+          cd include-what-you-use && git checkout clang_14
+          cd .. && mkdir build && cd build && cmake -G "Unix Makefiles" -DCMAKE_PREFIX_PATH=/usr/lib/llvm-14 ../include-what-you-use
+          make -j $(nproc)
+      - name: Check IWYU
+        run: |
+          PWD=`pwd`
+          ./run.sh build --test --compiler clang-14,clang++-14 -j $(nproc) -t release --iwyu ${PWD}/iwyu/build/bin/include-what-you-use --skip_thirdparty
+          python3 iwyu/include-what-you-use/fix_includes.py --blank_lines --reorder --nocomments --nosafe_headers --separate_project_includes="<tld>" --ignore_re="release__builder|rrdb_types|layer2_types|metrics_test.cpp" --dry_run < src/builder/iwyu.out
+          python3 iwyu/include-what-you-use/fix_includes.py --blank_lines --reorder --nocomments --nosafe_headers --separate_project_includes="<tld>" --ignore_re="release__builder|rrdb_types|layer2_types|metrics_test.cpp" --dry_run < src/builder/iwyu.out | egrep "IWYU edited 0 files on your behalf"
+
   build_Release:
     name: Build Release
     needs: cpp_clang_format_linter

.licenserc.yaml

@@ -176,7 +176,6 @@ header:
     - 'src/runtime/fault_injector.h'
     - 'src/runtime/profiler.h'
     - 'src/runtime/tracer.h'
-    - 'src/utils/apply.h'
     - 'src/utils/autoref_ptr.h'
     - 'src/utils/binary_reader.h'
     - 'src/utils/binary_writer.h'

run.sh

@@ -91,6 +91,7 @@ function usage_build()
     echo "   --skip_thirdparty     whether to skip building thirdparties, default no"
     echo "   --enable_rocksdb_portable      build a portable rocksdb binary"
     echo "   --test                whether to build test binaries"
+    echo "   --iwyu                specify the binary path of 'include-what-you-use' when build with IWYU"
 }
 
 function exit_if_fail() {
@@ -119,6 +120,7 @@ function run_build()
     ROCKSDB_PORTABLE=OFF
     USE_JEMALLOC=OFF
     BUILD_TEST=OFF
+    IWYU=""
     while [[ $# > 0 ]]; do
         key="$1"
         case $key in
@@ -183,6 +185,10 @@ function run_build()
             --test)
                 BUILD_TEST=ON
                 ;;
+            --iwyu)
+                IWYU="$2"
+                shift
+                ;;
             *)
                 echo "ERROR: unknown option \"$key\""
                 echo
@@ -264,6 +270,9 @@ function run_build()
 
         echo "Running cmake Pegasus..."
         pushd $BUILD_DIR
+        if [ ! -z "${IWYU}" ]; then
+            CMAKE_OPTIONS="${CMAKE_OPTIONS} -DCMAKE_CXX_INCLUDE_WHAT_YOU_USE=${IWYU}"
+        fi
         CMAKE_OPTIONS="${CMAKE_OPTIONS} -DBUILD_TEST=${BUILD_TEST}"
         cmake ../.. -DCMAKE_INSTALL_PREFIX=$BUILD_DIR/output $CMAKE_OPTIONS
         exit_if_fail $?
@@ -287,7 +296,11 @@ function run_build()
 
     echo "[$(date)] Building Pegasus ..."
     pushd $BUILD_DIR
-    make install $MAKE_OPTIONS
+    if [ ! -z "${IWYU}" ]; then
+        make $MAKE_OPTIONS 2> iwyu.out
+    else
+        make install $MAKE_OPTIONS
+    fi
     exit_if_fail $?
 
     echo "Build finish time: `date`"

src/CMakeLists.txt

@@ -30,6 +30,7 @@ endif()
 include_directories(${PEGASUS_PROJECT_DIR})
 include_directories(${PEGASUS_PROJECT_DIR}/common/serialization_helper)
 include_directories(${PEGASUS_PROJECT_DIR}/include)
+include_directories(${PEGASUS_PROJECT_DIR}/builder/output/include)
 
 add_subdirectory(aio)
 add_subdirectory(base)

src/aio/aio_provider.cpp

@@ -25,9 +25,13 @@
  */
 
 #include "aio_provider.h"
+
+// IWYU pragma: no_include <string>
+
 #include "disk_engine.h"
 
 namespace dsn {
+class aio_task;
 
 aio_provider::aio_provider(disk_engine *disk) : _engine(disk) {}
 

src/aio/aio_provider.h

@@ -26,15 +26,16 @@
 
 #pragma once
 
-#include "aio/aio_task.h"
+#include <stdint.h>
+
+#include "utils/error_code.h"
 #include "utils/factory_store.h"
 
 namespace dsn {
 
+class aio_context;
+class aio_task;
 class disk_engine;
-class service_node;
-class task_worker_pool;
-class task_queue;
 
 #define DSN_INVALID_FILE_HANDLE -1
 struct linux_fd_t

src/aio/aio_task.cpp

@@ -15,10 +15,29 @@
 // specific language governing permissions and limitations
 // under the License.
 
-#include "runtime/task/task_engine.h"
+#include <string.h>
+#include <functional>
+#include <list>
+#include <memory>
+#include <utility>
+#include <vector>
+
+#include "aio/aio_task.h"
 #include "aio/file_io.h"
+#include "runtime/api_task.h"
+#include "runtime/service_engine.h"
+#include "runtime/task/task.h"
+#include "runtime/task/task_code.h"
+#include "runtime/task/task_engine.h"
+#include "runtime/task/task_spec.h"
+#include "utils/autoref_ptr.h"
+#include "utils/blob.h"
 #include "utils/error_code.h"
+#include "utils/fmt_logging.h"
+#include "utils/join_point.h"
 #include "utils/latency_tracer.h"
+#include "utils/threadpool_code.h"
+#include "utils/utils.h"
 
 namespace dsn {
 

src/aio/aio_task.h

@@ -26,11 +26,21 @@
 
 #pragma once
 
+#include <stddef.h>
+#include <stdint.h>
+#include <functional>
+#include <memory>
 #include <vector>
 
+#include "runtime/api_task.h"
 #include "runtime/task/task.h"
+#include "runtime/task/task_code.h"
+#include "utils/autoref_ptr.h"
+#include "utils/blob.h"
 
 namespace dsn {
+class error_code;
+class service_node;
 
 namespace utils {
 class latency_tracer;
@@ -51,6 +61,7 @@ typedef struct
 
 class disk_engine;
 class disk_file;
+
 class aio_context : public ref_counter
 {
 public:

src/aio/disk_engine.cpp

@@ -24,15 +24,27 @@
  * THE SOFTWARE.
  */
 
-#include "utils/fmt_logging.h"
-#include "aio/aio_task.h"
-#include "utils/flags.h"
-
 #include "disk_engine.h"
-#include "runtime/service_engine.h"
-#include "native_linux_aio_provider.h"
 
-using namespace dsn::utils;
+#include <algorithm>
+#include <list>
+// IWYU pragma: no_include <string>
+#include <utility>
+#include <vector>
+
+#include "aio/aio_provider.h"
+#include "aio/aio_task.h"
+#include "native_linux_aio_provider.h"
+#include "runtime/task/task.h"
+#include "runtime/task/task_code.h"
+#include "runtime/task/task_spec.h"
+#include "runtime/tool_api.h"
+#include "utils/error_code.h"
+#include "utils/factory_store.h"
+#include "utils/fmt_logging.h"
+#include "utils/join_point.h"
+#include "utils/link.h"
+#include "utils/threadpool_code.h"
 
 namespace dsn {
 DEFINE_TASK_CODE_AIO(LPC_AIO_BATCH_WRITE, TASK_PRIORITY_COMMON, THREAD_POOL_DEFAULT)

src/aio/disk_engine.h

@@ -26,13 +26,17 @@
 
 #pragma once
 
-#include "aio_provider.h"
+#include <stddef.h>
+#include <stdint.h>
+#include <memory>
 
-#include "runtime/tool_api.h"
-#include "utils/synchronize.h"
+#include "aio/aio_task.h"
+#include "aio_provider.h"
+#include "utils/singleton.h"
 #include "utils/work_queue.h"
 
 namespace dsn {
+class error_code;
 
 class disk_write_queue : public work_queue<aio_task>
 {

src/aio/file_io.cpp

@@ -24,10 +24,17 @@
  * THE SOFTWARE.
  */
 
-#include "disk_engine.h"
 #include "aio/file_io.h"
 
+// IWYU pragma: no_include <algorithm>
+#include <vector>
+
+#include "aio/aio_provider.h"
+#include "disk_engine.h"
+
 namespace dsn {
+class task_tracker;
+
 namespace file {
 
 /*extern*/ disk_file *open(const char *file_name, int flag, int pmode)

src/aio/file_io.h

@@ -26,14 +26,24 @@
 
 #pragma once
 
-#include <fcntl.h>
+#include <stdint.h>
+#include <list>
+#include <utility>
 
 #include "aio/aio_task.h"
+#include "runtime/api_task.h"
+#include "runtime/task/task.h"
+#include "runtime/task/task_code.h"
+#include "runtime/task/task_spec.h"
+#include "utils/autoref_ptr.h"
+#include "utils/error_code.h"
+#include "utils/join_point.h"
 
 namespace dsn {
 
 // forward declaration
 class disk_file;
+class task_tracker;
 
 namespace file {
 

src/aio/native_linux_aio_provider.cpp

@@ -26,16 +26,21 @@
 
 #include "native_linux_aio_provider.h"
 
+#include <errno.h>
 #include <fcntl.h>
+#include <unistd.h>
+#include <memory>
 
+#include "aio/aio_provider.h"
 #include "aio/disk_engine.h"
 #include "runtime/service_engine.h"
 #include "runtime/task/async_calls.h"
-#include "utils/api_utilities.h"
 #include "utils/fail_point.h"
 #include "utils/fmt_logging.h"
 #include "utils/latency_tracer.h"
+#include "utils/ports.h"
 #include "utils/safe_strerror_posix.h"
+#include "utils/string_view.h"
 
 namespace dsn {
 

src/aio/native_linux_aio_provider.h

@@ -26,9 +26,14 @@
 
 #pragma once
 
+#include <stdint.h>
+
+#include "aio/aio_task.h"
 #include "aio_provider.h"
+#include "utils/error_code.h"
 
 namespace dsn {
+class disk_engine;
 
 class native_linux_aio_provider : public aio_provider
 {

src/aio/test/aio.cpp

@@ -24,13 +24,33 @@
  * THE SOFTWARE.
  */
 
+#include <alloca.h>
+#include <fcntl.h>
+// IWYU pragma: no_include <gtest/gtest-message.h>
+// IWYU pragma: no_include <gtest/gtest-test-part.h>
 #include <gtest/gtest.h>
-
-#include "runtime/task/async_calls.h"
+#include <stdint.h>
+#include <string.h>
+#include <list>
+#include <memory>
+#include <string>
+
+#include "aio/aio_task.h"
+#include "aio/file_io.h"
+#include "runtime/task/task_code.h"
+#include "runtime/tool_api.h"
+#include "utils/autoref_ptr.h"
+#include "utils/error_code.h"
 #include "utils/fail_point.h"
 #include "utils/filesystem.h"
-#include "utils/smart_pointers.h"
+#include "utils/fmt_logging.h"
+#include "utils/ports.h"
 #include "utils/strings.h"
+#include "utils/threadpool_code.h"
+
+namespace dsn {
+class disk_file;
+} // namespace dsn
 
 using namespace ::dsn;
 
@@ -154,8 +174,8 @@ TEST(core, operation_failed)
     auto fp = file::open("tmp_test_file", O_WRONLY, 0600);
     EXPECT_TRUE(fp == nullptr);
 
-    auto err = dsn::make_unique<dsn::error_code>();
-    auto count = dsn::make_unique<size_t>();
+    auto err = std::make_unique<dsn::error_code>();
+    auto count = std::make_unique<size_t>();
     auto io_callback = [&err, &count](::dsn::error_code e, size_t n) {
         *err = e;
         *count = n;