add clang-format

This introduces clang-format to enforce a consistent code style for Stockfish. The lack of an automated code formatter has been a long standing issue #3608. This PR includes a Makefile target to format the code accordingly and write a comment on the PR on GitHub if any formatting changes are required. closes #3608 Co-Authored-By: Joost VandeVondele <[email protected]>
official-stockfish · Oct 22, 2023 · cb8e2af · cb8e2af
1 parent 8366ec4
commit cb8e2af
Show file tree

Hide file tree

Showing 49 changed files with 6,964 additions and 6,757 deletions.
diff --git a/.clang-format b/.clang-format
@@ -0,0 +1,44 @@
+AccessModifierOffset: -1
+AlignAfterOpenBracket: Align
+AlignConsecutiveAssignments: Consecutive
+AlignConsecutiveDeclarations: Consecutive
+AlignEscapedNewlines: DontAlign
+AlignOperands: AlignAfterOperator
+AlignTrailingComments: true
+AllowAllParametersOfDeclarationOnNextLine: true
+AllowShortCaseLabelsOnASingleLine: false
+AllowShortEnumsOnASingleLine: false
+AllowShortIfStatementsOnASingleLine: false
+AlwaysBreakTemplateDeclarations: Yes
+BasedOnStyle: WebKit
+BitFieldColonSpacing: After
+BinPackParameters: false
+BreakBeforeBinaryOperators: NonAssignment
+BreakBeforeBraces: Custom
+BraceWrapping:
+  AfterFunction: false 
+  AfterClass: false
+  AfterControlStatement: true
+  BeforeElse: true
+BreakBeforeTernaryOperators: true
+BreakConstructorInitializers: AfterColon
+BreakStringLiterals: false
+ColumnLimit: 100
+ContinuationIndentWidth: 2
+Cpp11BracedListStyle: true
+IndentGotoLabels: false
+IndentPPDirectives: BeforeHash
+IndentWidth: 4
+MaxEmptyLinesToKeep: 2
+NamespaceIndentation: None
+PackConstructorInitializers: Never
+ReflowComments: false
+SortIncludes: false
+SortUsingDeclarations: false
+SpaceAfterCStyleCast: true
+SpaceAfterTemplateKeyword: false
+SpaceBeforeCaseColon: true
+SpaceBeforeCpp11BracedList: false
+SpaceBeforeInheritanceColon: false
+SpaceInEmptyBlock: false
+SpacesBeforeTrailingComments: 2
diff --git a/.github/workflows/stockfish_format_check.yml b/.github/workflows/stockfish_format_check.yml
@@ -0,0 +1,51 @@
+# This workflow will run clang-format and comment on the PR.
+# Because of security reasons, it is crucial that this workflow
+# executes no shell script nor runs make.
+# Read this before editing: https://securitylab.github.com/research/github-actions-preventing-pwn-requests/
+
+name: Stockfish
+on:
+  pull_request_target:
+    branches:
+      - 'master'
+    paths:
+      - '**.cpp'
+      - '**.h'
+jobs:
+  Stockfish:
+    name: clang-format check
+    runs-on: ubuntu-20.04
+    steps:
+      - uses: actions/checkout@v3
+        with:
+          ref: ${{ github.event.pull_request.head.sha }}
+
+      - name: Run clang-format style check
+        uses: jidicula/[email protected]
+        id: clang-format
+        continue-on-error: true
+        with:
+          clang-format-version: '17'
+          exclude-regex: 'incbin'
+
+      - name: Comment on PR
+        if: steps.clang-format.outcome == 'failure'
+        uses: thollander/actions-comment-pull-request@v2
+        with:
+          message: |
+            clang-format 17 needs to be run on this PR.
+            If you do not have clang-format installed, the maintainer will run it when merging.
+            For the exact version please see https://packages.ubuntu.com/mantic/clang-format-17.
+
+            _(execution **${{ github.run_id }}** / attempt **${{ github.run_attempt }}**)_
+          comment_tag: execution
+
+      - name: Comment on PR
+        if: steps.clang-format.outcome != 'failure'
+        uses: thollander/actions-comment-pull-request@v2
+        with:
+          message: |
+            _(execution **${{ github.run_id }}** / attempt **${{ github.run_attempt }}**)_
+          create_if_not_exists: false
+          comment_tag: execution
+          mode: delete
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -57,8 +57,10 @@ discussion._
 
 ## Code Style
 
-We do not have a strict code style. But it is best to stick to the existing
-style of the file you are editing.
+Changes to Stockfish C++ code should respect our coding style defined by
+[.clang-format](.clang-format). You can format your changes by running
+`make format`. This requires clang-format to be installed on your system.
+Preferably version 17 or higher.
 
 ## Community and Communication
 

diff --git a/src/Makefile b/src/Makefile
@@ -57,6 +57,14 @@ SRCS = benchmark.cpp bitboard.cpp evaluate.cpp main.cpp \
 	search.cpp thread.cpp timeman.cpp tt.cpp uci.cpp ucioption.cpp tune.cpp syzygy/tbprobe.cpp \
 	nnue/evaluate_nnue.cpp nnue/features/half_ka_v2_hm.cpp
 
