Cleanups

[ipapadop]

No description provided.

[qodo-code-review]

Review Summary by Qodo

Modernize codebase with SPDX headers, nodiscard attributes, and C++20 concepts

✨ Enhancement 📝 Documentation

Walkthroughs

Description

• Update all file headers to SPDX license identifier format
• Add [[nodiscard]] attribute to expression operators and functions
• Apply [[no_unique_address]] to storage members in constant and variable classes
• Simplify evaluation logic by removing intermediate variables
• Modernize Deferred concept using C++20 concepts instead of type traits
• Add code formatting infrastructure with clang-format CMake target

Diagram

flowchart LR
  A["License Headers"] -->|Convert to SPDX| B["Updated Headers"]
  C["Operators & Functions"] -->|Add nodiscard| D["Compiler Warnings"]
  E["Storage Members"] -->|Add no_unique_address| F["Optimized Layout"]
  G["Type Traits"] -->|Replace with Concepts| H["C++20 Deferred Concept"]
  I["Code Quality"] -->|Add Formatting| J["clang-format Integration"]

Loading

File Changes

1. include/deferred/type_traits/is_deferred.hpp ✨ Enhancement +10/-33

Modernize Deferred concept with C++20

include/deferred/type_traits/is_deferred.hpp

---

2. include/deferred/operators.hpp ✨ Enhancement +28/-35

Add nodiscard to all operator overloads

include/deferred/operators.hpp

---

3. include/deferred/expression.hpp ✨ Enhancement +7/-12

Add nodiscard to operator() and accessors

include/deferred/expression.hpp

---

View more (35)

4. include/deferred/constant.hpp ✨ Enhancement +6/-12

Add nodiscard and no_unique_address attributes

include/deferred/constant.hpp

---

5. include/deferred/variable.hpp ✨ Enhancement +25/-14

Add nodiscard, no_unique_address, and rvalue operator()

include/deferred/variable.hpp

---

6. include/deferred/evaluate.hpp ✨ Enhancement +7/-17

Simplify evaluation logic and update return types

include/deferred/evaluate.hpp

---

7. include/deferred/conditional.hpp ✨ Enhancement +5/-11

Add nodiscard to conditional operators

include/deferred/conditional.hpp

---

8. include/deferred/switch.hpp ✨ Enhancement +20/-20

Add nodiscard and improve visitor pattern

include/deferred/switch.hpp

---

9. include/deferred/while.hpp ✨ Enhancement +20/-12

Add rvalue operator() and improve documentation

include/deferred/while.hpp

---

10. include/deferred/invoke.hpp ✨ Enhancement +3/-9

Add nodiscard to invoke function

include/deferred/invoke.hpp

---

11. include/deferred/make_function_object.hpp ✨ Enhancement +6/-10

Add nodiscard to function object creation

include/deferred/make_function_object.hpp

---

12. include/deferred/apply.hpp 📝 Documentation +3/-13

Update license header to SPDX format

include/deferred/apply.hpp

---

13. include/deferred/type_name.hpp ✨ Enhancement +4/-10

Add nodiscard to type_name functions

include/deferred/type_name.hpp

---

14. CMakeLists.txt ⚙️ Configuration changes +15/-0

Add clang-format CMake target for code formatting

CMakeLists.txt

---

15. scripts/format_changed.sh ⚙️ Configuration changes +17/-0

Add script to format changed files with clang-format

scripts/format_changed.sh

---

16. README.md 📝 Documentation +14/-4

Add formatting instructions and update documentation

README.md

---

17. AGENTS.md 📝 Documentation +17/-0

Add formatting guidelines and verification requirements

AGENTS.md

---

18. examples/saxpy/saxpy_valarray.cpp 📝 Documentation +2/-7

Update license header to SPDX format

examples/saxpy/saxpy_valarray.cpp

---

19. examples/saxpy/saxpy_vector.cpp 📝 Documentation +2/-7

Update license header to SPDX format

examples/saxpy/saxpy_vector.cpp

---

20. examples/trivial/main.cpp 📝 Documentation +2/-7

Update license header to SPDX format

examples/trivial/main.cpp

---

21. test/integration/apply.cpp 📝 Documentation +2/-7

Update license header to SPDX format

test/integration/apply.cpp

---

22. test/integration/conditional.cpp 📝 Documentation +2/-7

Update license header to SPDX format

test/integration/conditional.cpp

---

23. test/integration/constant.cpp 📝 Documentation +2/-7

Update license header to SPDX format

test/integration/constant.cpp

---

24. test/integration/main.cpp 📝 Documentation +2/-7

Update license header to SPDX format

test/integration/main.cpp

---

25. test/integration/operators.cpp 📝 Documentation +2/-7

Update license header to SPDX format

test/integration/operators.cpp

---

26. test/integration/switch.cpp 📝 Documentation +2/-7

Update license header to SPDX format

test/integration/switch.cpp

---

27. test/integration/variable.cpp 📝 Documentation +2/-7

Update license header to SPDX format

test/integration/variable.cpp

---

28. test/integration/while.cpp 📝 Documentation +2/-7

