diff options
Diffstat (limited to 'STYLE_GUIDE.md')
| -rw-r--r-- | STYLE_GUIDE.md | 303 |
1 files changed, 303 insertions, 0 deletions
diff --git a/STYLE_GUIDE.md b/STYLE_GUIDE.md new file mode 100644 index 0000000..2581b5b --- /dev/null +++ b/STYLE_GUIDE.md @@ -0,0 +1,303 @@ +# Gacrux C Style Guide + +This document describes the coding conventions used throughout the Gacrux chess +engine. It is derived directly from the existing source so that new code matches +the established style. + +## Files & Compilation Units + +### Aggregation via `#include "*.c"` + +Multiple `.c` files are aggregated into a single translation unit by +`#include`-ing them, rather than compiling separately: + +- `src/engine.c` pulls in `engine/threads.c`, `engine/thread.c`, + `engine/moves.c`, `engine/ttable.c`. +- `src/ipc.c` pulls in `ipc/go_args.c`, `ipc/moves.c`, `ipc/engine_message.c`, + `ipc/state.c`, `ipc/threads.c`. +- `src/main.c` and `tests/main.c` append `#include "fen.c"`, + `#include "bitboard.c"`, `#include "random.c"` at the **bottom** of the file, + after `main()`. + +This means each leaf `.c` (e.g. `src/fen.c`, `src/random.c`) is a fragment +compiled only when included, never standalone. + +### Internal-define gating + +Headers expose internal-only declarations behind a `_INTERNAL` guard. The +consuming `.c` file `#define`s the sentinel before including the header: + +```c +#define ENGINE_THREADS_INTERNAL +#include "engine/threads.h" +``` + +Other gates in use: + +- `MOVES_INTERNAL` — `include/engine/moves.h` (`__forloop_*` declarations). +- `IPC_INTERNAL` — `include/ipc/state.h`. + +### Test gating + +`#ifdef TEST_MOD` blocks at the bottom of source files define `test_*` +functions and pull in any extras (e.g. `#include "fen.h"`, +`#include "bitboard.c"`). The test runner (`tests/main.c`) defines +`TEST_MOD` first so all tests are visible. + +## Indentation & Whitespace + +- **2 spaces** per indent level. No tabs. +- No trailing whitespace. +- A single blank line separates most statements; dense blocks omit blank + lines between related short statements. +- Long function signatures wrap one parameter per line, indented 2 spaces, + with the opening `{` on its own line: + +```c +void option_setting_combo( + option_setting_t *target, + char *option_name, + char **combinations, + int combinations_count, + int default_index, + combo_t *combo +) { + ... +} +``` + +When the signature fits, the `{` stays on the same line: + +```c +int main(int argc, char** argv) { + ... +} +``` + +## Braces + +- Opening brace on the same line as `if`/`else`/`for`/`while` when the condition + fits, or on its own line after a multi-line condition. +- `else` on the same line as the closing brace: `} else if (...)`. +- Single-statement bodies often omit braces: + +```c +if (i < MAX_TOKEN_SIZE) cmd->root[i] = 0; +``` + +```c +for (int j = 0; m; j++) { + if (i & (1 << j)) { + occs[i] |= (bitboard_t)1 << __builtin_ctzll(m); + } + m &= m - 1; +} +``` + +## Naming + +| Kind | Convention | Example | +|-----------------|------------------------|-------------------------------------------| +| Functions | `snake_case` | `get_pawn_moves`, `position_make_move` | +| Typedef structs | `name_t` | `position_t`, `move_t`, `ipc_state_t` | +| Named structs | `struct name` | `struct threads`, `struct child_thread_args` | +| Enums/constants | anonymous `enum { }` | `enum { WHITE_KING, ..., PIECE_TYPE_COUNT }` | +| Enum values | `UPPER_SNAKE_CASE` | `WHITE_SHORT_CASTLE`, `BLACK_TURN` | +| Macros/constants| `UPPER_SNAKE_CASE` | `BITMASK_RANK_1`, `TT_BUCKET_SIZE` | +| Private funcs | `_single_prefix` | `_add_move`, `_add_ipc_move` | +| Internal funcs | `__double_prefix` | `__ttable_insert`, `__threads_args_init` | +| File-scope vars | `static` | `static magic_t rook_table[64];` | + +### Struct vs. typedef choice + +- Use `typedef struct { ... } name_t;` (anonymous) for opaque/value types that + are only ever referenced through the typedef — `position_t`, `move_t`, + `move_t`, `ipc_state_t`, `tentry_t`, `magic_t`. +- Use `struct name { ... };` (named, no typedef) when the type must be + identified by name (forward declarations, self-reference, or union members) — + `struct engine_message`, `struct threads`, `struct child_thread_args`. + +### Self-referential structs + +```c +typedef struct engine_message { + atomic_int ready; + ... + struct engine_message *next; +} engine_message_t; +``` + +## Function Lifecycle Convention + +Every "object" type follows an `init` / `deinit` pair. + +- `init` operating on a caller-owned struct takes a pointer: + `void xxx_init(xxx_t *out)` / `void xxx_init(struct xxx *handler)`. +- `deinit` frees internal resources and takes a pointer (or by-value handle for + trivially-copyable types): `void xxx_deinit(xxx_t out)`, + `void uci_cmd_deinit(uci_cmd_t cmd)`. + +## Macros + +### Expression macros (no statement semantics) + +Written without surrounding parentheses, relying on call-site precedence — +notably the bitmask aggregates: + +```c +#define whites(position) \ + position.bitboards[WHITE_KING] | \ + position.bitboards[WHITE_QUEEN] | \ + ... +``` + +Single-expression macros may be parenthesized: + +```c +#define occupied_by(bitboard, index) (bitboard & ((bitboard_t)1 << index)) +``` + +### Variadic wrapper macros + +A thin macro delegates to an underscore-prefixed function, building a +designated-initializer struct inline: + +```c +#define add_move(moves, from, to, ...) _add_move(\ + moves, \ + from, \ + to, \ + (struct add_move_params) { .flags = 0, __VA_ARGS__ }\ +) +``` + +```c +#define ttable_insert(ttable, ...) __ttable_insert( \ + ttable, \ + (struct ttable_insert_args) { __VA_ARGS__ } \ +) +``` + +### Statement macros + +Multi-statement macros use `do { ... } while (0)` (the build system does this): + +```c +#define RUN_CMD(...) do { \ + if (fork() == 0) { \ + char* args[] = __VA_ARGS__; \ + ... \ + } else { \ + wait(&status); \ + } \ +} while (0); +``` + +## Function signatures & parameter style + +- Pointer-to-struct receiver is the first parameter: + `void get_king_moves(moves_t *moves, position_t position)`. +- Small structs/positional params passed by value: `move_t move`, + `position_t position`, `bitboard_t occ`. +- `int`/pointer params are not typedef'd; use `u8`/`u16`/`u32`/`u64` (from + `include/ints.h`) for widths. +- `const`-ness is not heavily used; raw pointers are common even for + read-only access. + +## Enums + +Anonymous enums for constant sets: + +```c +enum { WHITE_KING, WHITE_QUEEN, ..., PIECE_TYPE_COUNT }; +``` + +```c +enum { TT_EXACT, TT_LOWERBOUND, TT_UPPERBOUND }; +``` + +Flag bits use explicit bit shifts: + +```c +enum { + WHITE_SHORT_CASTLE = 1 << 0, + WHITE_LONG_CASTLE = 1 << 1, + ... +}; +``` + +## Error Handling + +- `assert(condition)` for internal invariants and unreachable code: + `assert(0 && "Failed to find magic number")`, `assert(0 && "Invalid argument...")` +- `bool`-returning predicates for validity checks: + `bool check_valid_position(position_t position)`, returns `true`/`false`. +- Lookup functions return `NULL` on miss: `tentry_t *ttable_find(...)`. +- Error-as-data via a `bool failed` struct field: + `struct fen_load { bool failed; position_t position; };` + +## Comments + +- `//` for inline and short comments. +- `/* ... */` for multi-line explanations. +- Trailing inline comments often document field semantics, sometimes aligned: + `i16 score; // depth analysed` +- Explanatory comments about timing constraints appear in the UCI layer: + `// TIMEOUT NOTICE: THE FOLLOWING BLOCK SHOULDN'T TAKE MORE THAN 5 SECONDS` + +## Atomics + +Use `<stdatomic.h>` primitives directly: + +- `atomic_int`, `atomic_uint_fast64_t`, `atomic_flag`. +- `atomic_init(&x, v)`, `atomic_load(&x)`, `atomic_store(&x, v)`. +- `atomic_flag_test_and_set(&flag)` / `atomic_flag_clear(&flag)` for spinlocks. + +## Test Functions + +Embedded test functions: + +- Signature: `bool test_<thing>(void)`. +- Return `true` = pass, `false` = fail. +- Use `assert()` for intermediate checks; `return false` to fail a specific + assertion so the runner can report it. + +```c +bool test_uci_cmd_t() { + uci_cmd_t cmd; + uci_cmd_init(&cmd); + uci_cmd_add(&cmd, "john"); + uci_cmd_add(&cmd, "debug"); + uci_cmd_add(&cmd, "on"); + + if (strncmp(cmd.root, "debug", MAX_TOKEN_SIZE) != 0) { + uci_cmd_deinit(cmd); + return false; + } + ... + uci_cmd_deinit(cmd); + return true; +} +``` + +## Build system (`build.c`) + +- A self-rebuilding `nob.h`-style driver. +- Defines `CC` macro: `#define CC "gcc", "-fsanitize=address", "-g", GLOBAL_INCLUDE` +- `needs_rebuild(bin, src)` compares mtimes via `stat()`. +- Subprocesses launched via `fork()`/`execvp()` inside a `RUN_CMD` `do/while(0)` + macro, with `wait(&status)`. +- Output object files live in `output/`. + +## Include ordering + +The canonical order (see `src/uci.c`, `src/main.c`) is: + +1. System headers: `<string.h>`, `<stdio.h>`, `<stdlib.h>`, `<unistd.h>`, etc. +2. Blank line. +3. Project headers using path-qualified includes: `"uci/internals/command.h"`, + `"ipc/state.h"`, `"engine/magic.h"`. + +Some older files (`src/bitboard.c`, `src/engine/moves.c`) place the project +header first or a system header last; prefer the system-first / blank-line / +project order for new files. |