+HEADERS = benchmark.h bitboard.h evaluate.h misc.h movegen.h movepick.h \
+		nnue/evaluate_nnue.h nnue/features/half_ka_v2_hm.h nnue/layers/affine_transform.h \
+		nnue/layers/affine_transform_sparse_input.h nnue/layers/clipped_relu.h nnue/layers/simd.h \
+		nnue/layers/sqr_clipped_relu.h nnue/nnue_accumulator.h nnue/nnue_architecture.h \
+		nnue/nnue_common.h nnue/nnue_feature_transformer.h position.h \
+		search.h syzygy/tbprobe.h thread.h thread_win32_osx.h timeman.h \
+		tt.h tune.h types.h uci.h
+
 OBJS = $(notdir $(SRCS:.cpp=.o))
 
 VPATH = syzygy:nnue:nnue/features
@@ -145,6 +153,12 @@ dotprod = no
 arm_version = 0
 STRIP = strip
 
+ifneq ($(shell command -v clang-format-17),)
+	CLANG-FORMAT = clang-format-17
+else
+	CLANG-FORMAT = clang-format
+endif
+
 ### 2.2 Architecture specific
 
 ifeq ($(findstring x86,$(ARCH)),x86)
@@ -936,6 +950,9 @@ net: netvariables
 		fi; \
 	fi; \
 
+format:
+	$(CLANG-FORMAT) -i $(SRCS) $(HEADERS) -style=file:../.clang-format
+
 # default target
 default:
 	help

diff --git a/src/benchmark.cpp b/src/benchmark.cpp
@@ -27,6 +27,7 @@
 
 namespace {
 
+// clang-format off
 const std::vector<std::string> Defaults = {
   "setoption name UCI_Chess960 value false",
   "rnbqkbnr/pppppppp/8/8/8/8/PPPPPPPP/RNBQKBNR w KQkq - 0 1",
@@ -90,8 +91,9 @@ const std::vector<std::string> Defaults = {
   "nqbnrkrb/pppppppp/8/8/8/8/PPPPPPPP/NQBNRKRB w KQkq - 0 1",
   "setoption name UCI_Chess960 value false"
 };
+// clang-format on
 
-} // namespace
+}  // namespace
 
 namespace Stockfish {
 
@@ -109,56 +111,56 @@ namespace Stockfish {
 
 std::vector<std::string> setup_bench(const Position& current, std::istream& is) {
 
-  std::vector<std::string> fens, list;
-  std::string go, token;
+    std::vector<std::string> fens, list;
+    std::string              go, token;
 
-  // Assign default values to missing arguments
-  std::string ttSize    = (is >> token) ? token : "16";
-  std::string threads   = (is >> token) ? token : "1";
-  std::string limit     = (is >> token) ? token : "13";
-  std::string fenFile   = (is >> token) ? token : "default";
-  std::string limitType = (is >> token) ? token : "depth";
+    // Assign default values to missing arguments
+    std::string ttSize    = (is >> token) ? token : "16";
+    std::string threads   = (is >> token) ? token : "1";
+    std::string limit     = (is >> token) ? token : "13";
+    std::string fenFile   = (is >> token) ? token : "default";
+    std::string limitType = (is >> token) ? token : "depth";
 
-  go = limitType == "eval" ? "eval" : "go " + limitType + " " + limit;
+    go = limitType == "eval" ? "eval" : "go " + limitType + " " + limit;
 
-  if (fenFile == "default")
-      fens = Defaults;
+    if (fenFile == "default")
+        fens = Defaults;
 
-  else if (fenFile == "current")
-      fens.push_back(current.fen());
+    else if (fenFile == "current")
+        fens.push_back(current.fen());
 
-  else
-  {
-      std::string fen;
-      std::ifstream file(fenFile);
+    else
+    {
+        std::string   fen;
+        std::ifstream file(fenFile);
 
-      if (!file.is_open())
-      {
-          std::cerr << "Unable to open file " << fenFile << std::endl;
-          exit(EXIT_FAILURE);
-      }
+        if (!file.is_open())
+        {
+            std::cerr << "Unable to open file " << fenFile << std::endl;
+            exit(EXIT_FAILURE);
+        }
 
-      while (getline(file, fen))
-          if (!fen.empty())
-              fens.push_back(fen);
+        while (getline(file, fen))
+            if (!fen.empty())
+                fens.push_back(fen);
 
-      file.close();
-  }
+        file.close();
+    }
 
-  list.emplace_back("setoption name Threads value " + threads);
-  list.emplace_back("setoption name Hash value " + ttSize);
-  list.emplace_back("ucinewgame");
+    list.emplace_back("setoption name Threads value " + threads);
+    list.emplace_back("setoption name Hash value " + ttSize);
+    list.emplace_back("ucinewgame");
 
-  for (const std::string& fen : fens)
-      if (fen.find("setoption") != std::string::npos)
-          list.emplace_back(fen);
-      else
-      {
-          list.emplace_back("position fen " + fen);
-          list.emplace_back(go);
-      }
+    for (const std::string& fen : fens)
+        if (fen.find("setoption") != std::string::npos)
+            list.emplace_back(fen);
+        else
+        {
+            list.emplace_back("position fen " + fen);
+            list.emplace_back(go);
+        }
 
-  return list;
+    return list;
 }
 
-} // namespace Stockfish
+}  // namespace Stockfish
diff --git a/src/benchmark.h b/src/benchmark.h
@@ -29,6 +29,6 @@ class Position;
 
 std::vector<std::string> setup_bench(const Position&, std::istream&);
 
-} // namespace Stockfish
+}  // namespace Stockfish
 
-#endif // #ifndef BENCHMARK_H_INCLUDED
+#endif  // #ifndef BENCHMARK_H_INCLUDED