Update license header to SPDX format

test/integration/while.cpp

---

29. test/unit/conditional.cpp 📝 Documentation +2/-7

Update license header to SPDX format

test/unit/conditional.cpp

---

30. test/unit/constant.cpp 📝 Documentation +2/-7

Update license header to SPDX format

test/unit/constant.cpp

---

31. test/unit/invoke.cpp 📝 Documentation +2/-7

Update license header to SPDX format

test/unit/invoke.cpp

---

32. test/unit/main.cpp 📝 Documentation +2/-7

Update license header to SPDX format

test/unit/main.cpp

---

33. test/unit/make_function_object.cpp 📝 Documentation +2/-7

Update license header to SPDX format

test/unit/make_function_object.cpp

---

34. test/unit/switch.cpp 📝 Documentation +2/-7

Update license header to SPDX format

test/unit/switch.cpp

---

35. test/unit/variable.cpp 📝 Documentation +2/-7

Update license header to SPDX format

test/unit/variable.cpp

---

36. test/unit/while.cpp 📝 Documentation +2/-7

Update license header to SPDX format

test/unit/while.cpp

---

37. include/deferred/deferred.hpp Additional files +2/-8

...

include/deferred/deferred.hpp

---

38. include/deferred/type_traits/is_constant_expression.hpp Additional files +2/-8

...

include/deferred/type_traits/is_constant_expression.hpp

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (3)   📘 Rule violations (1)   📎 Requirement gaps (0)

🐞\ ≡ Correctness (1) ☼ Reliability (2)

📘\ ⚙ Maintainability (1)

1. SPDX replaces MIT header 📘 ⚙

Description

Multiple modified C++ sources/headers replace the repository’s standard MIT license header block
with only SPDX identifiers. This violates the requirement that every modified source file includes
the standard MIT license header.

Code

include/deferred/deferred.hpp[R1-2]

+// SPDX-FileCopyrightText: 2019-2026 Yiannis Papadopoulos <giannis.papadopoulos@gmail.com>
+// SPDX-License-Identifier: MIT

Evidence

PR Compliance ID 5 requires the standard MIT license header in every modified source file; the
changed files show only // SPDX-... lines at the top instead of the standard MIT header block.

AGENTS.md
include/deferred/deferred.hpp[1-2]
test/unit/main.cpp[1-2]
examples/trivial/main.cpp[1-2]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Modified source files no longer include the repository’s standard MIT license header block; they only contain SPDX identifier lines.
## Issue Context
Compliance requires that every modified/added source file includes the standard MIT license header text.
## Fix Focus Areas
- include/deferred/deferred.hpp[1-8]
- test/unit/main.cpp[1-9]
- examples/trivial/main.cpp[1-8]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. evaluate drops references 🐞 ≡

Description

deferred::evaluate() and deferred::recursive_evaluate() now return auto, which decays references
and forces copies even when the evaluated object is an lvalue reference (e.g., evaluating a
variable_ yields T instead of T&). This changes library semantics (reference propagation) and
can break code relying on reference results, as well as add unintended copies for non-trivial types.

Code

include/deferred/evaluate.hpp[R21-24]

