From e24ccc66cc55121cb1041b7a7fd5aaeff5844970 Mon Sep 17 00:00:00 2001 From: Felix Morgner Date: Sun, 30 Aug 2026 12:17:19 +0200 Subject: doc: clean up and move code style docs --- CODESTYLE.md | 491 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 491 insertions(+) create mode 100644 CODESTYLE.md (limited to 'CODESTYLE.md') diff --git a/CODESTYLE.md b/CODESTYLE.md new file mode 100644 index 00000000..4401978a --- /dev/null +++ b/CODESTYLE.md @@ -0,0 +1,491 @@ +# TeachOS C++ Code Style Guide + +This document codifies the C++ coding idioms used throughout the TeachOS kernel. +It covers language usage, ownership and lifetime models, algorithm selection, error propagation, class design, and naming conventions. +It does **not** cover token-level formatting, which is enforced automatically by the `.clang-format` configuration. + +--- + +## 1. Language Standard and Vocabulary + +TeachOS targets **C++23** without compiler extensions. +Standard library features may be used freely in tests only. +All other parts of the codebase rely on a freestanding variant of the standard library as shipped with the toolchain. +Types that are not part of the toolchain's freestanding standard library are provided by the `kstd` library. +Below is an overview of `kstd` replacements to be used in the **non-test** kernel code, including the bundled support libraries. +This list is does not claim completeness. + +| Concept | Preferred | Replaces | +|--------------------|-------------------------|---------------------------------------| +| Dynamic array | `kstd::vector` | `std::vector` | +| Strings | `kstd::string` | `std::string` | +| Non-owning pointer | `kstd::observer_ptr` | raw `T*` for ownership-neutral access | +| Shared ownership | `kstd::shared_ptr` | `std::shared_ptr` | +| Unique ownership | `kstd::unique_ptr` | `std::unique_ptr` | +| Printing | `kstd::println(...)` | `std::println(...)`, ... | +| String Formatting | `kstd::format(...)` | `std::format(...)`, ... | + +Common standard library parts that are available in the freestanding implementation include but are not limited to: +- `std::string_view` +- `std::span` +- `std::array` +- `std::optional` +- `std::byte` +- `std::ranges` +- `std::views` + +These parts may be used freely throughout the codebase. + +## 2. Function Declarations — Trailing Return Types + +**All** functions and member functions use trailing return type syntax, including those returning `void`. +See the following code snippet for examples: + +```cpp +// Correct +auto device_registry::get() -> device_registry &; +auto bitmap_is_set(std::span bitmap, std::size_t index) -> bool; +auto init() -> void; + +// Wrong +device_registry & device_registry::get(); +bool bitmap_is_set(std::span bitmap, std::size_t index); +void init(); +``` + +This rule applies to: free functions, member functions, lambdas with explicit return types, and virtual functions. +The only exception is constructors and destructors, which have no return type at all. + +## 3. Parameter Passing Conventions + +The choice of passing convention encodes intent and must be consistent. + +### 3.1 View and cheaply copyable types — pass by value + +Types that are designed to be non-owning views or are trivially copyable must be passed and returned **by value**. +Passing them by `const &` is redundant and adds a pointer indirection with no benefit. + +This includes but is not limited to: +- `std::string_view` +- `std::span` +- `kstd::observer_ptr` +- `kstd::bytes` and similar unit types +- `kapi::capabilities::facet_id` +- `kapi::memory::page` +- `kapi::memory::frame` +- `kapi::memory::physical_address` +- `kapi::memory::linear_address` + +See the following code snippet for examples: + +```cpp +// Correct +auto resolve(kapi::capabilities::facet_id id, std::string_view name) -> std::observer_ptr; +auto has(std::span data) -> bool; + +// Wrong +auto resolve(kapi::capabilities::facet_id const & id, std::string_view const & name) -> std::observer_ptr; +``` + +### 3.2 Large or non-trivially copyable types — pass by `const &` + +For types that own heap memory or are non-trivially copyable, use `const &` when the callee does not take ownership. + +See the following code snippet for examples: + +```cpp +auto do_publish(kstd::string const & name) -> kstd::result; +auto add_child(kstd::string const & child_name) -> void; +``` + +### 3.3 Sink parameters — pass by value and move + +When a function is designed to take **ownership** of an argument, accept it by value and move it into its destination. +This makes the transfer explicit at the call site. + +See the following code snippet for examples: + +```cpp +// In the header +auto add_child(kstd::shared_ptr child) -> void; + +// In the implementation +auto bus::add_child(kstd::shared_ptr child) -> void +{ + m_devices.push_back(std::move(child)); // ownership transferred here +} +``` + +### 3.4 Mutable subsystem references — pass by non-const reference + +Services and subsystems that are mutated in-place (e.g., `page_mapper &`, `driver_state &`, `kapi::devices::bus &`) are passed by non-const reference. +This expresses that the function operates on a shared, mutable context. + +See the following code snippet for examples: + +```cpp +auto remap_kernel(kapi::memory::page_mapper & mapper) -> void; +auto add_directory_entry(inode & directory, inode & child, driver_state & state, write_batch & batch) -> kstd::result; +``` + +## 4. Error Handling + +TeachOS kernel code, and all library code used by it, cannot make use of exceptions. +Use of exception related keywords, `try`, `catch`, `throw` in kernel code will cause compilation to fail. +However, exceptions are allowed in test code. + +### 4.1 Recoverable errors — `kstd::result` + +Functions that can fail in an expected, recoverable way must return `kstd::result`. +Use the `kstd::success()` and `kstd::failure()` helpers consistently. +Define per-subsystem error code if necessary. + +See the following code snippet for examples: + +```cpp +auto mount(kstd::shared_ptr parent) -> kstd::result, state *>>; + +// In the implementation +if (!device) +{ + return kstd::failure(make_error_code(kstd::errc::invalid_argument)); +} +return kstd::success(result_value); +``` + +Callers must check the result before using its value. The idiomatic check is: + +See the following code snippet for examples: + +```cpp +auto result = some_function(); +if (!result) +{ + return kstd::failure(result.error()); // propagate +} +// use *result +``` + +Monadic composition (`transform`, `and_then`, `or_else`) is preferred over manual if-check-and-return chains when it produces clearer code. + +### 4.2 Unrecoverable errors — `kapi::system::panic` + +Violations of kernel invariants (e.g., a subsystem used before being initialized, +OOM during boot) call `kapi::system::panic(...)`. +`panic` is `[[noreturn]]`. +It must not be used for recoverable errors. + +Log prefix convention: `[SUBSYSTEM:TAG] message`. Examples: `[OS:DEV]`, `[OS:VFS]`, `[ARCH:DRV]`. + +See the following code snippet for examples: + +```cpp +if (!instance) +{ + system::panic("[OS:DEV] Device registry has not been initialized."); +} +``` + +## 5. Ownership and Lifetime + +### 5.1 Shared ownership — `kstd::shared_ptr` + +Use `kstd::shared_ptr` when a resource is co-owned by multiple subsystems and its lifetime must be extended by any of them (e.g., `device`, `inode`, `dentry`). +Weak back-references that must not extend lifetime use `kstd::weak_ptr`. + +### 5.2 Non-owning references — `kstd::observer_ptr` and raw references + +Use `kstd::observer_ptr` to express a non-owning pointer where null is a valid state and the holder has no say in the lifetime of the target. +Use a raw reference (`T &` or `T const &`) when null is not valid and the reference is short-lived (i.e. a function parameter or a local alias). + +Never use raw `T *` to mean "sometimes I own this, sometimes I don't". +Ownership must be expressed unambiguously through the pointer type. + +### 5.3 Device driver data — `kstd::shared_ptr` + +Drivers attach their state to a `device` using an untyped `kstd::shared_ptr` via `device::set_driver_data`. +A driver retrieves its state via `device::driver_data`. +This allows the device tree to destroy driver data automatically when the device is released, without the device knowing the concrete driver type. + +## 6. Algorithm and Range Usage + +### 6.1 Prefer `std::ranges` algorithms over manual loops + +When processing a range to search, filter, transform, or reduce, use the appropriate `std::ranges` algorithm or view pipeline instead of writing a raw `for` loop. + +See the following code snippet for examples: + +```cpp +// Correct +auto already_published = std::ranges::any_of(m_entries, [&](auto const & entry) { + return entry.id() == id && entry.device().get() == device.get(); +}); + +std::ranges::for_each(observers, [&](auto observer) { /* ... */ }); + +// Wrong — manual linear scan for a boolean result +for (auto const & entry : m_entries) +{ + if (entry.id() == id && entry.device().get() == device.get()) + { + return true; + } +} +return false; +``` + +Acceptable uses of explicit loops include: +- Accumulation or mutation that modifies state in-place and cannot be cleanly expressed with a ranges algorithm. +- Low-level routines dealing with raw memory arithmetic (allocators, page mappers). +- Iterator-pair loops in library internals (`kstd::vector`, `kstd::basic_string`). + +### 6.2 Prefer view composition over intermediate containers + +Build processing pipelines using `std::views::filter`, `std::views::transform`, `std::views::reverse`, `std::views::split`, and `std::ranges::subrange` rather than materialising intermediate vectors. + +See the following code snippet for examples: + +```cpp +// Correct +auto descriptors = std::span{&__start_platform_drivers, &__stop_platform_drivers} + | std::views::filter([](auto p) { return p != nullptr; }); + +auto modules_view = std::ranges::subrange(begin(), end()) + | std::views::filter(filter_modules) + | std::views::transform(transform_module); +``` + +### 6.3 Do not call the same function twice to avoid storing the result + +If an intermediate value is needed more than once, store it in a local variable. +This applies especially to factory calls and heap allocations. + +```cpp +// Wrong — double invocation, two allocations, different objects +for (auto driver : descriptors) +{ + kstd::println("registering driver '{}'", driver->make_instance()->name()); + registry.add(driver->make_instance()); +} + +// Correct +for (auto driver : descriptors) +{ + auto instance = driver->make_instance(); + kstd::println("registering driver '{}'", instance->name()); + registry.add(std::move(instance)); +} +``` + +## 7. Class and Struct Design + +### 7.1 Prefer `struct` over `class` + +The entire codebase uses `struct` for all type definitions with explicit `private:` +sections where necessary. +Do not introduce `class`. + +### 7.2 Member ordering within a type + +Follow this ordering within a `struct`: + +1. Nested types and type aliases. +2. Static data members and static constexpr constants (e.g., `static constexpr auto id = ...`). +3. Constructors and destructor. +4. Public member functions. +5. `protected:` section with virtual hooks. +6. `private:` section with helper functions, then data members. + +Data members are always in the `private` section and always prefixed with `m_`. + +See the following code snippet for examples: + +```cpp +struct facet_registry +{ + struct entry { /* ... */ }; // 1. nested type + + facet_registry() = default; // 3. constructor + + auto static init() -> void; // 4. public interface + auto static get() -> facet_registry &; + auto publish(...) -> kstd::result; + +private: + auto do_publish(...) -> kstd::result; // 6a. private helpers + + mutable tracked_mutex m_lock{}; // 6b. data members, m_ prefix + kstd::vector m_entries; +}; +``` + +### 7.3 `explicit` on single-argument constructors + +Mark every single-argument constructor `explicit` unless an implicit conversion is intentional and documented. +A deliberate implicit constructor must be accompanied by a comment explaining the decision. + +See the following code snippet for examples: + +```cpp +// Correct +explicit device(kstd::string const & name); +constexpr explicit facet_id(std::string_view name); + +// Intentional implicit — documented at the declaration +//! This constructor allows implicit conversion from chunk<...> to page for +//! convenience. It is deliberately not explicit. +constexpr page(chunk other) : chunk{other} {} +``` + +### 7.4 Declare deleted special members explicitly + +If a type is not copyable or not movable, declare the deleted special members explicitly rather than relying on implicit suppression. + +See the following code snippet for examples: + +```cpp +device(device const &) = delete; +auto operator=(device const &) -> device & = delete; +``` + +### 7.5 Virtual destructors + +Every base class with virtual member functions must have a `virtual` destructor, defaulted if not otherwise needed. + +See the following code snippet for examples: + +```cpp +virtual ~driver_descriptor() = default; +virtual ~facet_registry_observer() = default; +``` + +## 8. Static Singletons + +Several subsystems expose a single global instance via an `init()`/`get()` pair. +Follow this pattern: + +- Store the instance in an anonymous namespace as a `constinit std::optional`. +- `init()` asserts the instance is not yet constructed, then `emplace()`s it. +- `get()` asserts the instance exists and returns a reference to it. +- Both functions panic on violation rather than returning an error code, because incorrect call order is a programming error, not a recoverable runtime condition. + +See the following code snippet for examples: + +```cpp +namespace +{ + auto constinit instance = std::optional{}; +} + +auto device_registry::init() -> void +{ + if (instance) + { + system::panic("[OS:DEV] Device registry has already been initialized."); + } + instance.emplace(); +} + +auto device_registry::get() -> device_registry & +{ + if (!instance) + { + system::panic("[OS:DEV] Device registry has not been initialized."); + } + return *instance; +} +``` + +## 9. Enumerations + +All enumerations use `enum struct` (scoped enums), never plain `enum`. +Specify the underlying type explicitly when the representation matters (e.g., for hardware register fields). + +See the following code snippet for examples: + +```cpp +// Correct +enum struct state +{ + uninitialized, + present, + bound, +}; + +// Wrong +enum state { uninitialized, present, bound }; +``` + +## 10. Naming + +| Symbol | Convention | Example | +|-----------------------------------|---------------------------|-----------------------------------| +| Types (struct, enum) | `lower_case` | `device_registry`, `facet_id` | +| Functions and methods | `lower_case` | `add_child`, `make_instance` | +| Local variables | `lower_case` | `entry`, `block_index` | +| Private data members | `m_` prefix, `lower_case` | `m_entries`, `m_driver_data` | +| Template type parameters | `CamelCase` | `ValueType`, `FacetType` | +| Constants and constexpr variables | `lower_case` | `page_size`, `direct_block_count` | +| Type aliases | `lower_case` | `value_type`, `size_type` | +| Namespaces | `lower_case` | `kapi::devices`, `kernel::vfs` | + +Namespaces reflect directory structure: `kernel::vfs`, `kernel::filesystems::ext2`, `arch::devices`, etc. + +## 11. `[[nodiscard]]` + +Mark any function `[[nodiscard]]` whose return value the caller should not silently discard. +This includes in particular: + +- All functions returning `kstd::result`. +- All query functions (getters, lookups) that return computed data. +- Factory functions and builder utilities. + +See the following code snippet for examples: + +```cpp +[[nodiscard]] auto resolve(kapi::capabilities::facet_id id, std::string_view name) -> void *; +[[nodiscard]] auto children() const -> kstd::vector>; +[[nodiscard]] auto request_resource(resource_type type, std::size_t index = 0) const -> kstd::result; +``` + +## 12. `constexpr` and `constinit` + +Mark functions `constexpr` whenever they can be evaluated at compile time or in constant expressions, even if they are also called at runtime. +Mark module-scope variables `constinit` to guarantee zero-initialization before any dynamic initialization runs. + +## 13. Documentation + +Every non-trivial public type, function, and data member must be documented with a Doxygen comment using the `//!` line style. + +See the following code snippet for examples: + + +```cpp +//! A brief one-line description. +//! +//! Optional longer description providing context. +//! +//! @param name Description of the parameter. +//! @return Description of the return value. +//! @warning Any important warnings or preconditions. +auto add_child(kstd::shared_ptr child) -> void; +``` + +Doxygen grouping (`@name`, `@{`, `@}`) may be used to organise the API surface into logical sections visible in generated documentation. + +## 14. Header Guards + +All headers use traditional include guards, not `#pragma once`. +The guard name follows the pattern `TEACHOS___HPP`, where each path component is uppercased and separators are replaced by `_`. +The only exception to this pattern are the bundled libraries, which follow the pattern `___HPP`. + +See the following code snippet for examples: + +```cpp +#ifndef TEACHOS_KAPI_DEVICES_BUS_HPP +#define TEACHOS_KAPI_DEVICES_BUS_HPP +// ... +#endif +``` + +The closing `#endif` carries no comment with the guard name. -- cgit v1.2.3