chore: Move codegen venv setup into build stage (#6617)

Co-authored-by: JCW <a1q123456@users.noreply.github.com>
Co-authored-by: Bart <bthomee@users.noreply.github.com>

.clang-tidy

@@ -10,26 +10,26 @@ Checks: "-*,
   bugprone-chained-comparison,
   bugprone-compare-pointer-to-member-virtual-function,
   bugprone-copy-constructor-init,
-  # bugprone-crtp-constructor-accessibility, # has issues
+  bugprone-crtp-constructor-accessibility,
   bugprone-dangling-handle,
   bugprone-dynamic-static-initializers,
-  # bugprone-empty-catch, # has issues
+  bugprone-empty-catch,
   bugprone-fold-init-type,
-  # bugprone-forward-declaration-namespace, # has issues
-  # bugprone-inaccurate-erase,
-  # bugprone-inc-dec-in-conditions,
-  # bugprone-incorrect-enable-if,
-  # bugprone-incorrect-roundings,
-  # bugprone-infinite-loop,
-  # bugprone-integer-division,
+  bugprone-forward-declaration-namespace,
+  bugprone-inaccurate-erase,
+  bugprone-inc-dec-in-conditions,
+  bugprone-incorrect-enable-if,
+  bugprone-incorrect-roundings,
+  bugprone-infinite-loop,
+  bugprone-integer-division,
   bugprone-lambda-function-name,
-  # bugprone-macro-parentheses, # has issues
+  bugprone-macro-parentheses,
   bugprone-macro-repeated-side-effects,
   bugprone-misplaced-operator-in-strlen-in-alloc,
   bugprone-misplaced-pointer-arithmetic-in-alloc,
   bugprone-misplaced-widening-cast,
   bugprone-move-forwarding-reference,
-  # bugprone-multi-level-implicit-pointer-conversion, # has issues
+  bugprone-multi-level-implicit-pointer-conversion,
   bugprone-multiple-new-in-one-expression,
   bugprone-multiple-statement-macro,
   bugprone-no-escape,
@@ -39,13 +39,13 @@ Checks: "-*,
   bugprone-pointer-arithmetic-on-polymorphic-object,
   bugprone-posix-return,
   bugprone-redundant-branch-condition,
-  # bugprone-reserved-identifier, # has issues
-  # bugprone-return-const-ref-from-parameter, # has issues
+  bugprone-reserved-identifier,
+  bugprone-return-const-ref-from-parameter,
   bugprone-shared-ptr-array-mismatch,
   bugprone-signal-handler,
   bugprone-signed-char-misuse,
   bugprone-sizeof-container,
-  # bugprone-sizeof-expression, # has issues
+  bugprone-sizeof-expression,
   bugprone-spuriously-wake-up-functions,
   bugprone-standalone-empty,
   bugprone-string-constructor,
@@ -62,7 +62,7 @@ Checks: "-*,
   bugprone-suspicious-string-compare,
   bugprone-suspicious-stringview-data-usage,
   bugprone-swapped-arguments,
-  # bugprone-switch-missing-default-case, # has issues
+  bugprone-switch-missing-default-case,
   bugprone-terminating-continue,
   bugprone-throw-keyword-missing,
   bugprone-too-small-loop-variable,
@@ -73,7 +73,7 @@ Checks: "-*,
   bugprone-unhandled-self-assignment,
   bugprone-unique-ptr-array-mismatch,
   bugprone-unsafe-functions,
-  # bugprone-use-after-move, # has issues
+  bugprone-use-after-move, # has issues
   bugprone-unused-raii,
   bugprone-unused-return-value,
   bugprone-unused-local-non-trivial-variable,

.github/workflows/publish-docs.yml

@@ -82,7 +82,7 @@ jobs:
 
       - name: Create documentation artifact
         if: ${{ github.event.repository.visibility == 'public' && github.event_name == 'push' }}
-        uses: actions/upload-pages-artifact@fc324d3547104276b827a68afc52ff2a11cc49c9 # v5.0.0
+        uses: actions/upload-pages-artifact@7b1f4a764d45c48632c6b24a0339c27f5614fb0b # v4.0.0
         with:
           path: ${{ env.BUILD_DIR }}/docs/html
 

.github/workflows/reusable-build-test-config.yml

@@ -153,6 +153,32 @@ jobs:
             ${CMAKE_ARGS} \
             ..
 
+      - name: Check protocol autogen files are up-to-date
+        working-directory: ${{ env.BUILD_DIR }}
+        env:
+          MESSAGE: |
+
+            The generated protocol wrapper classes are out of date.
+
+            This typically happens when the macro files or generator scripts
+            have changed but the generated files were not regenerated.
+
+            To fix this:
+              1. Run: cmake --build . --target setup_code_gen
+              2. Run: cmake --build . --target code_gen
+              3. Commit and push the regenerated files
+        run: |
+          set -e
+          cmake --build . --target setup_code_gen
+          cmake --build . --target code_gen
+          DIFF=$(git -C .. status --porcelain -- include/xrpl/protocol_autogen src/tests/libxrpl/protocol_autogen)
+          if [ -n "${DIFF}" ]; then
+            echo "::error::Generated protocol files are out of date"
+            git -C .. diff -- include/xrpl/protocol_autogen src/tests/libxrpl/protocol_autogen
+            echo "${MESSAGE}"
+            exit 1
+          fi
+
       - name: Build the binary
         working-directory: ${{ env.BUILD_DIR }}
         env:
@@ -166,29 +192,6 @@ jobs:
             --parallel "${BUILD_NPROC}" \
             --target "${CMAKE_TARGET}"
 
-      - name: Check protocol autogen files are up-to-date
-        env:
-          MESSAGE: |
-
-            The generated protocol wrapper classes are out of date.
-
-            This typically happens when your branch is behind develop and
-            the macro files or generator scripts have changed.
-
-            To fix this:
-              1. Update your branch from develop (merge or rebase)
-              2. Build with code generation enabled (XRPL_NO_CODEGEN=OFF)
-              3. Commit and push the regenerated files
-        run: |
-          set -e
-          DIFF=$(git status --porcelain -- include/xrpl/protocol_autogen src/tests/libxrpl/protocol_autogen)
-          if [ -n "${DIFF}" ]; then
-            echo "::error::Generated protocol files are out of date"
-            git diff -- include/xrpl/protocol_autogen src/tests/libxrpl/protocol_autogen
-            echo "${MESSAGE}"
-            exit 1
-          fi
-
       - name: Show ccache statistics
         if: ${{ inputs.ccache_enabled }}
         run: |

.gitignore

@@ -13,6 +13,7 @@
 Debug/
 Release/
 /.build/
+/.venv/
 /build/
 /db/
 /out.txt

BUILD.md

@@ -459,6 +459,21 @@ install ccache --version 4.11.3 --allow-downgrade`.
    The location of `xrpld` binary in your build directory depends on your
    CMake generator. Pass `--help` to see the rest of the command line options.
 
+## Code generation
+
+The protocol wrapper classes in `include/xrpl/protocol_autogen/` are generated
+from macro definition files in `include/xrpl/protocol/detail/`. If you modify
+the macro files (e.g. `transactions.macro`, `ledger_entries.macro`) or the
+generation scripts/templates in `cmake/scripts/codegen/`, you need to regenerate the
+files:
+
+```
+cmake --build . --target setup_code_gen  # create venv and install dependencies (once)
+cmake --build . --target code_gen        # regenerate code
+```
+
+The regenerated files should be committed alongside your changes.
+
 ## Coverage report
 
 The coverage report is intended for developers using compilers GCC

CMakeLists.txt

@@ -132,6 +132,7 @@ if(coverage)
 endif()
 
 include(XrplCore)
+include(XrplProtocolAutogen)
 include(XrplInstall)
 include(XrplValidatorKeys)
 

cmake/XrplCore.cmake

@@ -108,24 +108,12 @@ target_link_libraries(
 )
 
 # Level 05
-## Set up code generation for protocol_autogen module
-include(XrplProtocolAutogen)
-# Must call setup_protocol_autogen before add_module so that:
-# 1. Stale generated files are cleared before GLOB runs
-# 2. Output file list is known for custom commands
-setup_protocol_autogen()
-
 add_module(xrpl protocol_autogen)
 target_link_libraries(
     xrpl.libxrpl.protocol_autogen
     PUBLIC xrpl.libxrpl.protocol
 )
 
-# Ensure code generation runs before compiling protocol_autogen
-if(TARGET protocol_autogen_generate)
-    add_dependencies(xrpl.libxrpl.protocol_autogen protocol_autogen_generate)
-endif()
-
 # Level 06
 add_module(xrpl core)
 target_link_libraries(

cmake/XrplProtocolAutogen.cmake

@@ -2,308 +2,145 @@
    Protocol Autogen - Code generation for protocol wrapper classes
 #]===================================================================]
 
-# Options for code generation
-option(
-    XRPL_NO_CODEGEN
-    "Disable code generation (use pre-generated files from repository)"
-    OFF
-)
 set(CODEGEN_VENV_DIR
-    ""
+    "${CMAKE_CURRENT_SOURCE_DIR}/.venv"
     CACHE PATH
-    "Path to Python virtual environment for code generation. If provided, automatic venv setup is skipped."
+    "Path to a Python virtual environment for code generation. A venv will be created here by setup_code_gen and used to run generation scripts."
 )
 
-# Function to set up code generation for protocol_autogen module
-# This runs at configure time to generate C++ wrapper classes from macro files
-function(setup_protocol_autogen)
-    # Directory paths
-    set(MACRO_DIR "${CMAKE_CURRENT_SOURCE_DIR}/include/xrpl/protocol/detail")
-    set(AUTOGEN_HEADER_DIR
-        "${CMAKE_CURRENT_SOURCE_DIR}/include/xrpl/protocol_autogen"
-    )
-    set(AUTOGEN_TEST_DIR
-        "${CMAKE_CURRENT_SOURCE_DIR}/src/tests/libxrpl/protocol_autogen"
-    )
-    set(SCRIPTS_DIR "${CMAKE_CURRENT_SOURCE_DIR}/scripts")
+# Directory paths
+set(MACRO_DIR "${CMAKE_CURRENT_SOURCE_DIR}/include/xrpl/protocol/detail")
+set(AUTOGEN_HEADER_DIR
+    "${CMAKE_CURRENT_SOURCE_DIR}/include/xrpl/protocol_autogen"
+)
+set(AUTOGEN_TEST_DIR
+    "${CMAKE_CURRENT_SOURCE_DIR}/src/tests/libxrpl/protocol_autogen"
+)
+set(SCRIPTS_DIR "${CMAKE_CURRENT_SOURCE_DIR}/cmake/scripts/codegen")
 
-    # Input macro files
-    set(TRANSACTIONS_MACRO "${MACRO_DIR}/transactions.macro")
-    set(LEDGER_ENTRIES_MACRO "${MACRO_DIR}/ledger_entries.macro")
-    set(SFIELDS_MACRO "${MACRO_DIR}/sfields.macro")
+# Input macro files
+set(TRANSACTIONS_MACRO "${MACRO_DIR}/transactions.macro")
+set(LEDGER_ENTRIES_MACRO "${MACRO_DIR}/ledger_entries.macro")
+set(SFIELDS_MACRO "${MACRO_DIR}/sfields.macro")
 
-    # Python scripts and templates
-    set(GENERATE_TX_SCRIPT "${SCRIPTS_DIR}/generate_tx_classes.py")
-    set(GENERATE_LEDGER_SCRIPT "${SCRIPTS_DIR}/generate_ledger_classes.py")
-    set(REQUIREMENTS_FILE "${SCRIPTS_DIR}/requirements.txt")
-    set(MACRO_PARSER_COMMON "${SCRIPTS_DIR}/macro_parser_common.py")
-    set(TX_TEMPLATE "${SCRIPTS_DIR}/templates/Transaction.h.mako")
-    set(TX_TEST_TEMPLATE "${SCRIPTS_DIR}/templates/TransactionTests.cpp.mako")
-    set(LEDGER_TEMPLATE "${SCRIPTS_DIR}/templates/LedgerEntry.h.mako")
-    set(LEDGER_TEST_TEMPLATE
-        "${SCRIPTS_DIR}/templates/LedgerEntryTests.cpp.mako"
+# Python scripts and templates
+set(GENERATE_TX_SCRIPT "${SCRIPTS_DIR}/generate_tx_classes.py")
+set(GENERATE_LEDGER_SCRIPT "${SCRIPTS_DIR}/generate_ledger_classes.py")
+set(REQUIREMENTS_FILE "${SCRIPTS_DIR}/requirements.txt")
+set(MACRO_PARSER_COMMON "${SCRIPTS_DIR}/macro_parser_common.py")
+set(TX_TEMPLATE "${SCRIPTS_DIR}/templates/Transaction.h.mako")
+set(TX_TEST_TEMPLATE "${SCRIPTS_DIR}/templates/TransactionTests.cpp.mako")
+set(LEDGER_TEMPLATE "${SCRIPTS_DIR}/templates/LedgerEntry.h.mako")
+set(LEDGER_TEST_TEMPLATE "${SCRIPTS_DIR}/templates/LedgerEntryTests.cpp.mako")
+set(ALL_INPUT_FILES
+    "${TRANSACTIONS_MACRO}"
+    "${LEDGER_ENTRIES_MACRO}"
+    "${SFIELDS_MACRO}"
+    "${GENERATE_TX_SCRIPT}"
+    "${GENERATE_LEDGER_SCRIPT}"
+    "${REQUIREMENTS_FILE}"
+    "${MACRO_PARSER_COMMON}"
+    "${TX_TEMPLATE}"
+    "${TX_TEST_TEMPLATE}"
+    "${LEDGER_TEMPLATE}"
+    "${LEDGER_TEST_TEMPLATE}"
+)
+
+# Create output directories
+file(MAKE_DIRECTORY "${AUTOGEN_HEADER_DIR}/transactions")
+file(MAKE_DIRECTORY "${AUTOGEN_HEADER_DIR}/ledger_entries")
+file(MAKE_DIRECTORY "${AUTOGEN_TEST_DIR}/ledger_entries")
+file(MAKE_DIRECTORY "${AUTOGEN_TEST_DIR}/transactions")
+
+# Find Python3
+if(NOT Python3_EXECUTABLE)
+    find_package(Python3 COMPONENTS Interpreter QUIET)
+endif()
+
+if(NOT Python3_EXECUTABLE)
+    find_program(Python3_EXECUTABLE NAMES python3 python)
+endif()
+
+if(NOT Python3_EXECUTABLE)
+    message(
+        WARNING
+        "Python3 not found. The 'code_gen' and 'setup_code_gen' targets will not be available."
     )
+    return()
+endif()
 
-    # Check if code generation is disabled
-    if(XRPL_NO_CODEGEN)
+# Warn if pip is configured with a non-default index (may need VPN).
+execute_process(
+    COMMAND ${Python3_EXECUTABLE} -m pip config get global.index-url
+    OUTPUT_VARIABLE PIP_INDEX_URL
+    OUTPUT_STRIP_TRAILING_WHITESPACE
+    ERROR_QUIET
+    RESULT_VARIABLE PIP_CONFIG_RESULT
+)
+if(PIP_CONFIG_RESULT EQUAL 0 AND PIP_INDEX_URL)
+    if(
+        NOT PIP_INDEX_URL STREQUAL "https://pypi.org/simple"
+        AND NOT PIP_INDEX_URL STREQUAL "https://pypi.python.org/simple"
+    )
         message(
             WARNING
-            "Protocol autogen: Code generation is disabled (XRPL_NO_CODEGEN=ON). "
-            "Generated files may be out of date."
+            "Private pip index URL detected: ${PIP_INDEX_URL}\n"
+            "You may need to connect to VPN to access this URL."
         )
-        return()
     endif()
+endif()
 
-    # Create output directories
-    file(MAKE_DIRECTORY "${AUTOGEN_HEADER_DIR}/transactions")
-    file(MAKE_DIRECTORY "${AUTOGEN_HEADER_DIR}/ledger_entries")
-    file(MAKE_DIRECTORY "${AUTOGEN_TEST_DIR}/ledger_entries")
-    file(MAKE_DIRECTORY "${AUTOGEN_TEST_DIR}/transactions")
-
-    # Find Python3 - check if already found by Conan or find it ourselves
-    if(NOT Python3_EXECUTABLE)
-        find_package(Python3 COMPONENTS Interpreter QUIET)
-    endif()
-
-    if(NOT Python3_EXECUTABLE)
-        # Try finding python3 executable directly
-        find_program(Python3_EXECUTABLE NAMES python3 python)
-    endif()
-
-    if(NOT Python3_EXECUTABLE)
-        message(
-            FATAL_ERROR
-            "Python3 not found. Code generation cannot proceed.\n"
-            "Please install Python 3, or set -DXRPL_NO_CODEGEN=ON to use existing generated files."
-        )
-        return()
-    endif()
-
-    message(STATUS "Using Python3 for code generation: ${Python3_EXECUTABLE}")
-
-    # Set up Python virtual environment for code generation
-    if(CODEGEN_VENV_DIR)
-        # User-provided venv - skip automatic setup
-        set(VENV_DIR "${CODEGEN_VENV_DIR}")
-        message(STATUS "Using user-provided Python venv: ${VENV_DIR}")
-    else()
-        # Use default venv in build directory
-        set(VENV_DIR "${CMAKE_CURRENT_BINARY_DIR}/codegen_venv")
-    endif()
-
-    # Determine the Python executable path in the venv
+# Determine which Python interpreter to use for code generation.
+if(CODEGEN_VENV_DIR)
     if(WIN32)
-        set(VENV_PYTHON "${VENV_DIR}/Scripts/python.exe")
-        set(VENV_PIP "${VENV_DIR}/Scripts/pip.exe")
+        set(CODEGEN_PYTHON "${CODEGEN_VENV_DIR}/Scripts/python.exe")
     else()
-        set(VENV_PYTHON "${VENV_DIR}/bin/python")
-        set(VENV_PIP "${VENV_DIR}/bin/pip")
+        set(CODEGEN_PYTHON "${CODEGEN_VENV_DIR}/bin/python")
     endif()
-
-    # Only auto-setup venv if not user-provided
-    if(NOT CODEGEN_VENV_DIR)
-        # Check if venv needs to be created or updated
-        set(VENV_NEEDS_UPDATE FALSE)
-        if(NOT EXISTS "${VENV_PYTHON}")
-            set(VENV_NEEDS_UPDATE TRUE)
-            message(
-                STATUS
-                "Creating Python virtual environment for code generation..."
-            )
-        elseif(
-            "${REQUIREMENTS_FILE}"
-                IS_NEWER_THAN
-                "${VENV_DIR}/.requirements_installed"
-        )
-            set(VENV_NEEDS_UPDATE TRUE)
-            message(
-                STATUS
-                "Updating Python virtual environment (requirements changed)..."
-            )
-        endif()
-
-        # Create/update virtual environment if needed
-        if(VENV_NEEDS_UPDATE)
-            message(
-                STATUS
-                "Setting up Python virtual environment at ${VENV_DIR}"
-            )
-            execute_process(
-                COMMAND ${Python3_EXECUTABLE} -m venv "${VENV_DIR}"
-                RESULT_VARIABLE VENV_RESULT
-                ERROR_VARIABLE VENV_ERROR
-            )
-            if(NOT VENV_RESULT EQUAL 0)
-                message(
-                    FATAL_ERROR
-                    "Failed to create virtual environment: ${VENV_ERROR}"
-                )
-            endif()
-
-            # Check pip index URL configuration
-            execute_process(
-                COMMAND ${VENV_PIP} config get global.index-url
-                OUTPUT_VARIABLE PIP_INDEX_URL
-                OUTPUT_STRIP_TRAILING_WHITESPACE
-                ERROR_QUIET
-            )
-
-            # Default PyPI URL
-            set(DEFAULT_PIP_INDEX "https://pypi.org/simple")
-
-            # Show warning if using non-default index
-            if(PIP_INDEX_URL AND NOT PIP_INDEX_URL STREQUAL "")
-                if(NOT PIP_INDEX_URL STREQUAL DEFAULT_PIP_INDEX)
-                    message(
-                        WARNING
-                        "Private pip index URL detected: ${PIP_INDEX_URL}\n"
-                        "You may need to connect to VPN to access this URL."
-                    )
-                endif()
-            endif()
-
-            message(STATUS "Installing Python dependencies...")
-            execute_process(
-                COMMAND ${VENV_PIP} install --upgrade pip
-                RESULT_VARIABLE PIP_UPGRADE_RESULT
-                OUTPUT_QUIET
-                ERROR_VARIABLE PIP_UPGRADE_ERROR
-            )
-            if(NOT PIP_UPGRADE_RESULT EQUAL 0)
-                message(WARNING "Failed to upgrade pip: ${PIP_UPGRADE_ERROR}")
-            endif()
-
-            execute_process(
-                COMMAND ${VENV_PIP} install -r "${REQUIREMENTS_FILE}"
-                RESULT_VARIABLE PIP_INSTALL_RESULT
-                ERROR_VARIABLE PIP_INSTALL_ERROR
-            )
-            if(NOT PIP_INSTALL_RESULT EQUAL 0)
-                message(
-                    FATAL_ERROR
-                    "Failed to install Python dependencies: ${PIP_INSTALL_ERROR}"
-                )
-            endif()
-
-            # Mark requirements as installed
-            file(TOUCH "${VENV_DIR}/.requirements_installed")
-            message(STATUS "Python virtual environment ready")
-        endif()
-    endif()
-
-    # At configure time - get list of output files for transactions
-    execute_process(
-        COMMAND
-            ${VENV_PYTHON} "${GENERATE_TX_SCRIPT}" "${TRANSACTIONS_MACRO}"
-            --header-dir "${AUTOGEN_HEADER_DIR}/transactions" --test-dir
-            "${AUTOGEN_TEST_DIR}/transactions" --list-outputs
-        OUTPUT_VARIABLE TX_OUTPUT_FILES
-        OUTPUT_STRIP_TRAILING_WHITESPACE
-        RESULT_VARIABLE TX_LIST_RESULT
-        ERROR_VARIABLE TX_LIST_ERROR
+else()
+    set(CODEGEN_PYTHON "${Python3_EXECUTABLE}")
+    message(
+        WARNING
+        "CODEGEN_VENV_DIR is not set. Dependencies will be installed globally.\n"
+        "If this is not intended, reconfigure with:\n"
+        "  cmake . -UCODEGEN_VENV_DIR"
     )
-    if(NOT TX_LIST_RESULT EQUAL 0)
-        message(
-            FATAL_ERROR
-            "Failed to list transaction output files:\n${TX_LIST_ERROR}"
-        )
-    endif()
-    # Convert newline-separated list to CMake list
-    string(REPLACE "\\" "/" TX_OUTPUT_FILES "${TX_OUTPUT_FILES}")
-    string(REPLACE "\n" ";" TX_OUTPUT_FILES "${TX_OUTPUT_FILES}")
+endif()
 
-    # At configure time - get list of output files for ledger entries
-    execute_process(
-        COMMAND
-            ${VENV_PYTHON} "${GENERATE_LEDGER_SCRIPT}" "${LEDGER_ENTRIES_MACRO}"
-            --header-dir "${AUTOGEN_HEADER_DIR}/ledger_entries" --test-dir
-            "${AUTOGEN_TEST_DIR}/ledger_entries" --list-outputs
-        OUTPUT_VARIABLE LEDGER_OUTPUT_FILES
-        OUTPUT_STRIP_TRAILING_WHITESPACE
-        RESULT_VARIABLE LEDGER_LIST_RESULT
-        ERROR_VARIABLE LEDGER_LIST_ERROR
-    )
-    if(NOT LEDGER_LIST_RESULT EQUAL 0)
-        message(
-            FATAL_ERROR
-            "Failed to list ledger entry output files:\n${LEDGER_LIST_ERROR}"
-        )
-    endif()
-    # Convert newline-separated list to CMake list
-    string(REPLACE "\\" "/" LEDGER_OUTPUT_FILES "${LEDGER_OUTPUT_FILES}")
-    string(REPLACE "\n" ";" LEDGER_OUTPUT_FILES "${LEDGER_OUTPUT_FILES}")
-
-    # Custom command to generate transaction classes at build time
-    add_custom_command(
-        OUTPUT ${TX_OUTPUT_FILES}
-        COMMAND
-            ${VENV_PYTHON} "${GENERATE_TX_SCRIPT}" "${TRANSACTIONS_MACRO}"
-            --header-dir "${AUTOGEN_HEADER_DIR}/transactions" --test-dir
-            "${AUTOGEN_TEST_DIR}/transactions" --sfields-macro
-            "${SFIELDS_MACRO}"
-        WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}"
-        DEPENDS
-            "${TRANSACTIONS_MACRO}"
-            "${SFIELDS_MACRO}"
-            "${GENERATE_TX_SCRIPT}"
-            "${MACRO_PARSER_COMMON}"
-            "${TX_TEMPLATE}"
-            "${TX_TEST_TEMPLATE}"
-            "${REQUIREMENTS_FILE}"
-        COMMENT "Generating transaction classes from transactions.macro..."
-        VERBATIM
-    )
-
-    # Custom command to generate ledger entry classes at build time
-    add_custom_command(
-        OUTPUT ${LEDGER_OUTPUT_FILES}
-        COMMAND
-            ${VENV_PYTHON} "${GENERATE_LEDGER_SCRIPT}" "${LEDGER_ENTRIES_MACRO}"
-            --header-dir "${AUTOGEN_HEADER_DIR}/ledger_entries" --test-dir
-            "${AUTOGEN_TEST_DIR}/ledger_entries" --sfields-macro
-            "${SFIELDS_MACRO}"
-        WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}"
-        DEPENDS
-            "${LEDGER_ENTRIES_MACRO}"
-            "${SFIELDS_MACRO}"
-            "${GENERATE_LEDGER_SCRIPT}"
-            "${MACRO_PARSER_COMMON}"
-            "${LEDGER_TEMPLATE}"
-            "${LEDGER_TEST_TEMPLATE}"
-            "${REQUIREMENTS_FILE}"
-        COMMENT "Generating ledger entry classes from ledger_entries.macro..."
-        VERBATIM
-    )
-
-    # Create a custom target that depends on all generated files
+# Custom target to create a venv and install Python dependencies.
+# Run manually with: cmake --build . --target setup_code_gen
+if(CODEGEN_VENV_DIR)
     add_custom_target(
-        protocol_autogen_generate
-        DEPENDS ${TX_OUTPUT_FILES} ${LEDGER_OUTPUT_FILES}
-        COMMENT "Protocol autogen code generation"
+        setup_code_gen
+        COMMAND ${Python3_EXECUTABLE} -m venv "${CODEGEN_VENV_DIR}"
+        COMMAND ${CODEGEN_PYTHON} -m pip install -r "${REQUIREMENTS_FILE}"
+        WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}"
+        COMMENT "Creating venv and installing code generation dependencies..."
     )
+else()
+    add_custom_target(
+        setup_code_gen
+        COMMAND ${Python3_EXECUTABLE} -m pip install -r "${REQUIREMENTS_FILE}"
+        WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}"
+        COMMENT "Installing code generation dependencies..."
+    )
+endif()
 
-    # Extract test files from output lists (files ending in Tests.cpp)
-    set(PROTOCOL_AUTOGEN_TEST_SOURCES "")
-    foreach(FILE ${TX_OUTPUT_FILES} ${LEDGER_OUTPUT_FILES})
-        if(FILE MATCHES "Tests\\.cpp$")
-            list(APPEND PROTOCOL_AUTOGEN_TEST_SOURCES "${FILE}")
-        endif()
-    endforeach()
-    # Export test sources to parent scope for use in test CMakeLists.txt
-    set(PROTOCOL_AUTOGEN_TEST_SOURCES
-        "${PROTOCOL_AUTOGEN_TEST_SOURCES}"
-        CACHE INTERNAL
-        "Generated protocol_autogen test sources"
-    )
-
-    # Register dependencies so CMake reconfigures when macro files change
-    # (to update the list of output files)
-    set_property(
-        DIRECTORY
-        APPEND
-        PROPERTY
-            CMAKE_CONFIGURE_DEPENDS
-                "${TRANSACTIONS_MACRO}"
-                "${LEDGER_ENTRIES_MACRO}"
-    )
-endfunction()
+# Custom target for code generation, excluded from ALL.
+# Run manually with: cmake --build . --target code_gen
+add_custom_target(
+    code_gen
+    COMMAND
+        ${CMAKE_COMMAND} -DCODEGEN_PYTHON=${CODEGEN_PYTHON}
+        -DGENERATE_TX_SCRIPT=${GENERATE_TX_SCRIPT}
+        -DGENERATE_LEDGER_SCRIPT=${GENERATE_LEDGER_SCRIPT}
+        -DTRANSACTIONS_MACRO=${TRANSACTIONS_MACRO}
+        -DLEDGER_ENTRIES_MACRO=${LEDGER_ENTRIES_MACRO}
+        -DSFIELDS_MACRO=${SFIELDS_MACRO}
+        -DAUTOGEN_HEADER_DIR=${AUTOGEN_HEADER_DIR}
+        -DAUTOGEN_TEST_DIR=${AUTOGEN_TEST_DIR} -P
+        "${CMAKE_CURRENT_SOURCE_DIR}/cmake/XrplProtocolAutogenRun.cmake"
+    WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}"
+    COMMENT "Running protocol code generation..."
+    SOURCES ${ALL_INPUT_FILES}
+)

cmake/XrplProtocolAutogenRun.cmake

@@ -0,0 +1,39 @@
+#[===================================================================[
+   Protocol Autogen - Run script invoked by the 'code_gen' target
+#]===================================================================]
+
+# Generate transaction classes.
+execute_process(
+    COMMAND
+        ${CODEGEN_PYTHON} "${GENERATE_TX_SCRIPT}" "${TRANSACTIONS_MACRO}"
+        --header-dir "${AUTOGEN_HEADER_DIR}/transactions" --test-dir
+        "${AUTOGEN_TEST_DIR}/transactions" --sfields-macro "${SFIELDS_MACRO}"
+    RESULT_VARIABLE TX_RESULT
+    OUTPUT_VARIABLE TX_OUTPUT
+    ERROR_VARIABLE TX_ERROR
+)
+if(NOT TX_RESULT EQUAL 0)
+    message(
+        FATAL_ERROR
+        "Transaction code generation failed:\n${TX_OUTPUT}\n${TX_ERROR}\n${TX_RESULT}"
+    )
+endif()
+
+# Generate ledger entry classes.
+execute_process(
+    COMMAND
+        ${CODEGEN_PYTHON} "${GENERATE_LEDGER_SCRIPT}" "${LEDGER_ENTRIES_MACRO}"
+        --header-dir "${AUTOGEN_HEADER_DIR}/ledger_entries" --test-dir
+        "${AUTOGEN_TEST_DIR}/ledger_entries" --sfields-macro "${SFIELDS_MACRO}"
+    RESULT_VARIABLE LEDGER_RESULT
+    OUTPUT_VARIABLE LEDGER_OUTPUT
+    ERROR_VARIABLE LEDGER_ERROR
+)
+if(NOT LEDGER_RESULT EQUAL 0)
+    message(
+        FATAL_ERROR
+        "Ledger entry code generation failed:\n${LEDGER_OUTPUT}\n${LEDGER_ERROR}\n${TX_RESULT}"
+    )
+endif()
+
+message(STATUS "Protocol autogen: code generation complete")

cmake/scripts/codegen/generate_ledger_classes.py

@@ -138,28 +138,12 @@ def main():
         "--sfields-macro",
         help="Path to sfields.macro (default: auto-detect from macro_path)",
     )
-    parser.add_argument(
-        "--list-outputs",
-        action="store_true",
-        help="List output files without generating (one per line)",
-    )
-
+    parser.add_argument("--venv-dir", help=argparse.SUPPRESS)
     args = parser.parse_args()
 
     # Parse the macro file to get ledger entry names
     entries = parse_macro_file(args.macro_path)
 
-    # If --list-outputs, just print the output file paths and exit
-    if args.list_outputs:
-        header_dir = Path(args.header_dir)
-        for entry in entries:
-            print(header_dir / f"{entry['name']}.h")
-        if args.test_dir:
-            test_dir = Path(args.test_dir)
-            for entry in entries:
-                print(test_dir / f"{entry['name']}Tests.cpp")
-        return
-
     # Auto-detect sfields.macro path if not provided
     if args.sfields_macro:
         sfields_path = Path(args.sfields_macro)

cmake/scripts/codegen/generate_tx_classes.py

@@ -147,28 +147,12 @@ def main():
         "--sfields-macro",
         help="Path to sfields.macro (default: auto-detect from macro_path)",
     )
-    parser.add_argument(
-        "--list-outputs",
-        action="store_true",
-        help="List output files without generating (one per line)",
-    )
-
+    parser.add_argument("--venv-dir", help=argparse.SUPPRESS)
     args = parser.parse_args()
 
     # Parse the macro file to get transaction names
     transactions = parse_macro_file(args.macro_path)
 
-    # If --list-outputs, just print the output file paths and exit
-    if args.list_outputs:
-        header_dir = Path(args.header_dir)
-        for tx in transactions:
-            print(header_dir / f"{tx['name']}.h")
-        if args.test_dir:
-            test_dir = Path(args.test_dir)
-            for tx in transactions:
-                print(test_dir / f"{tx['name']}Tests.cpp")
-        return
-
     # Auto-detect sfields.macro path if not provided
     if args.sfields_macro:
         sfields_path = Path(args.sfields_macro)

include/xrpl/basics/BasicConfig.h

@@ -296,7 +296,7 @@ set(T& target, std::string const& name, Section const& section)
         if ((found_and_valid = val.has_value()))
             target = *val;
     }
-    catch (boost::bad_lexical_cast&)
+    catch (boost::bad_lexical_cast const&)  // NOLINT(bugprone-empty-catch)
     {
     }
     return found_and_valid;
@@ -330,7 +330,7 @@ get(Section const& section, std::string const& name, T const& defaultValue = T{}
     {
         return section.value_or<T>(name, defaultValue);
     }
-    catch (boost::bad_lexical_cast&)
+    catch (boost::bad_lexical_cast const&)  // NOLINT(bugprone-empty-catch)
     {
     }
     return defaultValue;
@@ -345,7 +345,7 @@ get(Section const& section, std::string const& name, char const* defaultValue)
         if (val.has_value())
             return *val;
     }
-    catch (boost::bad_lexical_cast&)
+    catch (boost::bad_lexical_cast const&)  // NOLINT(bugprone-empty-catch)
     {
     }
     return defaultValue;

include/xrpl/basics/CountedObject.h

@@ -112,7 +112,6 @@ private:
         return c;
     }
 
-public:
     CountedObject() noexcept
     {
         getCounter().increment();
@@ -126,10 +125,13 @@ public:
     CountedObject&
     operator=(CountedObject const&) noexcept = default;
 
+public:
     ~CountedObject() noexcept
     {
         getCounter().decrement();
     }
+
+    friend Object;
 };
 
 }  // namespace xrpl

include/xrpl/basics/Log.h

@@ -226,7 +226,7 @@ private:
 // expensive argument lists if the stream is not active.
 #ifndef JLOG
 #define JLOG(x) \
-    if (!x)     \
+    if (!(x))   \
     {           \
     }           \
     else        \
@@ -235,7 +235,7 @@ private:
 
 #ifndef CLOG
 #define CLOG(ss) \
-    if (!ss)     \
+    if (!(ss))   \
         ;        \
     else         \
         *ss

include/xrpl/basics/SlabAllocator.h

@@ -60,7 +60,7 @@ class SlabAllocator
             {
                 // Use memcpy to avoid unaligned UB
                 // (will optimize to equivalent code)
-                std::memcpy(data, &l_, sizeof(std::uint8_t*));
+                std::memcpy(data, static_cast<void const*>(&l_), sizeof(std::uint8_t*));
                 l_ = data;
                 data += item;
             }
@@ -102,7 +102,7 @@ class SlabAllocator
                 {
                     // Use memcpy to avoid unaligned UB
                     // (will optimize to equivalent code)
-                    std::memcpy(&l_, ret, sizeof(std::uint8_t*));
+                    std::memcpy(static_cast<void*>(&l_), ret, sizeof(std::uint8_t*));
                 }
             }
 
@@ -127,7 +127,7 @@ class SlabAllocator
 
             // Use memcpy to avoid unaligned UB
             // (will optimize to equivalent code)
-            std::memcpy(ptr, &l_, sizeof(std::uint8_t*));
+            std::memcpy(ptr, static_cast<void const*>(&l_), sizeof(std::uint8_t*));
             l_ = ptr;
         }
     };

include/xrpl/beast/container/detail/aged_associative_container.h

@@ -26,7 +26,7 @@ struct aged_associative_container_extract_t<false>
     Value const&
     operator()(Value const& value) const
     {
-        return value;
+        return value;  // NOLINT(bugprone-return-const-ref-from-parameter)
     }
 };
 

include/xrpl/beast/container/detail/aged_ordered_container.h

@@ -257,7 +257,8 @@ private:
 
         config_t(config_t&& other)
             : KeyValueCompare(std::move(other.key_compare()))
-            , beast::detail::empty_base_optimization<ElementAllocator>(std::move(other))
+            , beast::detail::empty_base_optimization<ElementAllocator>(std::move(
+                  static_cast<beast::detail::empty_base_optimization<ElementAllocator>&>(other)))
             , clock(other.clock)
         {
         }

include/xrpl/beast/core/List.h

@@ -35,9 +35,11 @@ struct CopyConst<T const, U>
 template <typename T, typename Tag>
 class ListNode
 {
-private:
+    ListNode() = default;
+
     using value_type = T;
 
+    friend T;
     friend class List<T, Tag>;
 
     template <typename>

include/xrpl/beast/hash/hash_append.h

@@ -203,7 +203,8 @@ template <class Hasher, class T>
 inline std::enable_if_t<is_contiguously_hashable<T, Hasher>::value>
 hash_append(Hasher& h, T const& t) noexcept
 {
-    h(std::addressof(t), sizeof(t));
+    // NOLINTNEXTLINE(bugprone-sizeof-expression)
+    h(static_cast<void const*>(std::addressof(t)), sizeof(t));
 }
 
 template <class Hasher, class T>

include/xrpl/beast/rfc2616.h

@@ -53,8 +53,9 @@ is_white(char c)
         case '\t':
         case '\v':
             return true;
+        default:
+            return false;
     };
-    return false;
 }
 
 template <class FwdIter>

include/xrpl/beast/unit_test/reporter.h

@@ -118,18 +118,18 @@ private:
 
 //------------------------------------------------------------------------------
 
-template <class _>
+template <class Unused>
 void
-reporter<_>::suite_results::add(case_results const& r)
+reporter<Unused>::suite_results::add(case_results const& r)
 {
     ++cases;
     total += r.total;
     failed += r.failed;
 }
 
-template <class _>
+template <class Unused>
 void
-reporter<_>::results::add(suite_results const& r)
+reporter<Unused>::results::add(suite_results const& r)
 {
     ++suites;
     total += r.total;
@@ -160,13 +160,13 @@ reporter<_>::results::add(suite_results const& r)
 
 //------------------------------------------------------------------------------
 
-template <class _>
-reporter<_>::reporter(std::ostream& os) : os_(os)
+template <class Unused>
+reporter<Unused>::reporter(std::ostream& os) : os_(os)
 {
 }
 
-template <class _>
-reporter<_>::~reporter()
+template <class Unused>
+reporter<Unused>::~reporter()
 {
     if (results_.top.size() > 0)
     {
@@ -180,9 +180,9 @@ reporter<_>::~reporter()
         << amount{results_.failed, "failure"} << std::endl;
 }
 
-template <class _>
+template <class Unused>
 std::string
-reporter<_>::fmtdur(typename clock_type::duration const& d)
+reporter<Unused>::fmtdur(typename clock_type::duration const& d)
 {
     using namespace std::chrono;
     auto const ms = duration_cast<milliseconds>(d);
@@ -193,46 +193,46 @@ reporter<_>::fmtdur(typename clock_type::duration const& d)
     return ss.str();
 }
 
-template <class _>
+template <class Unused>
 void
-reporter<_>::on_suite_begin(suite_info const& info)
+reporter<Unused>::on_suite_begin(suite_info const& info)
 {
     suite_results_ = suite_results{info.full_name()};
 }
 
-template <class _>
+template <class Unused>
 void
-reporter<_>::on_suite_end()
+reporter<Unused>::on_suite_end()
 {
     results_.add(suite_results_);
 }
 
-template <class _>
+template <class Unused>
 void
-reporter<_>::on_case_begin(std::string const& name)
+reporter<Unused>::on_case_begin(std::string const& name)
 {
     case_results_ = case_results(name);
     os_ << suite_results_.name << (case_results_.name.empty() ? "" : (" " + case_results_.name))
         << std::endl;
 }
 
-template <class _>
+template <class Unused>
 void
-reporter<_>::on_case_end()
+reporter<Unused>::on_case_end()
 {
     suite_results_.add(case_results_);
 }
 
-template <class _>
+template <class Unused>
 void
-reporter<_>::on_pass()
+reporter<Unused>::on_pass()
 {
     ++case_results_.total;
 }
 
-template <class _>
+template <class Unused>
 void
-reporter<_>::on_fail(std::string const& reason)
+reporter<Unused>::on_fail(std::string const& reason)
 {
     ++case_results_.failed;
     ++case_results_.total;
@@ -240,9 +240,9 @@ reporter<_>::on_fail(std::string const& reason)
         << std::endl;
 }
 
-template <class _>
+template <class Unused>
 void
-reporter<_>::on_log(std::string const& s)
+reporter<Unused>::on_log(std::string const& s)
 {
     os_ << s;
 }

include/xrpl/beast/unit_test/results.h

@@ -145,9 +145,9 @@ public:
     void
     insert(case_results&& r)
     {
-        cont().emplace_back(std::move(r));
         total_ += r.tests.total();
         failed_ += r.tests.failed();
+        cont().emplace_back(std::move(r));
     }
 
     void

include/xrpl/beast/unit_test/suite.h

@@ -36,7 +36,7 @@ make_reason(String const& reason, char const* file, int line)
 
 }  // namespace detail
 
-class thread;
+class Thread;
 
 enum abort_t { no_abort_on_fail, abort_on_fail };
 
@@ -295,7 +295,7 @@ public:
     }
 
 private:
-    friend class thread;
+    friend class Thread;
 
     static suite**
     p_this_suite()
@@ -538,7 +538,7 @@ suite::run(runner& r)
     {
         run();
     }
-    catch (abort_exception const&)
+    catch (abort_exception const&)  // NOLINT(bugprone-empty-catch)
     {
         // ends the suite
     }

include/xrpl/beast/unit_test/thread.h

@@ -14,7 +14,7 @@ namespace beast {
 namespace unit_test {
 
 /** Replacement for std::thread that handles exceptions in unit tests. */
-class thread
+class Thread
 {
 private:
     suite* s_ = nullptr;
@@ -24,17 +24,17 @@ public:
     using id = std::thread::id;
     using native_handle_type = std::thread::native_handle_type;
 
-    thread() = default;
-    thread(thread const&) = delete;
-    thread&
-    operator=(thread const&) = delete;
+    Thread() = default;
+    Thread(Thread const&) = delete;
+    Thread&
+    operator=(Thread const&) = delete;
 
-    thread(thread&& other) : s_(other.s_), t_(std::move(other.t_))
+    Thread(Thread&& other) : s_(other.s_), t_(std::move(other.t_))
     {
     }
 
-    thread&
-    operator=(thread&& other)
+    Thread&
+    operator=(Thread&& other)
     {
         s_ = other.s_;
         t_ = std::move(other.t_);
@@ -42,10 +42,10 @@ public:
     }
 
     template <class F, class... Args>
-    explicit thread(suite& s, F&& f, Args&&... args) : s_(&s)
+    explicit Thread(suite& s, F&& f, Args&&... args) : s_(&s)
     {
         std::function<void(void)> b = std::bind(std::forward<F>(f), std::forward<Args>(args)...);
-        t_ = std::thread(&thread::run, this, std::move(b));
+        t_ = std::thread(&Thread::run, this, std::move(b));
     }
 
     bool
@@ -80,7 +80,7 @@ public:
     }
 
     void
-    swap(thread& other)
+    swap(Thread& other)
     {
         std::swap(s_, other.s_);
         std::swap(t_, other.t_);
@@ -94,7 +94,7 @@ private:
         {
             f();
         }
-        catch (suite::abort_exception const&)
+        catch (suite::abort_exception const&)  // NOLINT(bugprone-empty-catch)
         {
         }
         catch (std::exception const& e)

include/xrpl/beast/xor_shift_engine.h

@@ -43,15 +43,15 @@ private:
     murmurhash3(result_type x);
 };
 
-template <class _>
-xor_shift_engine<_>::xor_shift_engine(result_type val)
+template <class Unused>
+xor_shift_engine<Unused>::xor_shift_engine(result_type val)
 {
     seed(val);
 }
 
-template <class _>
+template <class Unused>
 void
-xor_shift_engine<_>::seed(result_type seed)
+xor_shift_engine<Unused>::seed(result_type seed)
 {
     if (seed == 0)
         throw std::domain_error("invalid seed");
@@ -59,9 +59,9 @@ xor_shift_engine<_>::seed(result_type seed)
     s_[1] = murmurhash3(s_[0]);
 }
 
-template <class _>
+template <class Unused>
 auto
-xor_shift_engine<_>::operator()() -> result_type
+xor_shift_engine<Unused>::operator()() -> result_type
 {
     result_type s1 = s_[0];
     result_type const s0 = s_[1];
@@ -70,9 +70,9 @@ xor_shift_engine<_>::operator()() -> result_type
     return (s_[1] = (s1 ^ s0 ^ (s1 >> 17) ^ (s0 >> 26))) + s0;
 }
 
-template <class _>
+template <class Unused>
 auto
-xor_shift_engine<_>::murmurhash3(result_type x) -> result_type
+xor_shift_engine<Unused>::murmurhash3(result_type x) -> result_type
 {
     x ^= x >> 33;
     x *= 0xff51afd7ed558ccdULL;

include/xrpl/ledger/helpers/NFTokenHelpers.h

@@ -20,10 +20,6 @@ removeTokenOffersWithLimit(
     Keylet const& directory,
     std::size_t maxDeletableOffers);
 
-/** Returns tesSUCCESS if NFToken has few enough offers that it can be burned */
-TER
-notTooManyOffers(ReadView const& view, uint256 const& nftokenID);
-
 /** Finds the specified token in the owner's token directory. */
 std::optional<STObject>
 findToken(ReadView const& view, AccountID const& owner, uint256 const& nftokenID);

include/xrpl/net/AutoSocket.h

@@ -134,7 +134,7 @@ public:
             {
                 lowest_layer().shutdown(plain_socket::shutdown_both);
             }
-            catch (boost::system::system_error& e)
+            catch (boost::system::system_error const& e)
             {
                 ec = e.code();
             }

include/xrpl/protocol/ApiVersion.h

@@ -138,9 +138,11 @@ forApiVersions(Fn const& fn, Args&&... args)
 {
     constexpr auto size = maxVer + 1 - minVer;
     [&]<std::size_t... offset>(std::index_sequence<offset...>) {
+        // NOLINTBEGIN(bugprone-use-after-move)
         (((void)fn(
              std::integral_constant<unsigned int, minVer + offset>{}, std::forward<Args>(args)...)),
          ...);
+        // NOLINTEND(bugprone-use-after-move)
     }(std::make_index_sequence<size>{});
 }
 

include/xrpl/protocol/Feature.h

@@ -125,10 +125,12 @@ namespace detail {
 #pragma push_macro("XRPL_RETIRE_FIX")
 #undef XRPL_RETIRE_FIX
 
+// NOLINTBEGIN(bugprone-macro-parentheses)
 #define XRPL_FEATURE(name, supported, vote) +1
 #define XRPL_FIX(name, supported, vote) +1
 #define XRPL_RETIRE_FEATURE(name) +1
 #define XRPL_RETIRE_FIX(name) +1
+// NOLINTEND(bugprone-macro-parentheses)
 
 // This value SHOULD be equal to the number of amendments registered in
 // Feature.cpp. Because it's only used to reserve storage, and determine how

include/xrpl/protocol/KnownFormats.h

@@ -74,10 +74,12 @@ public:
 
         Derived classes will load the object with all the known formats.
     */
+private:
     KnownFormats() : name_(beast::type_name<Derived>())
     {
     }
 
+public:
     /** Destroy the known formats object.
 
         The defined formats are deleted.
@@ -181,6 +183,7 @@ private:
 
     boost::container::flat_map<std::string, Item const*> names_;
     boost::container::flat_map<KeyType, Item const*> types_;
+    friend Derived;
 };
 
 }  // namespace xrpl

include/xrpl/protocol/LedgerFormats.h

@@ -211,7 +211,7 @@ enum LedgerEntryType : std::uint16_t {
 //     lsfRequireDestTag = 0x00020000,
 //     ...
 // };
-#define TO_VALUE(name, value) name = value,
+#define TO_VALUE(name, value) name = (value),
 #define NULL_NAME(name, values) values
 #define NULL_OUTPUT(name, value)
 enum LedgerSpecificFlags : std::uint32_t { XMACRO(NULL_NAME, TO_VALUE, NULL_OUTPUT) };

include/xrpl/protocol/Permissions.h

@@ -20,7 +20,7 @@ enum GranularPermissionType : std::uint32_t {
 #pragma push_macro("PERMISSION")
 #undef PERMISSION
 
-#define PERMISSION(type, txType, value) type = value,
+#define PERMISSION(type, txType, value) type = (value),
 
 #include <xrpl/protocol/detail/permissions.macro>
 

include/xrpl/protocol/SField.h

@@ -84,7 +84,7 @@ class STCurrency;
 #pragma push_macro("TO_MAP")
 #undef TO_MAP
 
-#define TO_ENUM(name, value) name = value,
+#define TO_ENUM(name, value) name = (value),
 #define TO_MAP(name, value) {#name, value},
 
 enum SerializedTypeID { XMACRO(TO_ENUM) };

include/xrpl/protocol/STAmount.h

@@ -401,7 +401,7 @@ amountFromJsonNoThrow(STAmount& result, Json::Value const& jvSource);
 inline STAmount const&
 toSTAmount(STAmount const& a)
 {
-    return a;
+    return a;  // NOLINT(bugprone-return-const-ref-from-parameter)
 }
 
 //------------------------------------------------------------------------------

include/xrpl/protocol/STObject.h

@@ -1188,6 +1188,7 @@ STObject::getFieldByConstRef(SField const& field, V const& empty) const
     SerializedTypeID const id = rf->getSType();
 
     if (id == STI_NOTPRESENT)
+        // NOLINTNEXTLINE(bugprone-return-const-ref-from-parameter)
         return empty;  // optional field not present
 
     T const* cf = dynamic_cast<T const*>(rf);

include/xrpl/protocol/TxFlags.h

@@ -235,7 +235,7 @@ XMACRO(NULL_NAME, TO_VALUE, NULL_OUTPUT, NULL_MASK_ADJ)
 // The mask adjustment (maskAdj) allows adding flags back to the mask, making them invalid.
 // For example, Batch uses MASK_ADJ(tfInnerBatchTxn) to reject tfInnerBatchTxn on outer Batch.
 #define TO_MASK(name, values, maskAdj) \
-    inline constexpr FlagValue tf##name##Mask = ~(tfUniversal values) | maskAdj;
+    inline constexpr FlagValue tf##name##Mask = ~(tfUniversal values) | (maskAdj);
 #define VALUE_TO_MASK(name, value) | name
 #define MASK_ADJ_TO_MASK(value) value
 XMACRO(TO_MASK, VALUE_TO_MASK, VALUE_TO_MASK, MASK_ADJ_TO_MASK)

include/xrpl/protocol/json_get_or_throw.h

@@ -146,7 +146,7 @@ getOptional(Json::Value const& v, xrpl::SField const& field)
     {
         return getOrThrow<T>(v, field);
     }
-    catch (...)
+    catch (...)  // NOLINT(bugprone-empty-catch)
     {
     }
     return {};

include/xrpl/protocol_autogen/README.md

@@ -4,42 +4,33 @@ This directory contains auto-generated C++ wrapper classes for XRP Ledger protoc
 
 ## Generated Files
 
-The files in this directory are automatically generated at **CMake configure time** from macro definition files:
+The files in this directory are generated from macro definition files:
 
-- **Transaction classes** (in `transactions/`): Generated from `include/xrpl/protocol/detail/transactions.macro` by `scripts/generate_tx_classes.py`
-- **Ledger entry classes** (in `ledger_entries/`): Generated from `include/xrpl/protocol/detail/ledger_entries.macro` by `scripts/generate_ledger_classes.py`
+- **Transaction classes** (in `transactions/`): Generated from `include/xrpl/protocol/detail/transactions.macro` by `cmake/scripts/codegen/generate_tx_classes.py`
+- **Ledger entry classes** (in `ledger_entries/`): Generated from `include/xrpl/protocol/detail/ledger_entries.macro` by `cmake/scripts/codegen/generate_ledger_classes.py`
 
 ## Generation Process
 
-The generation happens automatically when you **configure** the project (not during build). When you run CMake, the system:
+Generation requires a one-time setup step to create a virtual environment
+and install Python dependencies, followed by running the generation target:
 
-1. Creates a Python virtual environment in the build directory (`codegen_venv`)
-2. Installs Python dependencies from `scripts/requirements.txt` into the venv (only if needed)
-3. Runs the Python generation scripts using the venv Python interpreter
-4. Parses the macro files to extract type definitions
-5. Generates type-safe C++ wrapper classes using Mako templates
-6. Places the generated headers in this directory
+```bash
+cmake --build . --target setup_code_gen  # create venv and install dependencies (once)
+cmake --build . --target code_gen        # generate code
+```
 
-### When Regeneration Happens
-
-The code is regenerated when:
-
-- You run CMake configure for the first time
-- The Python virtual environment doesn't exist
-- `scripts/requirements.txt` has been modified
-
-To force regeneration, delete the build directory and reconfigure.
+By default, `CODEGEN_VENV_DIR` points to `.venv` in the project root. The
+`setup_code_gen` target creates a venv there and installs the required packages.
+The `code_gen` target then uses the venv's Python interpreter to run generation.
 
 ### Python Dependencies
 
-The code generation requires the following Python packages (automatically installed):
+The code generation requires the following Python packages (installed by `setup_code_gen`):
 
 - `pcpp` - C preprocessor for Python
 - `pyparsing` - Parser combinator library
 - `Mako` - Template engine
 
-These are isolated in a virtual environment and won't affect your system Python installation.
-
 ## Version Control
 
 The generated `.h` files **are checked into version control**. This means:
@@ -50,15 +41,15 @@ The generated `.h` files **are checked into version control**. This means:
 
 ## Modifying Generated Code
 
-**Do not manually edit generated files.** Any changes will be overwritten the next time CMake configure runs.
+**Do not manually edit generated files.** Any changes will be overwritten the next time `code_gen` is run.
 
 To modify the generated classes:
 
 - Edit the macro files in `include/xrpl/protocol/detail/`
-- Edit the Mako templates in `scripts/templates/`
-- Edit the generation scripts in `scripts/`
-- Update Python dependencies in `scripts/requirements.txt`
-- Run CMake configure to regenerate
+- Edit the Mako templates in `cmake/scripts/codegen/templates/`
+- Edit the generation scripts in `cmake/scripts/codegen/`
+- Update Python dependencies in `cmake/scripts/codegen/requirements.txt`
+- Run `cmake --build . --target code_gen` to regenerate
 
 ## Adding Common Fields
 
@@ -73,7 +64,7 @@ Base classes:
 
 Templates (update to pass required common fields to base class constructors):
 
-- `scripts/templates/Transaction.h.mako`
-- `scripts/templates/LedgerEntry.h.mako`
+- `cmake/scripts/codegen/templates/Transaction.h.mako`
+- `cmake/scripts/codegen/templates/LedgerEntry.h.mako`
 
 These files are **not auto-generated** and must be updated by hand.

include/xrpl/server/detail/BasePeer.h

@@ -34,6 +34,7 @@ protected:
     boost::asio::strand<boost::asio::executor> strand_;
 
 public:
+    // NOLINTNEXTLINE(bugprone-crtp-constructor-accessibility)
     BasePeer(
         Port const& port,
         Handler& handler,

include/xrpl/server/detail/BaseWSPeer.h

@@ -392,7 +392,7 @@ BaseWSPeer<Handler, Impl>::cancel_timer()
     {
         timer_.cancel();
     }
-    catch (boost::system::system_error const&)
+    catch (boost::system::system_error const&)  // NOLINT(bugprone-empty-catch)
     {
         // ignored
     }

include/xrpl/tx/paths/detail/FlowDebugInfo.h

@@ -220,6 +220,7 @@ struct FlowDebugInfo
                 write_list(amts, get_val, delim);
             };
             auto writeIntList = [&write_list](std::vector<size_t> const& vals, char delim = ';') {
+                // NOLINTNEXTLINE(bugprone-return-const-ref-from-parameter)
                 auto get_val = [](size_t const& v) -> size_t const& { return v; };
                 write_list(vals, get_val);
             };

include/xrpl/tx/paths/detail/Steps.h

@@ -429,8 +429,10 @@ toStrands(
 template <StepAmount TIn, StepAmount TOut, class TDerived>
 struct StepImp : public Step
 {
+private:
     explicit StepImp() = default;
 
+public:
     std::pair<EitherAmount, EitherAmount>
     rev(PaymentSandbox& sb,
         ApplyView& afView,
@@ -470,6 +472,7 @@ struct StepImp : public Step
     {
         return get<TIn>(lhs) == get<TIn>(rhs);
     }
+    friend TDerived;
 };
 /// @endcond
 

src/libxrpl/ledger/ApplyStateTable.cpp

@@ -5,6 +5,8 @@
 #include <xrpl/protocol/Feature.h>
 #include <xrpl/protocol/st.h>
 
+#include <stdexcept>
+
 namespace xrpl {
 namespace detail {
 
@@ -374,14 +376,14 @@ ApplyStateTable::erase(ReadView const& base, std::shared_ptr<SLE> const& sle)
 {
     auto const iter = items_.find(sle->key());
     if (iter == items_.end())
-        LogicError("ApplyStateTable::erase: missing key");
+        Throw<std::logic_error>("ApplyStateTable::erase: missing key");
     auto& item = iter->second;
     if (item.second != sle)
-        LogicError("ApplyStateTable::erase: unknown SLE");
+        Throw<std::logic_error>("ApplyStateTable::erase: unknown SLE");
     switch (item.first)
     {
         case Action::erase:
-            LogicError("ApplyStateTable::erase: double erase");
+            Throw<std::logic_error>("ApplyStateTable::erase: double erase");
             break;
         case Action::insert:
             items_.erase(iter);
@@ -405,7 +407,7 @@ ApplyStateTable::rawErase(ReadView const& base, std::shared_ptr<SLE> const& sle)
     switch (item.first)
     {
         case Action::erase:
-            LogicError("ApplyStateTable::rawErase: double erase");
+            Throw<std::logic_error>("ApplyStateTable::rawErase: double erase");
             break;
         case Action::insert:
             items_.erase(result.first);
@@ -436,11 +438,11 @@ ApplyStateTable::insert(ReadView const& base, std::shared_ptr<SLE> const& sle)
     switch (item.first)
     {
         case Action::cache:
-            LogicError("ApplyStateTable::insert: already cached");
+            Throw<std::logic_error>("ApplyStateTable::insert: already cached");
         case Action::insert:
-            LogicError("ApplyStateTable::insert: already inserted");
+            Throw<std::logic_error>("ApplyStateTable::insert: already inserted");
         case Action::modify:
-            LogicError("ApplyStateTable::insert: already modified");
+            Throw<std::logic_error>("ApplyStateTable::insert: already modified");
         case Action::erase:
             break;
     }
@@ -466,7 +468,7 @@ ApplyStateTable::replace(ReadView const& base, std::shared_ptr<SLE> const& sle)
     switch (item.first)
     {
         case Action::erase:
-            LogicError("ApplyStateTable::replace: already erased");
+            Throw<std::logic_error>("ApplyStateTable::replace: already erased");
         case Action::cache:
             item.first = Action::modify;
             break;
@@ -482,14 +484,14 @@ ApplyStateTable::update(ReadView const& base, std::shared_ptr<SLE> const& sle)
 {
     auto const iter = items_.find(sle->key());
     if (iter == items_.end())
-        LogicError("ApplyStateTable::update: missing key");
+        Throw<std::logic_error>("ApplyStateTable::update: missing key");
     auto& item = iter->second;
     if (item.second != sle)
-        LogicError("ApplyStateTable::update: unknown SLE");
+        Throw<std::logic_error>("ApplyStateTable::update: unknown SLE");
     switch (item.first)
     {
         case Action::erase:
-            LogicError("ApplyStateTable::update: erased");
+            Throw<std::logic_error>("ApplyStateTable::update: erased");
             break;
         case Action::cache:
             item.first = Action::modify;

src/libxrpl/ledger/ApplyView.cpp

@@ -4,6 +4,7 @@
 #include <xrpl/protocol/Protocol.h>
 
 #include <limits>
+#include <stdexcept>
 #include <type_traits>
 
 namespace xrpl {
@@ -40,10 +41,8 @@ findPreviousPage(ApplyView& view, Keylet const& directory, SLE::ref start)
     {
         node = view.peek(keylet::page(directory, page));
         if (!node)
-        {  // LCOV_EXCL_START
-            LogicError("Directory chain: root back-pointer broken.");
-            // LCOV_EXCL_STOP
-        }
+            Throw<std::logic_error>(
+                "Directory chain: root back-pointer broken.");  // LCOV_EXCL_LINE
     }
 
     auto indexes = node->getFieldV256(sfIndexes);
@@ -62,21 +61,20 @@ insertKey(
     if (preserveOrder)
     {
         if (std::find(indexes.begin(), indexes.end(), key) != indexes.end())
-            LogicError("dirInsert: double insertion");  // LCOV_EXCL_LINE
+            Throw<std::logic_error>("dirInsert: double insertion");  // LCOV_EXCL_LINE
 
         indexes.push_back(key);
     }
     else
     {
-        // We can't be sure if this page is already sorted because
-        // it may be a legacy page we haven't yet touched. Take
-        // the time to sort it.
+        // We can't be sure if this page is already sorted because it may be a
+        // legacy page we haven't yet touched. Take the time to sort it.
         std::sort(indexes.begin(), indexes.end());
 
         auto pos = std::lower_bound(indexes.begin(), indexes.end(), key);
 
         if (pos != indexes.end() && key == *pos)
-            LogicError("dirInsert: double insertion");  // LCOV_EXCL_LINE
+            Throw<std::logic_error>("dirInsert: double insertion");  // LCOV_EXCL_LINE
 
         indexes.insert(pos, key);
     }
@@ -129,8 +127,7 @@ insertPage(
     node->setFieldH256(sfRootIndex, directory.key);
     node->setFieldV256(sfIndexes, indexes);
 
-    // Save some space by not specifying the value 0 since
-    // it's the default.
+    // Save some space by not specifying the value 0 since it's the default.
     if (page != 1)
         node->setFieldU64(sfIndexPrevious, page - 1);
     XRPL_ASSERT_PARTS(!nextPage, "xrpl::directory::insertPage", "nextPage has default value");
@@ -199,28 +196,24 @@ ApplyView::emptyDirDelete(Keylet const& directory)
     auto nextPage = node->getFieldU64(sfIndexNext);
 
     if (nextPage == rootPage && prevPage != rootPage)
-        LogicError("Directory chain: fwd link broken");  // LCOV_EXCL_LINE
+        Throw<std::logic_error>("Directory chain: fwd link broken");  // LCOV_EXCL_LINE
 
     if (prevPage == rootPage && nextPage != rootPage)
-        LogicError("Directory chain: rev link broken");  // LCOV_EXCL_LINE
+        Throw<std::logic_error>("Directory chain: rev link broken");  // LCOV_EXCL_LINE
 
-    // Older versions of the code would, in some cases, allow the last
-    // page to be empty. Remove such pages:
+    // Older versions of the code would, in some cases, allow the last page to
+    // be empty. Remove such pages:
     if (nextPage == prevPage && nextPage != rootPage)
     {
         auto last = peek(keylet::page(directory, nextPage));
 
         if (!last)
-        {  // LCOV_EXCL_START
-            LogicError("Directory chain: fwd link broken.");
-            // LCOV_EXCL_STOP
-        }
+            Throw<std::logic_error>("Directory chain: fwd link broken.");  // LCOV_EXCL_LINE
 
         if (!last->getFieldV256(sfIndexes).empty())
             return false;
 
-        // Update the first page's linked list and
-        // mark it as updated.
+        // Update the first page's linked list and mark it as updated.
         node->setFieldU64(sfIndexNext, rootPage);
         node->setFieldU64(sfIndexPrevious, rootPage);
         update(node);
@@ -228,8 +221,7 @@ ApplyView::emptyDirDelete(Keylet const& directory)
         // And erase the empty last page:
         erase(last);
 
-        // Make sure our local values reflect the
-        // updated information:
+        // Make sure our local values reflect the updated information:
         nextPage = rootPage;
         prevPage = rootPage;
     }
@@ -269,46 +261,33 @@ ApplyView::dirRemove(Keylet const& directory, std::uint64_t page, uint256 const&
             return true;
     }
 
-    // The current page is now empty; check if it can be
-    // deleted, and, if so, whether the entire directory
-    // can now be removed.
+    // The current page is now empty; check if it can be deleted, and, if so,
+    // whether the entire directory can now be removed.
     auto prevPage = node->getFieldU64(sfIndexPrevious);
     auto nextPage = node->getFieldU64(sfIndexNext);
 
-    // The first page is the directory's root node and is
-    // treated specially: it can never be deleted even if
-    // it is empty, unless we plan on removing the entire
-    // directory.
+    // The first page is the directory's root node and is treated specially: it
+    // can never be deleted even if it is empty, unless we plan on removing the
+    // entire directory.
     if (page == rootPage)
     {
         if (nextPage == page && prevPage != page)
-        {  // LCOV_EXCL_START
-            LogicError("Directory chain: fwd link broken");
-            // LCOV_EXCL_STOP
-        }
+            Throw<std::logic_error>("Directory chain: fwd link broken");  // LCOV_EXCL_LINE
 
         if (prevPage == page && nextPage != page)
-        {  // LCOV_EXCL_START
-            LogicError("Directory chain: rev link broken");
-            // LCOV_EXCL_STOP
-        }
+            Throw<std::logic_error>("Directory chain: rev link broken");  // LCOV_EXCL_LINE
 
-        // Older versions of the code would, in some cases,
-        // allow the last page to be empty. Remove such
-        // pages if we stumble on them:
+        // Older versions of the code would, in some cases, allow the last page
+        // to be empty. Remove such pages if we stumble on them:
         if (nextPage == prevPage && nextPage != page)
         {
             auto last = peek(keylet::page(directory, nextPage));
             if (!last)
-            {  // LCOV_EXCL_START
-                LogicError("Directory chain: fwd link broken.");
-                // LCOV_EXCL_STOP
-            }
+                Throw<std::logic_error>("Directory chain: fwd link broken.");  // LCOV_EXCL_LINE
 
             if (last->getFieldV256(sfIndexes).empty())
             {
-                // Update the first page's linked list and
-                // mark it as updated.
+                // Update the first page's linked list and mark it as updated.
                 node->setFieldU64(sfIndexNext, page);
                 node->setFieldU64(sfIndexPrevious, page);
                 update(node);
@@ -316,8 +295,7 @@ ApplyView::dirRemove(Keylet const& directory, std::uint64_t page, uint256 const&
                 // And erase the empty last page:
                 erase(last);
 
-                // Make sure our local values reflect the
-                // updated information:
+                // Make sure our local values reflect the updated information:
                 nextPage = page;
                 prevPage = page;
             }
@@ -335,25 +313,24 @@ ApplyView::dirRemove(Keylet const& directory, std::uint64_t page, uint256 const&
 
     // This can never happen for nodes other than the root:
     if (nextPage == page)
-        LogicError("Directory chain: fwd link broken");  // LCOV_EXCL_LINE
+        Throw<std::logic_error>("Directory chain: fwd link broken");  // LCOV_EXCL_LINE
 
     if (prevPage == page)
-        LogicError("Directory chain: rev link broken");  // LCOV_EXCL_LINE
+        Throw<std::logic_error>("Directory chain: rev link broken");  // LCOV_EXCL_LINE
 
-    // This node isn't the root, so it can either be in the
-    // middle of the list, or at the end. Unlink it first
-    // and then check if that leaves the list with only a
-    // root:
+    // This node isn't the root, so it can either be in the middle of the list,
+    // or at the end. Unlink it first and then check if that leaves the list
+    // with only a root:
     auto prev = peek(keylet::page(directory, prevPage));
     if (!prev)
-        LogicError("Directory chain: fwd link broken.");  // LCOV_EXCL_LINE
+        Throw<std::logic_error>("Directory chain: fwd link broken.");  // LCOV_EXCL_LINE
     // Fix previous to point to its new next.
     prev->setFieldU64(sfIndexNext, nextPage);
     update(prev);
 
     auto next = peek(keylet::page(directory, nextPage));
     if (!next)
-        LogicError("Directory chain: rev link broken.");  // LCOV_EXCL_LINE
+        Throw<std::logic_error>("Directory chain: rev link broken.");  // LCOV_EXCL_LINE
     // Fix next to point to its new previous.
     next->setFieldU64(sfIndexPrevious, prevPage);
     update(next);
@@ -361,13 +338,12 @@ ApplyView::dirRemove(Keylet const& directory, std::uint64_t page, uint256 const&
     // The page is no longer linked. Delete it.
     erase(node);
 
-    // Check whether the next page is the last page and, if
-    // so, whether it's empty. If it is, delete it.
+    // Check whether the next page is the last page and, if so, whether it's
+    // empty. If it is, delete it.
     if (nextPage != rootPage && next->getFieldU64(sfIndexNext) == rootPage &&
         next->getFieldV256(sfIndexes).empty())
     {
-        // Since next doesn't point to the root, it
-        // can't be pointing to prev.
+        // Since next doesn't point to the root, it can't be pointing to prev.
         erase(next);
 
         // The previous page is now the last page:
@@ -377,18 +353,16 @@ ApplyView::dirRemove(Keylet const& directory, std::uint64_t page, uint256 const&
         // And the root points to the last page:
         auto root = peek(keylet::page(directory, rootPage));
         if (!root)
-        {  // LCOV_EXCL_START
-            LogicError("Directory chain: root link broken.");
-            // LCOV_EXCL_STOP
-        }
+            Throw<std::logic_error>("Directory chain: root link broken.");  // LCOV_EXCL_LINE
+
         root->setFieldU64(sfIndexPrevious, prevPage);
         update(root);
 
         nextPage = rootPage;
     }
 
-    // If we're not keeping the root, then check to see if
-    // it's left empty. If so, delete it as well.
+    // If we're not keeping the root, then check to see if it's left empty.
+    // If so, delete it as well.
     if (!keepRoot && nextPage == rootPage && prevPage == rootPage)
     {
         if (prev->getFieldV256(sfIndexes).empty())

src/libxrpl/ledger/Ledger.cpp

@@ -12,6 +12,7 @@
 #include <xrpl/protocol/digest.h>
 #include <xrpl/protocol/jss.h>
 
+#include <stdexcept>
 #include <utility>
 #include <vector>
 
@@ -461,14 +462,14 @@ void
 Ledger::rawErase(std::shared_ptr<SLE> const& sle)
 {
     if (!stateMap_.delItem(sle->key()))
-        LogicError("Ledger::rawErase: key not found");
+        Throw<std::logic_error>("Ledger::rawErase: key not found");
 }
 
 void
 Ledger::rawErase(uint256 const& key)
 {
     if (!stateMap_.delItem(key))
-        LogicError("Ledger::rawErase: key not found");
+        Throw<std::logic_error>("Ledger::rawErase: key not found");
 }
 
 void
@@ -478,7 +479,7 @@ Ledger::rawInsert(std::shared_ptr<SLE> const& sle)
     sle->add(ss);
     if (!stateMap_.addGiveItem(
             SHAMapNodeType::tnACCOUNT_STATE, make_shamapitem(sle->key(), ss.slice())))
-        LogicError("Ledger::rawInsert: key already exists");
+        Throw<std::logic_error>("Ledger::rawInsert: key already exists");
 }
 
 void
@@ -488,7 +489,7 @@ Ledger::rawReplace(std::shared_ptr<SLE> const& sle)
     sle->add(ss);
     if (!stateMap_.updateGiveItem(
             SHAMapNodeType::tnACCOUNT_STATE, make_shamapitem(sle->key(), ss.slice())))
-        LogicError("Ledger::rawReplace: key not found");
+        Throw<std::logic_error>("Ledger::rawReplace: key not found");
 }
 
 void
@@ -504,7 +505,7 @@ Ledger::rawTxInsert(
     s.addVL(txn->peekData());
     s.addVL(metaData->peekData());
     if (!txMap_.addGiveItem(SHAMapNodeType::tnTRANSACTION_MD, make_shamapitem(key, s.slice())))
-        LogicError("duplicate_tx: " + to_string(key));
+        Throw<std::logic_error>("duplicate_tx: " + to_string(key));
 }
 
 uint256
@@ -522,7 +523,7 @@ Ledger::rawTxInsertWithHash(
     auto item = make_shamapitem(key, s.slice());
     auto hash = sha512Half(HashPrefix::txNode, item->slice(), item->key());
     if (!txMap_.addGiveItem(SHAMapNodeType::tnTRANSACTION_MD, std::move(item)))
-        LogicError("duplicate_tx: " + to_string(key));
+        Throw<std::logic_error>("duplicate_tx: " + to_string(key));
 
     return hash;
 }

src/libxrpl/ledger/OpenView.cpp

@@ -1,6 +1,8 @@
 #include <xrpl/basics/contract.h>
 #include <xrpl/ledger/OpenView.h>
 
+#include <stdexcept>
+
 namespace xrpl {
 
 class OpenView::txs_iter_impl : public txs_type::iter_base
@@ -247,7 +249,7 @@ OpenView::rawTxInsert(
     auto const result = txs_.emplace(
         std::piecewise_construct, std::forward_as_tuple(key), std::forward_as_tuple(txn, metaData));
     if (!result.second)
-        LogicError("rawTxInsert: duplicate TX id: " + to_string(key));
+        Throw<std::logic_error>("rawTxInsert: duplicate TX id: " + to_string(key));
 }
 
 }  // namespace xrpl

src/libxrpl/ledger/RawStateTable.cpp

@@ -1,6 +1,8 @@
 #include <xrpl/basics/contract.h>
 #include <xrpl/ledger/detail/RawStateTable.h>
 
+#include <stdexcept>
+
 namespace xrpl {
 namespace detail {
 
@@ -241,7 +243,7 @@ RawStateTable::erase(std::shared_ptr<SLE> const& sle)
     switch (item.action)
     {
         case Action::erase:
-            LogicError("RawStateTable::erase: already erased");
+            Throw<std::logic_error>("RawStateTable::erase: already erased");
             break;
         case Action::insert:
             items_.erase(result.first);
@@ -270,10 +272,10 @@ RawStateTable::insert(std::shared_ptr<SLE> const& sle)
             item.sle = sle;
             break;
         case Action::insert:
-            LogicError("RawStateTable::insert: already inserted");
+            Throw<std::logic_error>("RawStateTable::insert: already inserted");
             break;
         case Action::replace:
-            LogicError("RawStateTable::insert: already exists");
+            Throw<std::logic_error>("RawStateTable::insert: already exists");
             break;
     }
 }
@@ -291,7 +293,7 @@ RawStateTable::replace(std::shared_ptr<SLE> const& sle)
     switch (item.action)
     {
         case Action::erase:
-            LogicError("RawStateTable::replace: was erased");
+            Throw<std::logic_error>("RawStateTable::replace: was erased");
             break;
         case Action::insert:
         case Action::replace:

src/libxrpl/ledger/helpers/AccountRootHelpers.cpp

@@ -8,6 +8,7 @@
 
 #include <algorithm>
 #include <limits>
+#include <stdexcept>
 
 namespace xrpl {
 
@@ -153,7 +154,7 @@ getPseudoAccountFields()
         if (!ar)
         {
             // LCOV_EXCL_START
-            LogicError(
+            Throw<std::logic_error>(
                 "xrpl::getPseudoAccountFields : unable to find account root "
                 "ledger format");
             // LCOV_EXCL_STOP

src/libxrpl/ledger/helpers/NFTokenHelpers.cpp

@@ -607,33 +607,6 @@ removeTokenOffersWithLimit(ApplyView& view, Keylet const& directory, std::size_t
     return deletedOffersCount;
 }
 
-TER
-notTooManyOffers(ReadView const& view, uint256 const& nftokenID)
-{
-    std::size_t totalOffers = 0;
-
-    {
-        Dir const buys(view, keylet::nft_buys(nftokenID));
-        for (auto iter = buys.begin(); iter != buys.end(); iter.next_page())
-        {
-            totalOffers += iter.page_size();
-            if (totalOffers > maxDeletableTokenOfferEntries)
-                return tefTOO_BIG;
-        }
-    }
-
-    {
-        Dir const sells(view, keylet::nft_sells(nftokenID));
-        for (auto iter = sells.begin(); iter != sells.end(); iter.next_page())
-        {
-            totalOffers += iter.page_size();
-            if (totalOffers > maxDeletableTokenOfferEntries)
-                return tefTOO_BIG;
-        }
-    }
-    return tesSUCCESS;
-}
-
 bool
 deleteTokenOffer(ApplyView& view, std::shared_ptr<SLE> const& offer)
 {

src/test/core/Config_test.cpp

@@ -507,7 +507,7 @@ port_wss_admin
             {
                 c.loadFromString(boost::str(configTemplate % validationSeed % token));
             }
-            catch (std::runtime_error& e)
+            catch (std::runtime_error const& e)
             {
                 error = e.what();
             }
@@ -528,7 +528,7 @@ port_wss_admin
 main
 )xrpldConfig");
         }
-        catch (std::runtime_error& e)
+        catch (std::runtime_error const& e)
         {
             error = e.what();
         }
@@ -541,7 +541,7 @@ main
             c.loadFromString(R"xrpldConfig(
 )xrpldConfig");
         }
-        catch (std::runtime_error& e)
+        catch (std::runtime_error const& e)
         {
             error = e.what();
         }
@@ -556,7 +556,7 @@ main
 255
 )xrpldConfig");
         }
-        catch (std::runtime_error& e)
+        catch (std::runtime_error const& e)
         {
             error = e.what();
         }
@@ -571,7 +571,7 @@ main
 10000
 )xrpldConfig");
         }
-        catch (std::runtime_error& e)
+        catch (std::runtime_error const& e)
         {
             error = e.what();
         }
@@ -598,7 +598,7 @@ main
                 Config c;
                 c.loadFromString(boost::str(cc % missingPath));
             }
-            catch (std::runtime_error& e)
+            catch (std::runtime_error const& e)
             {
                 error = e.what();
             }
@@ -617,7 +617,7 @@ main
                 Config c;
                 c.loadFromString(boost::str(cc % invalidFile.string()));
             }
-            catch (std::runtime_error& e)
+            catch (std::runtime_error const& e)
             {
                 error = e.what();
             }
@@ -725,7 +725,7 @@ trust-these-validators.gov
                 c.loadFromString(toLoad);
                 fail();
             }
-            catch (std::runtime_error& e)
+            catch (std::runtime_error const& e)
             {
                 error = e.what();
             }
@@ -754,7 +754,7 @@ value = 2
                 c.loadFromString(toLoad);
                 fail();
             }
-            catch (std::runtime_error& e)
+            catch (std::runtime_error const& e)
             {
                 error = e.what();
             }
@@ -802,7 +802,7 @@ trust-these-validators.gov
                 c.loadFromString(toLoad);
                 fail();
             }
-            catch (std::runtime_error& e)
+            catch (std::runtime_error const& e)
             {
                 error = e.what();
             }
@@ -948,7 +948,7 @@ trust-these-validators.gov
                 c.loadFromString(boost::str(cc % vtg.validatorsFile()));
                 fail();
             }
-            catch (std::runtime_error& e)
+            catch (std::runtime_error const& e)
             {
                 error = e.what();
             }
@@ -974,7 +974,7 @@ trust-these-validators.gov
                 Config c2;
                 c2.loadFromString(boost::str(cc % vtg.validatorsFile()));
             }
-            catch (std::runtime_error& e)
+            catch (std::runtime_error const& e)
             {
                 error = e.what();
             }
@@ -1451,7 +1451,7 @@ r.ripple.com:51235
                     fail();
                 }
             }
-            catch (std::runtime_error&)
+            catch (std::runtime_error const&)
             {
                 if (!shouldPass)
                 {
@@ -1477,7 +1477,7 @@ r.ripple.com:51235
                 c.loadFromString("[overlay]\nmax_unknown_time=" + value);
                 return c.MAX_UNKNOWN_TIME;
             }
-            catch (std::runtime_error&)
+            catch (std::runtime_error const&)
             {
                 return {};
             }
@@ -1511,7 +1511,7 @@ r.ripple.com:51235
                 c.loadFromString("[overlay]\nmax_diverged_time=" + value);
                 return c.MAX_DIVERGED_TIME;
             }
-            catch (std::runtime_error&)
+            catch (std::runtime_error const&)
             {
                 return {};
             }

src/test/csf/Scheduler.h

@@ -265,7 +265,7 @@ inline Scheduler::queue_type::~queue_type()
         auto e = &*iter;
         ++iter;
         e->~event();
-        alloc_->deallocate(e, sizeof(e));
+        alloc_->deallocate(e, sizeof(e));  // NOLINT(bugprone-sizeof-expression)
     }
 }
 

src/test/jtx/TrustedPublisherServer.h

@@ -452,7 +452,7 @@ private:
         bool ssl;
 
         lambda(int id_, TrustedPublisherServer& self_, socket_type&& sock_, bool ssl_)
-            : id(id_), self(self_), sock(std::move(sock_)), work(sock_.get_executor()), ssl(ssl_)
+            : id(id_), self(self_), sock(std::move(sock_)), work(sock.get_executor()), ssl(ssl_)
         {
         }
 

src/test/nodestore/TestBase.h

@@ -75,9 +75,10 @@ public:
                         return hotTRANSACTION_NODE;
                     case 3:
                         return hotUNKNOWN;
+                    default:
+                        // will never happen, but make static analysis tool happy.
+                        return hotUNKNOWN;
                 }
-                // will never happen, but make static analysis tool happy.
-                return hotUNKNOWN;
             }();
 
             uint256 hash;

src/test/nodestore/Timing_test.cpp

@@ -211,7 +211,7 @@ public:
     parallel_for(std::size_t const n, std::size_t number_of_threads, Args const&... args)
     {
         std::atomic<std::size_t> c(0);
-        std::vector<beast::unit_test::thread> t;
+        std::vector<beast::unit_test::Thread> t;
         t.reserve(number_of_threads);
         for (std::size_t id = 0; id < number_of_threads; ++id)
             t.emplace_back(*this, parallel_for_lambda<Body>(n, c), args...);
@@ -224,7 +224,7 @@ public:
     parallel_for_id(std::size_t const n, std::size_t number_of_threads, Args const&... args)
     {
         std::atomic<std::size_t> c(0);
-        std::vector<beast::unit_test::thread> t;
+        std::vector<beast::unit_test::Thread> t;
         t.reserve(number_of_threads);
         for (std::size_t id = 0; id < number_of_threads; ++id)
             t.emplace_back(*this, parallel_for_lambda<Body>(n, c), id, args...);

src/test/overlay/reduce_relay_test.cpp

@@ -1345,7 +1345,7 @@ vp_enable=0
             {
                 c.loadFromString(toLoad);
             }
-            catch (std::runtime_error& e)
+            catch (std::runtime_error const& e)
             {
                 error = e.what();
             }
@@ -1389,7 +1389,7 @@ vp_base_squelch_max_selected_peers=2
             {
                 c2.loadFromString(toLoad);
             }
-            catch (std::runtime_error& e)
+            catch (std::runtime_error const& e)
             {
                 error = e.what();
             }

src/test/protocol/STParsedJSON_test.cpp

@@ -2130,7 +2130,7 @@ class STParsedJSON_test : public beast::unit_test::suite
                 STParsedJSONObject const parsed("test", faultyJson);
                 BEAST_EXPECT(!parsed.object);
             }
-            catch (std::runtime_error& e)
+            catch (std::runtime_error const& e)
             {
                 std::string const what(e.what());
                 unexpected(what.find("First level children of `Template`") != 0);

src/tests/libxrpl/CMakeLists.txt

@@ -32,20 +32,9 @@ xrpl_add_test(json)
 target_link_libraries(xrpl.test.json PRIVATE xrpl.imports.test)
 add_dependencies(xrpl.tests xrpl.test.json)
 
-# protocol_autogen tests use explicit source list (not GLOB) because sources are generated
-# Mark generated sources so CMake knows they'll be created at build time
-set_source_files_properties(
-    ${PROTOCOL_AUTOGEN_TEST_SOURCES}
-    PROPERTIES GENERATED TRUE
-)
-add_executable(xrpl.test.protocol_autogen ${PROTOCOL_AUTOGEN_TEST_SOURCES})
+xrpl_add_test(protocol_autogen)
 target_link_libraries(xrpl.test.protocol_autogen PRIVATE xrpl.imports.test)
 add_dependencies(xrpl.tests xrpl.test.protocol_autogen)
-add_test(NAME xrpl.test.protocol_autogen COMMAND xrpl.test.protocol_autogen)
-# Ensure code generation runs before compiling tests
-if(TARGET protocol_autogen_generate)
-    add_dependencies(xrpl.test.protocol_autogen protocol_autogen_generate)
-endif()
 
 # Network unit tests are currently not supported on Windows
 if(NOT WIN32)

src/xrpld/app/main/Application.h

@@ -16,15 +16,6 @@
 
 namespace xrpl {
 
-namespace unl {
-class Manager;
-}  // namespace unl
-namespace Resource {
-class Manager;
-}  // namespace Resource
-namespace NodeStore {
-class Database;
-}  // namespace NodeStore
 namespace perf {
 class PerfLog;
 }  // namespace perf

src/xrpld/app/misc/Transaction.h

@@ -21,7 +21,6 @@ namespace xrpl {
 //
 
 class Application;
-class Database;
 class Rules;
 
 enum TransStatus {

src/xrpld/app/misc/detail/WorkBase.h

@@ -47,7 +47,7 @@ protected:
     endpoint_type lastEndpoint_;
     bool lastStatus_;
 
-public:
+private:
     WorkBase(
         std::string const& host,
         std::string const& path,
@@ -56,6 +56,8 @@ public:
         endpoint_type const& lastEndpoint,
         bool lastStatus,
         callback_type cb);
+
+public:
     ~WorkBase();
 
     Impl&
@@ -91,6 +93,8 @@ public:
 private:
     void
     close();
+
+    friend Impl;
 };
 
 //------------------------------------------------------------------------------

src/xrpld/overlay/Compression.h

@@ -49,7 +49,7 @@ decompress(
             // LCOV_EXCL_STOP
         }
     }
-    catch (...)
+    catch (...)  // NOLINT(bugprone-empty-catch)
     {
     }
     return 0;
@@ -88,7 +88,7 @@ compress(
             // LCOV_EXCL_STOP
         }
     }
-    catch (...)
+    catch (...)  // NOLINT(bugprone-empty-catch)
     {
     }
     return 0;

src/xrpld/rpc/detail/PathRequest.h

@@ -24,7 +24,7 @@ class AssetCache;
 class PathRequestManager;
 
 // Return values from parseJson <0 = invalid, >0 = valid
-#define PFR_PJ_INVALID -1
+#define PFR_PJ_INVALID (-1)
 #define PFR_PJ_NOCHANGE 0
 
 class PathRequest final : public InfoSubRequest,

src/xrpld/rpc/detail/RPCCall.cpp

@@ -1632,7 +1632,7 @@ rpcClient(
         // YYY We could have a command line flag for single line output for
         // scripts. YYY We would intercept output here and simplify it.
     }
-    catch (RequestNotParsable& e)
+    catch (RequestNotParsable const& e)
     {
         jvOutput = rpcError(rpcINVALID_PARAMS);
         jvOutput["error_what"] = e.what();

src/xrpld/rpc/detail/TransactionSign.cpp

@@ -619,7 +619,7 @@ transactionPreProcessImpl(
 
         stTx = std::make_shared<STTx>(std::move(parsed.object.value()));
     }
-    catch (STObject::FieldErr& err)
+    catch (STObject::FieldErr const& err)
     {
         return RPC::make_error(rpcINVALID_PARAMS, err.what());
     }
@@ -1291,7 +1291,7 @@ transactionSubmitMultiSigned(
         {
             stTx = std::make_shared<STTx>(std::move(parsedTx_json.object.value()));
         }
-        catch (STObject::FieldErr& err)
+        catch (STObject::FieldErr const& err)
         {
             return RPC::make_error(rpcINVALID_PARAMS, err.what());
         }

src/xrpld/rpc/handlers/ledger/LedgerEntry.cpp

@@ -897,7 +897,7 @@ doLedgerEntry(RPC::JsonContext& context)
             return RPC::make_param_error("No ledger_entry params provided.");
         }
     }
-    catch (Json::error& e)
+    catch (Json::error const& e)
     {
         if (context.apiVersion > 1u)
         {

src/xrpld/rpc/handlers/subscribe/Subscribe.cpp

@@ -65,7 +65,7 @@ doSubscribe(RPC::JsonContext& context)
                 ispSub =
                     context.netOps.addRpcSub(strUrl, std::dynamic_pointer_cast<InfoSub>(rspSub));
             }
-            catch (std::runtime_error& ex)
+            catch (std::runtime_error const& ex)
             {
                 return RPC::make_param_error(ex.what());
             }