Add formatting infrastructure to the project and upgrade LLVM.

Upgrading project wide LLVM version to 18 and add infrastructure to format c++ files with clang-format. We upgrade to llvm-18 because some of the clang-format options we're using have only been released with clang-format 18. Added: 1. clang-format file matching internal one 2. formatting tool based on python for cross-platform 3. pre-commit and pre-push hooks for formatting validation 4. Lint CI action.
cloudflare · Aug 9, 2024 · 82c8a62 · 82c8a62
1 parent fe883d8
commit 82c8a62
Show file tree

Hide file tree

Showing 18 changed files with 394 additions and 53 deletions.
diff --git a/.bazelrc b/.bazelrc
@@ -220,7 +220,7 @@ build:linux --linkopt='-stdlib=libc++' --host_linkopt='-stdlib=libc++'
 build:linux --linkopt='-l:libc++.a' --linkopt='-lm' --linkopt='-static-libgcc'
 # We don't expect to distribute host tools, no need to statically link libgcc.
 # TODO(cleanup): Ideally we'd also use shared libc++ here, we'd just need to install the
-# libunwind-15-dev and libc++abi-15-dev packages on CI to have all of shared libc++ available.
+# libunwind-18-dev and libc++abi-18-dev packages on CI to have all of shared libc++ available.
 build:linux --host_linkopt='-l:libc++.a' --host_linkopt='-lm'
 
 # On Linux, enable PIC. In macos pic is the default, and the objc_library rule does not work

diff --git a/.clang-format b/.clang-format
@@ -1,10 +1,91 @@
+Language: Cpp
+Standard: c++20
+ColumnLimit: 100
+
 WhitespaceSensitiveMacros:
   # clang format doesn't understand TypeScript, so make sure it doesn't mangle
   # overrides and additional definitions
   - JSG_TS_OVERRIDE
   - JSG_TS_DEFINE
   - JSG_STRUCT_TS_OVERRIDE
   - JSG_STRUCT_TS_DEFINE
-PointerAlignment: Left
-ColumnLimit: 100
 AllowShortFunctionsOnASingleLine: Empty
+
+# We should have "true" here but adding an include header would result in too many changed lines.
+# Once we turn this on we should use IncludeCategories so that things are consistently sorted.
+# For example, to protect against brittle headers (a header not including everything its using
+# because existing users happened to include a dependency for it), one could use the following
+# order:
+#   local folder files
+#   project files
+#   3p dependency includes
+#   standard language headers (put kj/ here?)
+#   system headers
+# While this would be ideal it's also important to note that this isn't the (non-documented) style
+# that KJ uses, so it may be worth documenting the style & making it consistent.
+SortIncludes: false
+
+AllowShortIfStatementsOnASingleLine: true
+AllowShortLoopsOnASingleLine: true
+
+IndentWidth: 2
+IndentCaseBlocks: false
+IndentCaseLabels: true
+PointerAlignment: Left
+DerivePointerAlignment: true
+
+# Really "Attach" but empty braces aren't split.
+BreakBeforeBraces: Custom
+BraceWrapping:
+  AfterCaseLabel: false
+  AfterClass: false
+  AfterControlStatement: Never
+  AfterEnum: false
+  AfterFunction: false
+  AfterNamespace: false
+  AfterObjCDeclaration: false
+  AfterStruct: false
+  AfterUnion: false
+  AfterExternBlock: false
+  BeforeCatch: false
+  BeforeElse: false
+  BeforeLambdaBody: false
+  BeforeWhile: false
+  IndentBraces: false
+  SplitEmptyFunction: false
+  SplitEmptyRecord: false
+  SplitEmptyNamespace: false
+
+Cpp11BracedListStyle: true
+
+AlignAfterOpenBracket: DontAlign
+AlignOperands: DontAlign
+AlignTrailingComments:
+  Kind: Always
+  OverEmptyLines: 0
+AlwaysBreakAfterReturnType: None
+AlwaysBreakTemplateDeclarations: Yes
+BreakStringLiterals: false
+BinPackArguments: true
+BinPackParameters: false
+BracedInitializerIndentWidth: 2
+BreakInheritanceList: BeforeColon
+ContinuationIndentWidth: 4
+IfMacros:
+  [
+    "KJ_SWITCH_ONEOF",
+    "KJ_CASE_ONEOF",
+    "KJ_IF_MAYBE",
+    "KJ_IF_SOME",
+    "CFJS_RESOURCE_TYPE",
+  ]
+LambdaBodyIndentation: OuterScope
+Macros: ["KJ_MAP(x,y)=[y](auto x)", "JSG_VISITABLE_LAMBDA(x,y,z)=[x,y]z"]
+PenaltyReturnTypeOnItsOwnLine: 1000
+PackConstructorInitializers: CurrentLine
+ReflowComments: false
+SpaceBeforeCtorInitializerColon: false
+SpaceBeforeInheritanceColon: false
+SpaceBeforeParens: ControlStatementsExceptControlMacros
+SpaceBeforeRangeBasedForLoopColon: false
+SpacesBeforeTrailingComments: 2
diff --git a/.devcontainer/Dockerfile b/.devcontainer/Dockerfile
@@ -2,7 +2,7 @@ FROM mcr.microsoft.com/devcontainers/base:jammy
 
 # Install dependencies, including clang via through LLVM APT repository. Note that this
 # will also install lldb and clangd alongside dependencies.
