add clang-format

This introduces clang-format to enforce a consistent code style for Stockfish. Having a documented and consistent style across the code will make contributing easier for new developers, and will make larger changes to the codebase easier to make. To facilitate formatting, this PR includes a Makefile target (`make format`) to format the code, this requires clang-format (version 17 currently) to be installed locally. Installing clang-format is straightforward on most OS and distros (e.g. with https://apt.llvm.org/, brew install clang-format, etc), as this is part of quite commonly used suite of tools and compilers (llvm / clang). Additionally, a CI action is present that will verify if the code requires formatting, and comment on the PR as needed. Initially, correct formatting is not required, it will be done by maintainers as part of the merge or in later commits, but obviously this is encouraged. fixes #3608 closes #4790 Co-Authored-By: Joost VandeVondele <[email protected]>
official-stockfish · Oct 22, 2023 · 2d0237d · 2d0237d
1 parent 8366ec4
commit 2d0237d
Show file tree

Hide file tree

Showing 49 changed files with 6,963 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/clang-format-action@f62da5e3d3a2d88ff364771d9d938773a618ab5e
+        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@1d3973dc4b8e1399c0620d3f2b1aa5e795465308
+        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@1d3973dc4b8e1399c0620d3f2b1aa5e795465308
+        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,9 @@ 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 version 17 to be installed on your system.
 
 ## 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