+constexpr auto evaluate(T&& t)
{
if constexpr (Deferred<T>)
{

Evidence

evaluate() returns auto and returns std::forward(t) for non-deferred values; with auto
return type, an expression of type U& deduces to U (copy), so references are not preserved. This
directly conflicts with variable_ exposing its stored value by reference via `operator()()
&/const&, and any deferred constructs (e.g., switch case bodies) that rely on evaluate()` to
preserve reference category.

include/deferred/evaluate.hpp[20-55]
include/deferred/variable.hpp[69-87]
include/deferred/switch.hpp[83-104]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`deferred::evaluate()` / `deferred::recursive_evaluate()` now return `auto`, which decays references and forces copies. This breaks reference-propagation semantics (e.g., evaluating a `variable_` lvalue should yield `T&`, not `T`).
### Issue Context
The current implementation returns `std::forward<T>(t)` for non-deferred values, but with `auto` return type that becomes a value return. This also affects any call-sites that expect `evaluate()` to preserve `decltype(auto)` behavior.
### Fix Focus Areas
- include/deferred/evaluate.hpp[20-55]
### Suggested fix
- Change both function signatures back to `constexpr decltype(auto) evaluate(T&&)` and `constexpr decltype(auto) recursive_evaluate(T&&)`.
- Keep the simplification (direct `return evaluate(...)`) if desired, but preserve `decltype(auto)` so references/`const` are not lost.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. while rvalue consumes state ☑ 🐞 ≡

Description

while_expression::operator()() && evaluates both condition and body using `std::get(...,
std::move(m_expressions))` inside the loop, which makes subexpressions rvalues on every iteration.
This can repeatedly invoke rvalue-qualified operator()&& on stateful subexpressions (e.g.,
variable_), moving-from their internal state and corrupting loop behavior across iterations.

Code

include/deferred/while.hpp[R65-71]

+  constexpr void operator()() &&
+  {
+    while (evaluate(std::get<0>(std::move(m_expressions))))
+    {
+      evaluate(std::get<1>(std::move(m_expressions)));
+    }
+  }

Evidence

In the rvalue overload, std::get(std::move(m_expressions)) and
std::get(std::move(m_expressions)) produce rvalue references to the stored subexpressions on every
iteration. For variable_, the new rvalue-qualified accessor operator()() && returns
std::move(m_t), so evaluating a moved variable_ can consume its stored value (e.g., emptying a
std::string) and change subsequent iterations unexpectedly.

include/deferred/while.hpp[64-71]
include/deferred/variable.hpp[83-87]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`while_expression::operator()() &&` evaluates subexpressions as rvalues on every loop iteration, which can invoke rvalue-qualified call operators and move-from internal state repeatedly.
### Issue Context
A while-loop inherently needs to re-evaluate its condition/body multiple times. Treating the stored expressions as rvalues each time is unsafe for stateful expressions (e.g., `variable_::operator()&&` moves out the stored value).
### Fix Focus Areas
- include/deferred/while.hpp[64-71]
### Suggested fix
- In `operator()() &&`, do **not** use `std::move(m_expressions)` inside the loop.
- Option A (simplest): implement `operator()() &&` identically to `operator()() &` (evaluate via `m_expressions` as lvalues).
- Option B: if you want to allow consuming *once*, move subexpressions into local bindings **once** before the loop (only if the tuple/elements are movable), then evaluate locals as lvalues inside the loop.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

4. Format target command too long 🐞 ☼

Description

The CMake format target expands a recursive glob of all source files directly onto a single
clang-format -i ... command line. On some platforms (notably Windows), this can exceed
command-line length limits and make the target fail or behave inconsistently.

Code

CMakeLists.txt[R101-111]

+find_program(CLANG_FORMAT clang-format)
+if(CLANG_FORMAT)
+  file(GLOB_RECURSE ALL_SOURCE_FILES
+    "include/*.h*"
+    "test/*.c*"
+    "examples/*.c*")
+  add_custom_target(format
+    COMMAND ${CLANG_FORMAT} -i ${ALL_SOURCE_FILES}
+    WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
+    COMMENT "Formatting code with clang-format"
+    VERBATIM)

Evidence

The custom target runs clang-format -i ${ALL_SOURCE_FILES} where ALL_SOURCE_FILES is produced by
GLOB_RECURSE. Large expansions can exceed shell/OS command length limits; CMake will pass this as
one command invocation.

CMakeLists.txt[99-112]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The `format` target may fail on platforms with strict command-line length limits because it passes the entire `${ALL_SOURCE_FILES}` list to a single `clang-format` invocation.
### Issue Context
This is a common portability issue for CMake custom targets that expand large file lists.
### Fix Focus Areas
- CMakeLists.txt[99-112]
### Suggested fix
- Implement formatting via a CMake script (`cmake -P`) that iterates files and runs clang-format per file (or in manageable batches).
- Alternatively, generate a file list and use tooling that supports reading file names from stdin/response file semantics (while staying cross-platform).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

5. format_changed splits filenames 🐞 ☼

Description

scripts/format_changed.sh iterates over a whitespace-separated $FILES list, which will break if
any changed path contains spaces (word-splitting). This can cause missed formatting or attempts to
format non-existent partial paths.

Code

scripts/format_changed.sh[R3-16]

+# Find changed files (Added, Copied, Modified, Renamed) in the working tree and staged area
+FILES=$(git diff --name-only --diff-filter=ACMR HEAD | grep -E "\.(cpp|hpp)$")
+
+if [ -z "$FILES" ]; then
+  echo "No changed source files to format."
+  exit 0
+fi
+
+echo "Formatting changed files:"
+for file in $FILES; do
+  if [ -f "$file" ]; then
+    echo "  $file"
+    clang-format -i "$file"
+  fi

Evidence

The script collects filenames into a single variable and then loops with for file in $FILES; do,
which relies on shell word-splitting. This is not safe for paths with whitespace; `git diff
--name-only` can output such paths.

scripts/format_changed.sh[3-16]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The formatter script breaks on filenames containing spaces due to word-splitting when iterating `$FILES`.
### Issue Context
`git diff --name-only` emits one path per line. Converting that into a space-delimited string and iterating with `for` is unsafe.
### Fix Focus Areas
- scripts/format_changed.sh[3-16]
### Suggested fix
- Use NUL-delimited output and a safe read loop, e.g.:
- `git diff -z --name-only --diff-filter=ACMR HEAD -- '*.cpp' '*.hpp' | while IFS= read -r -d '' file; do ...; done`
- Or keep newline-delimited output and iterate with `while IFS= read -r file; do ...; done`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more

[gemini-code-assist]

Code Review

This pull request introduces project-wide code formatting using clang-format, updates license headers to the SPDX format, and adopts C++20 features such as concepts and the [[no_unique_address]] attribute. It also adds [[nodiscard]] to various functions and refactors type traits. Several critical issues were identified in the review: a bug in the while_expression rvalue overload where members are moved inside a loop, a regression in return type deduction for evaluation functions, and an incomplete visit implementation for switch expressions.