-ARG LLVM_VERSION=15
+ARG LLVM_VERSION=18
 RUN apt-get update && export DEBIAN_FRONTEND=noninteractive \
     && apt-get -y install --no-install-recommends software-properties-common python3 python3-distutils tclsh \
     && curl -fSsL -o /tmp/llvm.sh https://apt.llvm.org/llvm.sh && chmod +x /tmp/llvm.sh && bash /tmp/llvm.sh ${LLVM_VERSION} \

diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml
@@ -0,0 +1,23 @@
+name: Tests
+
+on:
+  pull_request:
+    paths:
+      - src/**
+  push:
+    branches:
+    - main
+
+jobs:
+  lint:
+    runs-on: ubuntu-24.04
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          show-progress: false
+      - name: Setup Linux
+        run: |
+          sudo apt-get install -y --no-install-recommends clang-format-18
+      - name: Lint
+        run: |
+          python3 ./tools/cross/format.py --check
diff --git a/.github/workflows/npm-types.yml b/.github/workflows/npm-types.yml
@@ -30,7 +30,7 @@ jobs:
           echo "version=${{ inputs.prerelease == false && '4' || '0'}}.$(cat src/workerd/io/supported-compatibility-date.txt | tr -d '-').${{ inputs.patch }}" >> $GITHUB_OUTPUT;
           echo "release_version=1.$(cat src/workerd/io/supported-compatibility-date.txt | tr -d '-').0" >> $GITHUB_OUTPUT;
   build-and-publish-types:
-    runs-on: ubuntu-20.04
+    runs-on: ubuntu-24.04
     needs: version
     steps:
       - uses: actions/checkout@v4
@@ -50,10 +50,10 @@ jobs:
             wget https://apt.llvm.org/llvm.sh
             sed -i '/apt-get install/d' llvm.sh
             chmod +x llvm.sh
-            sudo ./llvm.sh 15
-            sudo apt-get install -y --no-install-recommends clang-15 lld-15 libunwind-15 libc++abi1-15 libc++1-15 libc++-15-dev
-            echo "build:linux --action_env=CC=/usr/lib/llvm-15/bin/clang --action_env=CXX=/usr/lib/llvm-15/bin/clang++" >> .bazelrc
-            echo "build:linux --host_action_env=CC=/usr/lib/llvm-15/bin/clang --host_action_env=CXX=/usr/lib/llvm-15/bin/clang++" >> .bazelrc
+            sudo ./llvm.sh 18
+            sudo apt-get install -y --no-install-recommends clang-18 lld-18 libunwind-18 libc++abi1-18 libc++1-18 libc++-18-dev
+            echo "build:linux --action_env=CC=/usr/lib/llvm-18/bin/clang --action_env=CXX=/usr/lib/llvm-18/bin/clang++" >> .bazelrc
+            echo "build:linux --host_action_env=CC=/usr/lib/llvm-18/bin/clang --host_action_env=CXX=/usr/lib/llvm-18/bin/clang++" >> .bazelrc
       - name: build types
         run: |
             bazel build --disk_cache=~/bazel-disk-cache --strip=always --remote_cache=https://bazel:${{ secrets.BAZEL_CACHE_KEY }}@bazel-remote-cache.devprod.cloudflare.dev --config=release_linux //types:types

diff --git a/.github/workflows/npm.yml b/.github/workflows/npm.yml
@@ -101,10 +101,10 @@ jobs:
           wget https://apt.llvm.org/llvm.sh
           sed -i '/apt-get install/d' llvm.sh
           chmod +x llvm.sh
-          sudo ./llvm.sh 15
-          sudo apt-get install -y --no-install-recommends clang-15 lld-15 libunwind-15 libc++abi1-15 libc++1-15 libc++-15-dev
-          echo "build:linux --action_env=CC=/usr/lib/llvm-15/bin/clang --action_env=CXX=/usr/lib/llvm-15/bin/clang++" >> .bazelrc
-          echo "build:linux --host_action_env=CC=/usr/lib/llvm-15/bin/clang --host_action_env=CXX=/usr/lib/llvm-15/bin/clang++" >> .bazelrc
+          sudo ./llvm.sh 18
+          sudo apt-get install -y --no-install-recommends clang-18 lld-18 libunwind-18 libc++abi1-18 libc++1-18 libc++-18-dev
+          echo "build:linux --action_env=CC=/usr/lib/llvm-18/bin/clang --action_env=CXX=/usr/lib/llvm-18/bin/clang++" >> .bazelrc
+          echo "build:linux --host_action_env=CC=/usr/lib/llvm-18/bin/clang --host_action_env=CXX=/usr/lib/llvm-18/bin/clang++" >> .bazelrc
       - name: Build type generating Worker
         run: |
           bazel build --disk_cache=~/bazel-disk-cache --strip=always --remote_cache=https://bazel:${{ secrets.BAZEL_CACHE_KEY }}@bazel-remote-cache.devprod.cloudflare.dev --config=release_linux //types:types_worker

diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml
@@ -88,14 +88,15 @@ jobs:
           wget https://apt.llvm.org/llvm.sh
           sed -i '/apt-get install/d' llvm.sh
           chmod +x llvm.sh
-          sudo ./llvm.sh 15
-          sudo apt-get install -y --no-install-recommends clang-15 lld-15 libunwind-15 libc++abi1-15 libc++1-15 libc++-15-dev
-          echo "build:linux --action_env=CC=/usr/lib/llvm-15/bin/clang --action_env=CXX=/usr/lib/llvm-15/bin/clang++" >> .bazelrc
-          echo "build:linux --host_action_env=CC=/usr/lib/llvm-15/bin/clang --host_action_env=CXX=/usr/lib/llvm-15/bin/clang++" >> .bazelrc
+          sudo ./llvm.sh 18
+          sudo apt-get install -y --no-install-recommends clang-18 lld-18 libunwind-18 libc++abi1-18 libc++1-18 libc++-18-dev
+          echo "build:linux --action_env=CC=/usr/lib/llvm-18/bin/clang --action_env=CXX=/usr/lib/llvm-18/bin/clang++" >> .bazelrc
+          echo "build:linux --host_action_env=CC=/usr/lib/llvm-18/bin/clang --host_action_env=CXX=/usr/lib/llvm-18/bin/clang++" >> .bazelrc
       - name: Setup macOS
         if: runner.os == 'macOS'
         run: |
-          sudo xcode-select -s "/Applications/Xcode_15.1.app"
+          sudo brew install llvm@18
+          export PATH="$(brew --prefix llvm@18)/bin/bin:$PATH"
       - name: Setup Windows
         if: runner.os == 'Windows'
         run: |

diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -75,32 +75,21 @@ jobs:
         # libc++, but this appears to cause errors so they are also being explicitly installed.
         # Since the GitHub runner image comes with a number of preinstalled packages, we don't need
         # to use APT much otherwise.
-        # TODO(cleanup): Upgrade this to LLVM 16 as soon as it is available on the latest Ubuntu
-        # LTS release. Debian includes LLVM 16 since the Bookworm 12.4 point release.
         run: |
           export DEBIAN_FRONTEND=noninteractive
           wget https://apt.llvm.org/llvm.sh
           sed -i '/apt-get install/d' llvm.sh
           chmod +x llvm.sh
-          sudo ./llvm.sh 15
-          sudo apt-get install -y --no-install-recommends clang-15 lld-15 libunwind-15 libc++abi1-15 libc++1-15 libc++-15-dev libclang-rt-15-dev
-          echo "build:linux --action_env=CC=/usr/lib/llvm-15/bin/clang --action_env=CXX=/usr/lib/llvm-15/bin/clang++" >> .bazelrc
-          echo "build:linux --host_action_env=CC=/usr/lib/llvm-15/bin/clang --host_action_env=CXX=/usr/lib/llvm-15/bin/clang++" >> .bazelrc
-          sed -i -e "s%llvm-symbolizer%/usr/lib/llvm-15/bin/llvm-symbolizer%" .bazelrc
+          sudo ./llvm.sh 18
+          sudo apt-get install -y --no-install-recommends clang-18 lld-18 libunwind-18 libc++abi1-18 libc++1-18 libc++-18-dev libclang-rt-18-dev
+          echo "build:linux --action_env=CC=/usr/lib/llvm-18/bin/clang --action_env=CXX=/usr/lib/llvm-18/bin/clang++" >> .bazelrc
+          echo "build:linux --host_action_env=CC=/usr/lib/llvm-18/bin/clang --host_action_env=CXX=/usr/lib/llvm-18/bin/clang++" >> .bazelrc
+          sed -i -e "s%llvm-symbolizer%/usr/lib/llvm-18/bin/llvm-symbolizer%" .bazelrc
       - name: Setup macOS
         if: matrix.os.name == 'macOS'
-        # TODO: We want to symbolize stacks for crashes on CI. Xcode is currently based on LLVM 16
-        # and the macos-13 image has llvm@15 installed:
-        # https://github.com/actions/runner-images/blob/main/images/macos/macos-13-Readme.md
-        #
-        # Not enabled because symbolication does not work on workerd macOS builds yet and running
-        # llvm-symbolizer in the currently broken state causes some tests to time out on the
-        # runner.
-        # Use latest available Xcode version – runner still defaults to 15.0.1.
         run: |
-          sudo xcode-select -s "/Applications/Xcode_15.1.app"
-          # export LLVM_SYMBOLIZER=$(brew --prefix llvm@15)/bin/llvm-symbolizer
-          # sed -i -e "s%llvm-symbolizer%${LLVM_SYMBOLIZER}%" .bazelrc
+          sudo brew install llvm@18
+          export PATH="$(brew --prefix llvm@18)/bin/bin:$PATH"
       - name: Setup Windows
         if: matrix.os.name == 'windows'
         # Set a custom output root directory to avoid long file name issues.

diff --git a/Dockerfile.release b/Dockerfile.release
@@ -13,12 +13,12 @@ RUN wget https://apt.llvm.org/llvm.sh
 # Drop the install step here so we can install just the packages we need.
 RUN sed -i '/apt-get install/d' llvm.sh
 RUN chmod +x llvm.sh
-RUN ./llvm.sh 15
-RUN apt-get install -y --no-install-recommends clang-15 lld-15 libunwind-15 libc++abi1-15 libc++1-15 libc++-15-dev libclang-rt-15-dev
+RUN ./llvm.sh 18
+RUN apt-get install -y --no-install-recommends clang-18 lld-18 libunwind-18 libc++abi1-18 libc++1-18 libc++-18-dev libclang-rt-18-dev
 COPY . .
 
-RUN echo "build:linux --action_env=CC=/usr/lib/llvm-15/bin/clang --action_env=CXX=/usr/lib/llvm-15/bin/clang++" >> .bazelrc
-RUN echo "build:linux --host_action_env=CC=/usr/lib/llvm-15/bin/clang --host_action_env=CXX=/usr/lib/llvm-15/bin/clang++" >> .bazelrc
+RUN echo "build:linux --action_env=CC=/usr/lib/llvm-18/bin/clang --action_env=CXX=/usr/lib/llvm-18/bin/clang++" >> .bazelrc
+RUN echo "build:linux --host_action_env=CC=/usr/lib/llvm-18/bin/clang --host_action_env=CXX=/usr/lib/llvm-18/bin/clang++" >> .bazelrc
 COPY .bazel-cache /bazel-disk-cache
 RUN npm install -g pnpm@latest-7
 RUN pnpm install

diff --git a/README.md b/README.md
@@ -72,14 +72,14 @@ To build `workerd`, you need:
 * [Bazel](https://bazel.build/)
   * If you use [Bazelisk](https://github.com/bazelbuild/bazelisk) (recommended), it will automatically download and use the right version of Bazel for building workerd.
 * On Linux:
-  * We use the clang/LLVM toolchain to build workerd and support version 15 and higher. Earlier versions of clang may still work, but are not officially supported.
-  * Clang 15+ (e.g. package `clang-15` on Debian Bookworm). If clang is installed as `clang-<version>` please create a symlink to it in your PATH named `clang`, or use `--action_env=CC=clang-<version>` on `bazel` command lines to specify the compiler name.
-  
-  * libc++ 15+ (e.g. packages `libc++-15-dev` and `libc++abi-15-dev`)
-  * LLD 15+ (e.g. package `lld-15`). The build may still succeed if a recent version of GNU gold or the system linker is installed, but lld is highly recommended for link performance.
+  * We use the clang/LLVM toolchain to build workerd and support version 18 and higher. Earlier versions of clang may still work, but are not officially supported.
+  * Clang 18+ (e.g. package `clang-18` on Debian Bookworm). If clang is installed as `clang-<version>` please create a symlink to it in your PATH named `clang`, or use `--action_env=CC=clang-<version>` on `bazel` command lines to specify the compiler name.
+
+  * libc++ 18+ (e.g. packages `libc++-18-dev` and `libc++abi-18-dev`)
+  * LLD 18+ (e.g. package `lld-18`). The build may still succeed if a recent version of GNU gold or the system linker is installed, but lld is highly recommended for link performance.
   * `python3`, `python3-distutils`, and `tcl8.6`
 * On macOS:
-  * Xcode 15 installation (available on macOS 13 and higher)
+  * Homebrew installed LLVM 18 package (brew install llvm@18)
   * Homebrew installed `tcl-tk` package (provides Tcl 8.6)
 * On Windows:
   * Install [App Installer](https://learn.microsoft.com/en-us/windows/package-manager/winget/#install-winget)

diff --git a/compile_flags.txt b/compile_flags.txt
@@ -14,8 +14,8 @@
 -Iexternal/perfetto/include/perfetto/base/build_configs/bazel/
 -Iexternal/ssl/src/include
 -Isrc
--isystem/usr/lib/llvm-15/include/c++/v1
--isystem/usr/lib/llvm-15/lib/clang/15.0.7/include
+-isystem/usr/lib/llvm-18/include/c++/v1
+-isystem/usr/lib/llvm-18/lib/clang/18.1.8/include
 -isystem/usr/local/include
 -isystem/usr/include/x86_64-linux-gnu
 -isystem/usr/include

diff --git a/docs/development.md b/docs/development.md
@@ -21,3 +21,10 @@ just prepare
 ```sh
 just compile-commands
 ```
+
+## Code Formatting
+
+workerd code is automatically formatted by clang-format. Run `python ./tools/cross/format.py` to reformat the code
+or use the appropriate IDE extension.
+
+Code formatting is checked before check-in and during `Linting` CI build.
diff --git a/doxyfile b/doxyfile
@@ -1219,8 +1219,8 @@ CLANG_OPTIONS          = -std=c++20     \
                          -I/usr/include/c++/11/bits \
                          -I/usr/include/x86_64-linux-gnu \
                          -I/usr/include/x86_64-linux-gnu/c++/11 \
-                         -I/usr/lib/llvm-15/include/c++/v1 \
-                         -I/usr/lib/llvm-15/lib/clang/15.0.7/include
+                         -I/usr/lib/llvm-18/include/c++/v1 \
+                         -I/usr/lib/llvm-18/lib/clang/15.0.7/include
 
 # If clang assisted parsing is enabled you can provide the clang parser with the
 # path to the directory containing a file called compile_commands.json. This

diff --git a/githooks/pre-commit b/githooks/pre-commit
@@ -10,3 +10,43 @@ then
   echo -e "To commit anyway, use --no-verify\n"
   exit 1
 fi
+
+clang_format_check() {
+  source "$(dirname -- $BASH_SOURCE)/../tools/unix/find-python3.sh"
+  PYTHON_PATH=$(get_python3)
+  if [[ -z "$PYTHON_PATH" ]]; then
+    echo
+    echo "python3 is required for formatting and was not found"
+    echo
+    echo "ERROR: you must either install python3 and try pushing again or run `git push` with `--no-verify`"
+    return 1
+  fi
+
+  set +e
+  $PYTHON_PATH "$(dirname -- $BASH_SOURCE)/../tools/cross/format.py" --check git --staged
+  EXIT_CODE=$?
+  set -e
+  case $EXIT_CODE in
+    0)
+      # No lint.
+      return 0
+      ;;
+    1)
+      echo
+      echo "ERROR: changes staged for commit have lint. Pass '--no-verify' or '-n' to skip."
+      echo
+      echo "To fix lint:"
+      echo "  python3 ./tools/cross/format.py"
+      echo
+      return 1
+      ;;
+    2)
+      echo
+      echo "ERROR: failed to run format.py, Pass '--no-verify' or '-n' to skip."
+      echo
+      return 1
+      ;;
+  esac
+}
+
+clang_format_check
diff --git a/githooks/pre-push b/githooks/pre-push
@@ -0,0 +1,45 @@
+#!/usr/bin/env bash
+
+set -euo pipefail
+
+
+source "$(dirname -- $BASH_SOURCE)/../tools/unix/find-python3.sh"
+PYTHON_PATH=$(get_python3)
+if [[ -z "$PYTHON_PATH" ]]; then
+  echo
+  echo "python3 is required for formatting and was not found"
+  echo
+  echo "ERROR: you must either install python3 and try pushing again or run `git push` with `--no-verify`"
+  exit 1
+fi
+
+while read LOCAL_REF LOCAL_SHA REMOTE_REF REMOTE_SHA
+do
+  git fetch origin master &>/dev/null
+  # Check all local changes, not present in origin/master, for lint.
+  set +e
+  $PYTHON_PATH "$(dirname -- $BASH_SOURCE)/../tools/cross/format.py" --check git --source $LOCAL_SHA --target origin/master
+  EXIT_CODE=$?
+  set -e
+  case $EXIT_CODE in
+    0)
+      # No lint.
+      ;;
+    1)
+      echo
+      echo "ERROR: changes in $LOCAL_REF have lint which may fail CI."
+      echo
+      echo "To fix lint:"
+      echo "  python3 ./tools/cross/format.py"
+      echo
+      exit 1
+      ;;
+    2)
+      echo
+      echo "ERROR: failed to run format.py, Pass '--no-verify' or '-n' to skip."
+      echo
+      exit 1
+      ;;
+  esac
+done
+