diff --git a/.clang-tidy b/.clang-tidy index 6a967532db..33569be50a 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -1,11 +1,10 @@ --- -# This entire group of checks was applied to all cpp files but not all header files. -# --- Checks: "-*, bugprone-argument-comment, bugprone-assert-side-effect, bugprone-bad-signal-to-kill-thread, bugprone-bool-pointer-implicit-conversion, + bugprone-capturing-this-in-member-variable, bugprone-casting-through-void, bugprone-chained-comparison, bugprone-compare-pointer-to-member-virtual-function, @@ -25,6 +24,7 @@ Checks: "-*, bugprone-lambda-function-name, bugprone-macro-parentheses, bugprone-macro-repeated-side-effects, + bugprone-misleading-setter-of-reference, bugprone-misplaced-operator-in-strlen-in-alloc, bugprone-misplaced-pointer-arithmetic-in-alloc, bugprone-misplaced-widening-cast, @@ -73,10 +73,10 @@ Checks: "-*, bugprone-unhandled-self-assignment, bugprone-unique-ptr-array-mismatch, bugprone-unsafe-functions, - bugprone-use-after-move, + bugprone-unused-local-non-trivial-variable, bugprone-unused-raii, bugprone-unused-return-value, - bugprone-unused-local-non-trivial-variable, + bugprone-use-after-move, bugprone-virtual-near-miss, cppcoreguidelines-init-variables, cppcoreguidelines-misleading-capture-default-by-value, @@ -85,8 +85,10 @@ Checks: "-*, cppcoreguidelines-pro-type-static-cast-downcast, cppcoreguidelines-rvalue-reference-param-not-moved, cppcoreguidelines-use-default-member-init, + cppcoreguidelines-use-enum-class, cppcoreguidelines-virtual-class-destructor, hicpp-ignored-remove-result, + llvm-namespace-comment, misc-const-correctness, misc-definitions-in-headers, misc-header-include-cycle, @@ -98,6 +100,7 @@ Checks: "-*, misc-unused-alias-decls, misc-unused-using-decls, modernize-concat-nested-namespaces, + modernize-deprecated-headers, modernize-make-shared, modernize-make-unique, modernize-pass-by-value, @@ -109,11 +112,10 @@ Checks: "-*, modernize-use-nodiscard, modernize-use-override, modernize-use-ranges, + modernize-use-scoped-lock, modernize-use-starts-ends-with, modernize-use-std-numbers, modernize-use-using, - modernize-deprecated-headers, - llvm-namespace-comment, performance-faster-string-find, performance-for-range-copy, performance-implicit-conversion-in-loop, @@ -122,6 +124,7 @@ Checks: "-*, performance-move-constructor-init, performance-no-automatic-move, performance-trivially-destructible, + readability-ambiguous-smartptr-reset-call, readability-avoid-nested-conditional-operator, readability-avoid-return-with-void-value, readability-braces-around-statements, @@ -132,6 +135,7 @@ Checks: "-*, readability-duplicate-include, readability-else-after-return, readability-enum-initial-value, + readability-identifier-naming, readability-implicit-bool-conversion, readability-make-member-function-const, readability-math-missing-parentheses, @@ -149,52 +153,47 @@ Checks: "-*, readability-use-std-min-max " # --- -# other checks that have issues that need to be resolved: -# # readability-inconsistent-declaration-parameter-name, # in this codebase this check will break a lot of arg names # readability-static-accessed-through-instance, # this check is probably unnecessary. it makes the code less readable -# readability-identifier-naming, # https://github.com/XRPLF/rippled/pull/6571 # --- -# CheckOptions: readability-braces-around-statements.ShortStatementLines: 2 - # readability-identifier-naming.MacroDefinitionCase: UPPER_CASE - # readability-identifier-naming.ClassCase: CamelCase - # readability-identifier-naming.StructCase: CamelCase - # readability-identifier-naming.UnionCase: CamelCase - # readability-identifier-naming.EnumCase: CamelCase - # readability-identifier-naming.EnumConstantCase: CamelCase - # readability-identifier-naming.ScopedEnumConstantCase: CamelCase - # readability-identifier-naming.GlobalConstantCase: UPPER_CASE - # readability-identifier-naming.GlobalConstantPrefix: "k" - # readability-identifier-naming.GlobalVariableCase: CamelCase - # readability-identifier-naming.GlobalVariablePrefix: "g" - # readability-identifier-naming.ConstexprFunctionCase: camelBack - # readability-identifier-naming.ConstexprMethodCase: camelBack - # readability-identifier-naming.ClassMethodCase: camelBack - # readability-identifier-naming.ClassMemberCase: camelBack - # readability-identifier-naming.ClassConstantCase: UPPER_CASE - # readability-identifier-naming.ClassConstantPrefix: "k" - # readability-identifier-naming.StaticConstantCase: UPPER_CASE - # readability-identifier-naming.StaticConstantPrefix: "k" - # readability-identifier-naming.StaticVariableCase: UPPER_CASE - # readability-identifier-naming.StaticVariablePrefix: "k" - # readability-identifier-naming.ConstexprVariableCase: UPPER_CASE - # readability-identifier-naming.ConstexprVariablePrefix: "k" - # readability-identifier-naming.LocalConstantCase: camelBack - # readability-identifier-naming.LocalVariableCase: camelBack - # readability-identifier-naming.TemplateParameterCase: CamelCase - # readability-identifier-naming.ParameterCase: camelBack - # readability-identifier-naming.FunctionCase: camelBack - # readability-identifier-naming.MemberCase: camelBack - # readability-identifier-naming.PrivateMemberSuffix: _ - # readability-identifier-naming.ProtectedMemberSuffix: _ - # readability-identifier-naming.PublicMemberSuffix: "" - # readability-identifier-naming.FunctionIgnoredRegexp: ".*tag_invoke.*" + readability-identifier-naming.MacroDefinitionCase: UPPER_CASE + readability-identifier-naming.ClassCase: CamelCase + readability-identifier-naming.StructCase: CamelCase + readability-identifier-naming.UnionCase: CamelCase + readability-identifier-naming.EnumCase: CamelCase + readability-identifier-naming.EnumConstantCase: CamelCase + readability-identifier-naming.ScopedEnumConstantCase: CamelCase + readability-identifier-naming.GlobalConstantCase: UPPER_CASE + readability-identifier-naming.GlobalConstantPrefix: "k" + readability-identifier-naming.GlobalVariableCase: CamelCase + readability-identifier-naming.GlobalVariablePrefix: "g" + readability-identifier-naming.ConstexprFunctionCase: camelBack + readability-identifier-naming.ConstexprMethodCase: camelBack + readability-identifier-naming.ClassMethodCase: camelBack + readability-identifier-naming.ClassMemberCase: camelBack + readability-identifier-naming.ClassConstantCase: UPPER_CASE + readability-identifier-naming.ClassConstantPrefix: "k" + readability-identifier-naming.StaticConstantCase: UPPER_CASE + readability-identifier-naming.StaticConstantPrefix: "k" + readability-identifier-naming.StaticVariableCase: UPPER_CASE + readability-identifier-naming.StaticVariablePrefix: "k" + readability-identifier-naming.ConstexprVariableCase: UPPER_CASE + readability-identifier-naming.ConstexprVariablePrefix: "k" + readability-identifier-naming.LocalConstantCase: camelBack + readability-identifier-naming.LocalVariableCase: camelBack + readability-identifier-naming.TemplateParameterCase: CamelCase + readability-identifier-naming.ParameterCase: camelBack + readability-identifier-naming.FunctionCase: camelBack + readability-identifier-naming.MemberCase: camelBack + readability-identifier-naming.PrivateMemberSuffix: _ + readability-identifier-naming.ProtectedMemberSuffix: _ + readability-identifier-naming.PublicMemberSuffix: "" + readability-identifier-naming.GlobalFunctionIgnoredRegexp: "^(to_string|hash_append|tuple_hash)$" bugprone-unsafe-functions.ReportMoreUnsafeFunctions: true bugprone-unused-return-value.CheckedReturnTypes: ::std::error_code;::std::error_condition;::std::errc misc-include-cleaner.IgnoreHeaders: ".*/(detail|impl)/.*;.*fwd\\.h(pp)?;time.h;stdlib.h;sqlite3.h;netinet/in\\.h;sys/resource\\.h;sys/sysinfo\\.h;linux/sysinfo\\.h;__chrono/.*;bits/.*;_abort\\.h;boost/uuid/uuid_hash.hpp;boost/beast/core/flat_buffer\\.hpp;boost/beast/http/field\\.hpp;boost/beast/http/dynamic_body\\.hpp;boost/beast/http/message\\.hpp;boost/beast/http/read\\.hpp;boost/beast/http/write\\.hpp;openssl/obj_mac\\.h" -# -HeaderFilterRegex: '^.*/(test|xrpl|xrpld)/.*\.(h|hpp)$' +HeaderFilterRegex: '^.*/(test|xrpl|xrpld)/.*\.(h|hpp|ipp)$' ExcludeHeaderFilterRegex: '^.*/protocol_autogen/.*\.(h|hpp)$' WarningsAsErrors: "*" diff --git a/.git-blame-ignore-revs b/.git-blame-ignore-revs index 0cf704b051..d61cab7e03 100644 --- a/.git-blame-ignore-revs +++ b/.git-blame-ignore-revs @@ -5,6 +5,8 @@ # This file is sorted in reverse chronological order, with the most recent commits at the top. # The commits listed here are ignored by git blame, which is useful for formatting-only commits that would otherwise obscure the history of changes to a file. +# refactor: Enable clang-tidy `readability-identifier-naming` check (#6571) +8995564ed6b9e453e144bb663303072a3c1ba305 # refactor: Enable remaining clang-tidy `cppcoreguidelines` checks (#6538) 72f4cb097f626b08b02fc3efcb4aa11cb2e7adb8 # refactor: Rename system name from 'ripple' to 'xrpld' (#6347) diff --git a/.github/actions/print-env/action.yml b/.github/actions/print-env/action.yml deleted file mode 100644 index 3527ca6f02..0000000000 --- a/.github/actions/print-env/action.yml +++ /dev/null @@ -1,43 +0,0 @@ -name: Print build environment -description: "Print environment and some tooling versions" - -runs: - using: composite - steps: - - name: Check configuration (Windows) - if: ${{ runner.os == 'Windows' }} - shell: bash - run: | - echo 'Checking environment variables.' - set - - - name: Check configuration (Linux and macOS) - if: ${{ runner.os == 'Linux' || runner.os == 'macOS' }} - shell: bash - run: | - echo 'Checking path.' - echo ${PATH} | tr ':' '\n' - - echo 'Checking environment variables.' - env | sort - - echo 'Checking compiler version.' - ${{ runner.os == 'Linux' && '${CC}' || 'clang' }} --version - - echo 'Checking Ninja version.' - ninja --version - - echo 'Checking nproc version.' - nproc --version - - - name: Check configuration (all) - shell: bash - run: | - echo 'Checking Ccache version.' - ccache --version - - echo 'Checking CMake version.' - cmake --version - - echo 'Checking Conan version.' - conan --version diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 66e319e0e7..0e6b840fe7 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -33,17 +33,6 @@ updates: prefix: "ci: [DEPENDABOT] " target-branch: develop - - package-ecosystem: github-actions - directory: .github/actions/print-env/ - schedule: - interval: weekly - day: monday - time: "04:00" - timezone: Etc/GMT - commit-message: - prefix: "ci: [DEPENDABOT] " - target-branch: develop - - package-ecosystem: github-actions directory: .github/actions/setup-conan/ schedule: diff --git a/.github/scripts/levelization/results/ordering.txt b/.github/scripts/levelization/results/ordering.txt index d2a1894585..c2000d1768 100644 --- a/.github/scripts/levelization/results/ordering.txt +++ b/.github/scripts/levelization/results/ordering.txt @@ -188,10 +188,16 @@ test.toplevel > xrpl.json test.unit_test > xrpl.basics test.unit_test > xrpl.protocol tests.libxrpl > xrpl.basics +tests.libxrpl > xrpl.core tests.libxrpl > xrpl.json +tests.libxrpl > xrpl.ledger tests.libxrpl > xrpl.net +tests.libxrpl > xrpl.nodestore tests.libxrpl > xrpl.protocol tests.libxrpl > xrpl.protocol_autogen +tests.libxrpl > xrpl.server +tests.libxrpl > xrpl.shamap +tests.libxrpl > xrpl.tx xrpl.conditions > xrpl.basics xrpl.conditions > xrpl.protocol xrpl.core > xrpl.basics diff --git a/.github/scripts/rename/config.sh b/.github/scripts/rename/config.sh index b7aff82cdf..a27d0823a9 100755 --- a/.github/scripts/rename/config.sh +++ b/.github/scripts/rename/config.sh @@ -62,7 +62,7 @@ ${SED_COMMAND} -i 's@ripple/@xrpld/@g' src/test/core/Config_test.cpp ${SED_COMMAND} -i 's/Rippled/File/g' src/test/core/Config_test.cpp # Restore the old config file name in the code that maintains support for now. -${SED_COMMAND} -i 's/configLegacyName = "xrpld.cfg"/configLegacyName = "rippled.cfg"/g' src/xrpld/core/detail/Config.cpp +${SED_COMMAND} -i 's/kCONFIG_LEGACY_NAME = "xrpld.cfg"/kCONFIG_LEGACY_NAME = "rippled.cfg"/g' src/xrpld/core/detail/Config.cpp # Restore an URL. ${SED_COMMAND} -i 's/connect-your-xrpld-to-the-xrp-test-net.html/connect-your-rippled-to-the-xrp-test-net.html/g' cfg/xrpld-example.cfg diff --git a/.github/scripts/rename/docs.sh b/.github/scripts/rename/docs.sh index 59cc5665bf..23ff76bc05 100755 --- a/.github/scripts/rename/docs.sh +++ b/.github/scripts/rename/docs.sh @@ -90,7 +90,7 @@ ${SED_COMMAND} -i 's/www.ripple.com/www.xrpl.org/g' src/test/protocol/Seed_test. # Restore specific changes. ${SED_COMMAND} -i 's@b5efcc/src/xrpld@b5efcc/src/ripple@' include/xrpl/protocol/README.md ${SED_COMMAND} -i 's/dbPrefix_ = "xrpldb"/dbPrefix_ = "rippledb"/' src/xrpld/app/misc/SHAMapStoreImp.h # cspell: disable-line -${SED_COMMAND} -i 's/configLegacyName = "xrpld.cfg"/configLegacyName = "rippled.cfg"/' src/xrpld/core/detail/Config.cpp +${SED_COMMAND} -i 's/kCONFIG_LEGACY_NAME = "xrpld.cfg"/kCONFIG_LEGACY_NAME = "rippled.cfg"/' src/xrpld/core/detail/Config.cpp popd echo "Renaming complete." diff --git a/.github/scripts/strategy-matrix/generate.py b/.github/scripts/strategy-matrix/generate.py index 4784142b7b..dec41a2610 100755 --- a/.github/scripts/strategy-matrix/generate.py +++ b/.github/scripts/strategy-matrix/generate.py @@ -51,20 +51,21 @@ def generate_strategy_matrix(all: bool, config: Config) -> list: # Only generate a subset of configurations in PRs. if not all: # Debian: - # - Bookworm using GCC 13: Release on linux/amd64, set the reference - # fee to 500. - # - Bookworm using GCC 15: Debug on linux/amd64, enable code - # coverage (which will be done below). + # - Bookworm using GCC 13: Debug on linux/amd64, set the reference + # fee to 500 and enable code coverage (which will be done below). + # - Bookworm using GCC 15: Debug on linux/amd64, enable Address and + # UB sanitizers (which will be done below). # - Bookworm using Clang 16: Debug on linux/amd64, enable voidstar. # - Bookworm using Clang 17: Release on linux/amd64, set the # reference fee to 1000. - # - Bookworm using Clang 20: Debug on linux/amd64. + # - Bookworm using Clang 20: Debug on linux/amd64, enable Address + # and UB sanitizers (which will be done below). if os["distro_name"] == "debian": skip = True if os["distro_version"] == "bookworm": if ( f"{os['compiler_name']}-{os['compiler_version']}" == "gcc-13" - and build_type == "Release" + and build_type == "Debug" and architecture["platform"] == "linux/amd64" ): cmake_args = f"-DUNIT_TEST_REFERENCE_FEE=500 {cmake_args}" @@ -193,11 +194,11 @@ def generate_strategy_matrix(all: bool, config: Config) -> list: ): continue - # Enable code coverage for Debian Bookworm using GCC 15 in Debug on - # linux/amd64 + # Enable code coverage for Debian Bookworm using GCC 13 in Debug on + # linux/amd64. if ( f"{os['distro_name']}-{os['distro_version']}" == "debian-bookworm" - and f"{os['compiler_name']}-{os['compiler_version']}" == "gcc-15" + and f"{os['compiler_name']}-{os['compiler_version']}" == "gcc-13" and build_type == "Debug" and architecture["platform"] == "linux/amd64" ): @@ -234,23 +235,39 @@ def generate_strategy_matrix(all: bool, config: Config) -> list: # Add the configuration to the list, with the most unique fields first, # so that they are easier to identify in the GitHub Actions UI, as long # names get truncated. - # Add Address and Thread (both coupled with UB) sanitizers for specific bookworm distros. + # Add Address and UB sanitizers as separate configurations for specific + # bookworm distros. Thread sanitizer is currently disabled (see below). # GCC-Asan xrpld-embedded tests are failing because of https://github.com/google/sanitizers/issues/856 - if ( - os["distro_version"] == "bookworm" - and f"{os['compiler_name']}-{os['compiler_version']}" == "clang-20" - ): - # Add ASAN + UBSAN configuration. + if os[ + "distro_version" + ] == "bookworm" and f"{os['compiler_name']}-{os['compiler_version']}" in [ + "gcc-15", + "clang-20", + ]: + # Add ASAN configuration. configurations.append( { - "config_name": config_name + "-asan-ubsan", + "config_name": config_name + "-asan", "cmake_args": cmake_args, "cmake_target": cmake_target, "build_only": build_only, "build_type": build_type, "os": os, "architecture": architecture, - "sanitizers": "address,undefinedbehavior", + "sanitizers": "address", + } + ) + # Add UBSAN configuration. + configurations.append( + { + "config_name": config_name + "-ubsan", + "cmake_args": cmake_args, + "cmake_target": cmake_target, + "build_only": build_only, + "build_type": build_type, + "os": os, + "architecture": architecture, + "sanitizers": "undefinedbehavior", } ) # TSAN is deactivated due to seg faults with latest compilers. diff --git a/.github/workflows/on-pr.yml b/.github/workflows/on-pr.yml index 28299a1264..d95f3a6c00 100644 --- a/.github/workflows/on-pr.yml +++ b/.github/workflows/on-pr.yml @@ -58,15 +58,12 @@ jobs: # Keep the paths below in sync with those in `on-trigger.yml`. .github/actions/build-deps/** - .github/actions/build-test/** .github/actions/generate-version/** .github/actions/setup-conan/** .github/scripts/strategy-matrix/** - .github/workflows/reusable-build.yml .github/workflows/reusable-build-test-config.yml .github/workflows/reusable-build-test.yml .github/workflows/reusable-clang-tidy.yml - .github/workflows/reusable-clang-tidy-files.yml .github/workflows/reusable-strategy-matrix.yml .github/workflows/reusable-test.yml .github/workflows/reusable-upload-recipe.yml @@ -176,4 +173,4 @@ jobs: runs-on: ubuntu-latest steps: - name: Fail - run: false + run: exit 1 diff --git a/.github/workflows/on-trigger.yml b/.github/workflows/on-trigger.yml index 5856c67bd3..11d98bffb7 100644 --- a/.github/workflows/on-trigger.yml +++ b/.github/workflows/on-trigger.yml @@ -15,15 +15,12 @@ on: # Keep the paths below in sync with those in `on-pr.yml`. - ".github/actions/build-deps/**" - - ".github/actions/build-test/**" - ".github/actions/generate-version/**" - ".github/actions/setup-conan/**" - ".github/scripts/strategy-matrix/**" - - ".github/workflows/reusable-build.yml" - ".github/workflows/reusable-build-test-config.yml" - ".github/workflows/reusable-build-test.yml" - ".github/workflows/reusable-clang-tidy.yml" - - ".github/workflows/reusable-clang-tidy-files.yml" - ".github/workflows/reusable-strategy-matrix.yml" - ".github/workflows/reusable-test.yml" - ".github/workflows/reusable-upload-recipe.yml" diff --git a/.github/workflows/pre-commit.yml b/.github/workflows/pre-commit.yml index 89255f0e47..2f15b82266 100644 --- a/.github/workflows/pre-commit.yml +++ b/.github/workflows/pre-commit.yml @@ -14,7 +14,7 @@ on: jobs: # Call the workflow in the XRPLF/actions repo that runs the pre-commit hooks. run-hooks: - uses: XRPLF/actions/.github/workflows/pre-commit.yml@9307df762265e15c745ddcdb38a581c989f7f349 + uses: XRPLF/actions/.github/workflows/pre-commit.yml@5e942d61bf32f7557a7c159cfac4712a687b3e3a with: runs_on: ubuntu-latest container: '{ "image": "ghcr.io/xrplf/ci/tools-rippled-pre-commit:sha-41ec7c1" }' diff --git a/.github/workflows/reusable-build-test-config.yml b/.github/workflows/reusable-build-test-config.yml index c2c862d73f..528934f6de 100644 --- a/.github/workflows/reusable-build-test-config.yml +++ b/.github/workflows/reusable-build-test-config.yml @@ -116,7 +116,7 @@ jobs: run: echo "CCACHE_LOGFILE=${{ runner.temp }}/ccache.log" >> "${GITHUB_ENV}" - name: Print build environment - uses: ./.github/actions/print-env + uses: XRPLF/actions/print-build-env@59dec886e4afb05a1724443af08baccbc045b574 - name: Get number of processors uses: XRPLF/actions/get-nproc@cf0433aa74563aead044a1e395610c96d65a37cf @@ -143,7 +143,6 @@ jobs: working-directory: ${{ env.BUILD_DIR }} env: BUILD_TYPE: ${{ inputs.build_type }} - SANITIZERS: ${{ inputs.sanitizers }} CMAKE_ARGS: ${{ inputs.cmake_args }} run: | cmake \ diff --git a/.github/workflows/reusable-clang-tidy-files.yml b/.github/workflows/reusable-clang-tidy-files.yml deleted file mode 100644 index 9b99f418b1..0000000000 --- a/.github/workflows/reusable-clang-tidy-files.yml +++ /dev/null @@ -1,175 +0,0 @@ -name: Run clang-tidy on files - -on: - workflow_call: - inputs: - files: - description: "List of files to check (empty means check all files)" - type: string - default: "" - create_issue_on_failure: - description: "Whether to create an issue if the check failed" - type: boolean - default: false - -defaults: - run: - shell: bash - -env: - # Conan installs the generators in the build/generators directory, see the - # layout() method in conanfile.py. We then run CMake from the build directory. - BUILD_DIR: build - BUILD_TYPE: Release - -jobs: - run-clang-tidy: - name: Run clang tidy - runs-on: ["self-hosted", "Linux", "X64", "heavy"] - container: "ghcr.io/xrplf/ci/debian-trixie:clang-21-sha-53033a2" - permissions: - issues: write - contents: read - steps: - - name: Checkout repository - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - - - name: Prepare runner - uses: XRPLF/actions/prepare-runner@90f11ee655d1687824fb8793db770477d52afbab - with: - enable_ccache: false - - - name: Print build environment - uses: ./.github/actions/print-env - - - name: Get number of processors - uses: XRPLF/actions/get-nproc@cf0433aa74563aead044a1e395610c96d65a37cf - id: nproc - - - name: Setup Conan - uses: ./.github/actions/setup-conan - - - name: Build dependencies - uses: ./.github/actions/build-deps - with: - build_nproc: ${{ steps.nproc.outputs.nproc }} - build_type: ${{ env.BUILD_TYPE }} - log_verbosity: verbose - - - name: Configure CMake - working-directory: ${{ env.BUILD_DIR }} - run: | - cmake \ - -G 'Ninja' \ - -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake \ - -DCMAKE_BUILD_TYPE="${BUILD_TYPE}" \ - -Dtests=ON \ - -Dwerr=ON \ - -Dxrpld=ON \ - .. - - # clang-tidy needs headers generated from proto files - - name: Build libxrpl.libpb - working-directory: ${{ env.BUILD_DIR }} - run: | - ninja -j ${{ steps.nproc.outputs.nproc }} xrpl.libpb - - - name: Run clang tidy - id: run_clang_tidy - continue-on-error: true - env: - TARGETS: ${{ inputs.files != '' && inputs.files || 'src tests' }} - run: | - run-clang-tidy -j ${{ steps.nproc.outputs.nproc }} -p "${BUILD_DIR}" -quiet -fix -allow-no-checks ${TARGETS} 2>&1 | tee clang-tidy-output.txt - - - name: Upload clang-tidy output - if: ${{ github.event.repository.visibility == 'public' && steps.run_clang_tidy.outcome != 'success' }} - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 - with: - name: clang-tidy-results - path: clang-tidy-output.txt - retention-days: 30 - - - name: Generate git diff - if: ${{ steps.run_clang_tidy.outcome != 'success' }} - run: | - git diff | tee clang-tidy-git-diff.txt - - - name: Upload clang-tidy diff output - if: ${{ github.event.repository.visibility == 'public' && steps.run_clang_tidy.outcome != 'success' }} - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 - with: - name: clang-tidy-git-diff - path: clang-tidy-git-diff.txt - retention-days: 30 - - - name: Create an issue - if: ${{ steps.run_clang_tidy.outcome != 'success' && inputs.create_issue_on_failure }} - id: create_issue - shell: bash - env: - GH_TOKEN: ${{ github.token }} - run: | - # Prepare issue body with clang-tidy output - cat > issue.md < filtered-output.txt || true - - # If filtered output is empty, use original (might be a different error format) - if [ ! -s filtered-output.txt ]; then - cp clang-tidy-output.txt filtered-output.txt - fi - - # Truncate if too large - head -c 60000 filtered-output.txt >> issue.md - if [ "$(wc -c < filtered-output.txt)" -gt 60000 ]; then - echo "" >> issue.md - echo "... (output truncated, see artifacts for full output)" >> issue.md - fi - - rm filtered-output.txt - else - echo "No output file found" >> issue.md - fi - - cat >> issue.md < create_issue.log - - created_issue="$(sed 's|.*/||' create_issue.log)" - echo "created_issue=$created_issue" >> $GITHUB_OUTPUT - echo "Created issue #$created_issue" - - rm -f create_issue.log issue.md clang-tidy-output.txt - - - name: Fail the workflow if clang-tidy failed - if: ${{ steps.run_clang_tidy.outcome != 'success' }} - run: | - echo "Clang-tidy check failed!" - exit 1 diff --git a/.github/workflows/reusable-clang-tidy.yml b/.github/workflows/reusable-clang-tidy.yml index 7a8bf6de57..10e36f957f 100644 --- a/.github/workflows/reusable-clang-tidy.yml +++ b/.github/workflows/reusable-clang-tidy.yml @@ -1,4 +1,4 @@ -name: Clang-tidy check +name: Run clang-tidy on files on: workflow_call: @@ -16,40 +16,175 @@ defaults: run: shell: bash +env: + BUILD_DIR: build + BUILD_TYPE: Debug # Debug so that ASSERTS and such participate in clang-tidy check + + OUTPUT_FILE: clang-tidy-output.txt + DIFF_FILE: clang-tidy-git-diff.txt + ISSUE_FILE: clang-tidy-issue.md + jobs: determine-files: - name: Determine files to check if: ${{ inputs.check_only_changed }} - runs-on: ubuntu-latest - outputs: - clang_tidy_config_changed: ${{ steps.changed_clang_tidy.outputs.any_changed }} - any_cpp_changed: ${{ steps.changed_files.outputs.any_changed }} - all_changed_files: ${{ steps.changed_files.outputs.all_changed_files }} + permissions: + contents: read + uses: XRPLF/actions/.github/workflows/determine-tidy-files.yml@12f5dbc98a2260259a66970e57fa4d26fb7f285c + + run-clang-tidy: + name: Run clang tidy + needs: [determine-files] + if: ${{ always() && !cancelled() && (!inputs.check_only_changed || needs.determine-files.outputs.cpp_changed_files != '' || needs.determine-files.outputs.clang_tidy_config_changed == 'true') }} + runs-on: ["self-hosted", "Linux", "X64", "heavy"] + container: "ghcr.io/xrplf/ci/debian-trixie:clang-21-sha-53033a2" + permissions: + contents: read + issues: write steps: - name: Checkout repository uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - - name: Get changed C++ files - id: changed_files - uses: tj-actions/changed-files@9426d40962ed5378910ee2e21d5f8c6fcbf2dd96 # v47.0.6 + - name: Prepare runner + uses: XRPLF/actions/prepare-runner@90f11ee655d1687824fb8793db770477d52afbab with: - files: | - **/*.cpp - **/*.h - **/*.ipp - separator: " " + enable_ccache: false - - name: Get changed clang-tidy configuration - id: changed_clang_tidy - uses: tj-actions/changed-files@9426d40962ed5378910ee2e21d5f8c6fcbf2dd96 # v47.0.6 + - name: Print build environment + uses: XRPLF/actions/print-build-env@59dec886e4afb05a1724443af08baccbc045b574 + + - name: Get number of processors + uses: XRPLF/actions/get-nproc@cf0433aa74563aead044a1e395610c96d65a37cf + id: nproc + + - name: Setup Conan + uses: ./.github/actions/setup-conan + + - name: Build dependencies + uses: ./.github/actions/build-deps with: - files: | - .clang-tidy + build_nproc: ${{ steps.nproc.outputs.nproc }} + build_type: ${{ env.BUILD_TYPE }} + log_verbosity: verbose - run-clang-tidy: - needs: [determine-files] - if: ${{ always() && !cancelled() && (!inputs.check_only_changed || needs.determine-files.outputs.any_cpp_changed == 'true' || needs.determine-files.outputs.clang_tidy_config_changed == 'true') }} - uses: ./.github/workflows/reusable-clang-tidy-files.yml - with: - files: ${{ (needs.determine-files.outputs.clang_tidy_config_changed != 'true' && inputs.check_only_changed) && needs.determine-files.outputs.all_changed_files || '' }} - create_issue_on_failure: ${{ inputs.create_issue_on_failure }} + - name: Configure CMake + working-directory: ${{ env.BUILD_DIR }} + run: | + cmake \ + -G 'Ninja' \ + -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake \ + -DCMAKE_BUILD_TYPE="${BUILD_TYPE}" \ + -Dtests=ON \ + -Dwerr=ON \ + -Dxrpld=ON \ + .. + + # clang-tidy needs headers generated from proto files + - name: Build libxrpl.libpb + working-directory: ${{ env.BUILD_DIR }} + run: | + ninja -j ${{ steps.nproc.outputs.nproc }} xrpl.libpb + + - name: Run clang tidy + id: run_clang_tidy + continue-on-error: true + env: + TARGETS: ${{ (needs.determine-files.outputs.clang_tidy_config_changed != 'true' && inputs.check_only_changed) && needs.determine-files.outputs.cpp_changed_files || 'src tests' }} + run: | + set -o pipefail + run-clang-tidy -j ${{ steps.nproc.outputs.nproc }} -p "${BUILD_DIR}" -quiet -fix -allow-no-checks ${TARGETS} 2>&1 | tee "${OUTPUT_FILE}" + + - name: Print errors + if: ${{ steps.run_clang_tidy.outcome != 'success' }} + run: | + sed '/error\||/!d' "${OUTPUT_FILE}" + + - name: Upload clang-tidy output + if: ${{ github.event.repository.visibility == 'public' && steps.run_clang_tidy.outcome != 'success' }} + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + path: ${{ env.OUTPUT_FILE }} + archive: false + retention-days: 30 + + - name: Check for changes + id: files_changed + continue-on-error: true + run: | + git diff --exit-code + + - name: Fix style + if: ${{ steps.files_changed.outcome != 'success' }} + run: | + pre-commit run --all-files || true + + - name: Generate git diff + if: ${{ steps.files_changed.outcome != 'success' }} + run: | + git diff | tee "${DIFF_FILE}" + + - name: Upload clang-tidy diff output + if: ${{ github.event.repository.visibility == 'public' && steps.files_changed.outcome != 'success' }} + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + path: ${{ env.DIFF_FILE }} + archive: false + retention-days: 30 + + - name: Write issue header + if: ${{ steps.run_clang_tidy.outcome != 'success' }} + run: | + cat > "${ISSUE_FILE}" < filtered-output.txt || true + + # If filtered output is empty, use original (might be a different error format) + if [ ! -s filtered-output.txt ]; then + cp "${OUTPUT_FILE}" filtered-output.txt + fi + + # Truncate if too large + head -c 60000 filtered-output.txt >> "${ISSUE_FILE}" + if [ "$(wc -c < filtered-output.txt)" -gt 60000 ]; then + echo "" >> "${ISSUE_FILE}" + echo "... (output truncated, see artifacts for full output)" >> "${ISSUE_FILE}" + fi + + rm filtered-output.txt + else + echo "No output file found" >> "${ISSUE_FILE}" + fi + + - name: Append issue footer + if: ${{ steps.run_clang_tidy.outcome != 'success' }} + run: | + cat >> "${ISSUE_FILE}" < # @@ -1270,7 +1270,7 @@ # default. Don't change this without understanding the consequences. # # Example: -# owner_reserve = 2000000 # 2 XRP +# owner_reserve = 200000 # 0.2 XRP # #------------------------------------------------------------------------------- # diff --git a/cmake/XrplInterface.cmake b/cmake/XrplInterface.cmake index 7add613f5a..825cb63310 100644 --- a/cmake/XrplInterface.cmake +++ b/cmake/XrplInterface.cmake @@ -23,7 +23,6 @@ target_compile_definitions( BOOST_FILESYSTEM_NO_DEPRECATED > $<$>: - BOOST_COROUTINES2_NO_DEPRECATION_WARNING BOOST_BEAST_ALLOW_DEPRECATED BOOST_FILESYSTEM_DEPRECATED > diff --git a/cmake/XrplSanitizers.cmake b/cmake/XrplSanitizers.cmake index f9630f6856..64f1841bfb 100644 --- a/cmake/XrplSanitizers.cmake +++ b/cmake/XrplSanitizers.cmake @@ -1,138 +1,33 @@ #[===================================================================[ - Configure sanitizers based on environment variables. + Apply sanitizer flags built by the Conan profile. - This module reads the following environment variables: - - SANITIZERS: The sanitizers to enable. Possible values: - - "address" - - "address,undefinedbehavior" - - "thread" - - "thread,undefinedbehavior" - - "undefinedbehavior" + Parsing, validation, and flag construction are performed in conan/profiles/sanitizers. + This module reads the following CMake variables injected by the Conan toolchain via extra_variables: - The compiler type and platform are detected in CompilationEnv.cmake. - The sanitizer compile options are applied to the 'common' interface library - which is linked to all targets in the project. + - SANITIZERS: The active sanitizers (e.g. "address,undefinedbehavior"). + - SANITIZERS_COMPILER_FLAGS: Space-separated compiler flags. + - SANITIZERS_LINKER_FLAGS: Space-separated linker flags. - Internal flag variables set by this module: - - - SANITIZER_TYPES: List of sanitizer types to enable (e.g., "address", - "thread", "undefined"). And two more flags for undefined behavior sanitizer (e.g., "float-divide-by-zero", "unsigned-integer-overflow"). - This list is joined with commas and passed to -fsanitize=. - - - SANITIZERS_COMPILE_FLAGS: Compiler flags for sanitizer instrumentation. - Includes: - * -fno-omit-frame-pointer: Preserves frame pointers for stack traces - * -O1: Minimum optimization for reasonable performance - * -fsanitize=: Enables sanitizer instrumentation - * -fsanitize-ignorelist=: (Clang only) Compile-time ignorelist - * -mcmodel=large/medium: (GCC only) Code model for large binaries - * -Wno-stringop-overflow: (GCC only) Suppresses false positive warnings - * -Wno-tsan: (For GCC TSAN combination only) Suppresses atomic_thread_fence warnings - - - SANITIZERS_LINK_FLAGS: Linker flags for sanitizer runtime libraries. - Includes: - * -fsanitize=: Links sanitizer runtime libraries - * -mcmodel=large/medium: (GCC only) Matches compile-time code model - - - SANITIZERS_RELOCATION_FLAGS: (GCC only) Code model flags for linking. - Used to handle large instrumented binaries on x86_64: - * -mcmodel=large: For AddressSanitizer (prevents relocation errors) - * -mcmodel=medium: For ThreadSanitizer (large model is incompatible) + The flags are applied to the 'common' interface library which is linked to all targets in the project. #]===================================================================] +include_guard(GLOBAL) include(CompilationEnv) -# Read environment variable -set(SANITIZERS "") -if(DEFINED ENV{SANITIZERS}) - set(SANITIZERS "$ENV{SANITIZERS}") -endif() - -# Set SANITIZERS_ENABLED flag for use in other modules -if(SANITIZERS MATCHES "address|thread|undefinedbehavior") - set(SANITIZERS_ENABLED TRUE) -else() +if(NOT DEFINED SANITIZERS) set(SANITIZERS_ENABLED FALSE) return() endif() +set(SANITIZERS_ENABLED TRUE) -# Sanitizers are not supported on Windows/MSVC -if(is_msvc) - message( - FATAL_ERROR - "Sanitizers are not supported on Windows/MSVC. " - "Please unset the SANITIZERS environment variable." - ) -endif() +message(STATUS "=== Configuring Sanitizers ===") +message(STATUS " SANITIZERS: ${SANITIZERS}") +message(STATUS " Compile flags: ${SANITIZERS_COMPILER_FLAGS}") +message(STATUS " Link flags: ${SANITIZERS_LINKER_FLAGS}") -message(STATUS "Configuring sanitizers: ${SANITIZERS}") - -# Parse SANITIZERS value to determine which sanitizers to enable -set(enable_asan FALSE) -set(enable_tsan FALSE) -set(enable_ubsan FALSE) - -# Normalize SANITIZERS into a list -set(san_list "${SANITIZERS}") -string(REPLACE "," ";" san_list "${san_list}") -separate_arguments(san_list) - -foreach(san IN LISTS san_list) - if(san STREQUAL "address") - set(enable_asan TRUE) - elseif(san STREQUAL "thread") - set(enable_tsan TRUE) - elseif(san STREQUAL "undefinedbehavior") - set(enable_ubsan TRUE) - else() - message( - FATAL_ERROR - "Unsupported sanitizer type: ${san}" - "Supported: address, thread, undefinedbehavior and their combinations." - ) - endif() -endforeach() - -# Validate sanitizer compatibility -if(enable_asan AND enable_tsan) - message( - FATAL_ERROR - "AddressSanitizer and ThreadSanitizer are incompatible and cannot be enabled simultaneously. " - "Use 'address' or 'thread', optionally with 'undefinedbehavior'." - ) -endif() - -# Frame pointer is required for meaningful stack traces. Sanitizers recommend minimum of -O1 for reasonable performance -set(SANITIZERS_COMPILE_FLAGS "-fno-omit-frame-pointer" "-O1") - -# Build the sanitizer flags list -set(SANITIZER_TYPES) - -if(enable_asan) - list(APPEND SANITIZER_TYPES "address") -elseif(enable_tsan) - list(APPEND SANITIZER_TYPES "thread") -endif() - -if(enable_ubsan) - # UB sanitizer flags - list(APPEND SANITIZER_TYPES "undefined" "float-divide-by-zero") - if(is_clang) - # Clang supports additional UB checks. More info here - # https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html - list(APPEND SANITIZER_TYPES "unsigned-integer-overflow") - endif() -endif() - -# Configure code model for GCC on amd64 Use large code model for ASAN to avoid relocation errors Use medium code model -# for TSAN (large is not compatible with TSAN) -set(SANITIZERS_RELOCATION_FLAGS) - -# Compiler-specific configuration +# GCC with sanitizers is incompatible with mold, gold, and lld linkers. +# Namely, the instrumented binary exceeds size limits imposed by these linkers. if(is_gcc) - # Disable mold, gold and lld linkers for GCC with sanitizers Use default linker (bfd/ld) which is more lenient with - # mixed code models This is needed since the size of instrumented binary exceeds the limits set by mold, lld and - # gold linkers set(use_mold OFF CACHE BOOL "Use mold linker" FORCE) set(use_gold OFF CACHE BOOL "Use gold linker" FORCE) set(use_lld OFF CACHE BOOL "Use lld linker" FORCE) @@ -140,80 +35,62 @@ if(is_gcc) STATUS " Disabled mold, gold, and lld linkers for GCC with sanitizers" ) - - # Suppress false positive warnings in GCC with stringop-overflow - list(APPEND SANITIZERS_COMPILE_FLAGS "-Wno-stringop-overflow") - - if(is_amd64 AND enable_asan) - message(STATUS " Using large code model (-mcmodel=large)") - list(APPEND SANITIZERS_COMPILE_FLAGS "-mcmodel=large") - list(APPEND SANITIZERS_RELOCATION_FLAGS "-mcmodel=large") - elseif(enable_tsan) - # GCC doesn't support atomic_thread_fence with tsan. Suppress warnings. - list(APPEND SANITIZERS_COMPILE_FLAGS "-Wno-tsan") - message(STATUS " Using medium code model (-mcmodel=medium)") - list(APPEND SANITIZERS_COMPILE_FLAGS "-mcmodel=medium") - list(APPEND SANITIZERS_RELOCATION_FLAGS "-mcmodel=medium") - endif() - - # Join sanitizer flags with commas for -fsanitize option - list(JOIN SANITIZER_TYPES "," SANITIZER_TYPES_STR) - - # Add sanitizer to compile and link flags - list(APPEND SANITIZERS_COMPILE_FLAGS "-fsanitize=${SANITIZER_TYPES_STR}") - set(SANITIZERS_LINK_FLAGS - "${SANITIZERS_RELOCATION_FLAGS}" - "-fsanitize=${SANITIZER_TYPES_STR}" - ) -elseif(is_clang) - # Add ignorelist for Clang (GCC doesn't support this) Use CMAKE_SOURCE_DIR to get the path to the ignorelist - set(IGNORELIST_PATH - "${CMAKE_SOURCE_DIR}/sanitizers/suppressions/sanitizer-ignorelist.txt" - ) - if(NOT EXISTS "${IGNORELIST_PATH}") - message( - FATAL_ERROR - "Sanitizer ignorelist not found: ${IGNORELIST_PATH}" - ) - endif() - - list( - APPEND SANITIZERS_COMPILE_FLAGS - "-fsanitize-ignorelist=${IGNORELIST_PATH}" - ) - message(STATUS " Using sanitizer ignorelist: ${IGNORELIST_PATH}") - - # Join sanitizer flags with commas for -fsanitize option - list(JOIN SANITIZER_TYPES "," SANITIZER_TYPES_STR) - - # Add sanitizer to compile and link flags - list(APPEND SANITIZERS_COMPILE_FLAGS "-fsanitize=${SANITIZER_TYPES_STR}") - set(SANITIZERS_LINK_FLAGS "-fsanitize=${SANITIZER_TYPES_STR}") endif() -message(STATUS " Compile flags: ${SANITIZERS_COMPILE_FLAGS}") -message(STATUS " Link flags: ${SANITIZERS_LINK_FLAGS}") +# Flags arrive as space-separated strings; split into CMake lists before use +separate_arguments( + sanitizers_compiler_flags + UNIX_COMMAND + "${SANITIZERS_COMPILER_FLAGS}" +) +separate_arguments( + sanitizers_linker_flags + UNIX_COMMAND + "${SANITIZERS_LINKER_FLAGS}" +) -# Apply the sanitizer flags to the 'common' interface library This is the same library used by XrplCompiler.cmake target_compile_options( common INTERFACE - $<$:${SANITIZERS_COMPILE_FLAGS}> - $<$:${SANITIZERS_COMPILE_FLAGS}> + $<$:${sanitizers_compiler_flags}> + $<$:${sanitizers_compiler_flags}> ) +target_link_options(common INTERFACE ${sanitizers_linker_flags}) -# Apply linker flags -target_link_options(common INTERFACE ${SANITIZERS_LINK_FLAGS}) +# This module appends -fsanitize-ignorelist= for Clang builds. +# The ignorelist path contains CMAKE_SOURCE_DIR, so it must be set here, rather than in the Conan profile. +# GCC does not support -fsanitize-ignorelist. +if(is_clang) + set(ignorelist_path + "${CMAKE_SOURCE_DIR}/sanitizers/suppressions/sanitizer-ignorelist.txt" + ) + if(NOT EXISTS "${ignorelist_path}") + message( + FATAL_ERROR + "Sanitizer ignorelist not found: ${ignorelist_path}" + ) + endif() + target_compile_options( + common + INTERFACE + $<$:-fsanitize-ignorelist=${ignorelist_path}> + $<$:-fsanitize-ignorelist=${ignorelist_path}> + ) + message(STATUS " Ignorelist: ${ignorelist_path}") +endif() # Define SANITIZERS macro for BuildInfo.cpp set(sanitizers_list) -if(enable_asan) +if(SANITIZERS MATCHES "address") + set(enable_asan ON) list(APPEND sanitizers_list "ASAN") endif() -if(enable_tsan) +if(SANITIZERS MATCHES "thread") + set(enable_tsan ON) list(APPEND sanitizers_list "TSAN") endif() -if(enable_ubsan) +if(SANITIZERS MATCHES "undefinedbehavior") + set(enable_ubsan ON) list(APPEND sanitizers_list "UBSAN") endif() diff --git a/cmake/scripts/codegen/macro_parser_common.py b/cmake/scripts/codegen/macro_parser_common.py index eae4df39db..40cd9f50c5 100644 --- a/cmake/scripts/codegen/macro_parser_common.py +++ b/cmake/scripts/codegen/macro_parser_common.py @@ -152,10 +152,10 @@ def parse_sfields_macro(sfields_path): def create_field_list_parser(): """Create a pyparsing parser for field lists like '({...})'.""" - # A field identifier (e.g., sfDestination, soeREQUIRED, soeMPTSupported) + # A field identifier (e.g., sfDestination, SoeRequired, SoeMptSupported) field_identifier = pp.Word(pp.alphas + "_", pp.alphanums + "_") - # A single field definition: {sfName, soeREQUIRED, ...} + # A single field definition: {sfName, SoeRequired, ...} # Allow optional trailing comma inside the braces field_def = ( pp.Suppress("{") @@ -185,8 +185,8 @@ def parse_field_list(fields_str): Args: fields_str: A string like '({ - {sfDestination, soeREQUIRED}, - {sfAmount, soeREQUIRED, soeMPTSupported} + {sfDestination, SoeRequired}, + {sfAmount, SoeRequired, SoeMptSupported} })' Returns: @@ -205,7 +205,7 @@ def parse_field_list(fields_str): field_name = field_parts[0] requirement = field_parts[1] flags = list(field_parts[2:]) if len(field_parts) > 2 else [] - supports_mpt = "soeMPTSupported" in flags + supports_mpt = "SoeMptSupported" in flags fields.append( { diff --git a/cmake/scripts/codegen/templates/LedgerEntry.h.mako b/cmake/scripts/codegen/templates/LedgerEntry.h.mako index fdb55a973a..31029cd311 100644 --- a/cmake/scripts/codegen/templates/LedgerEntry.h.mako +++ b/cmake/scripts/codegen/templates/LedgerEntry.h.mako @@ -52,13 +52,13 @@ public: % if field.get('mpt_support'): * MPT Support: ${field['mpt_support']} % endif -% if field['requirement'] == 'soeREQUIRED': +% if field['requirement'] == 'SoeRequired': * @return The field value. % else: * @return The field value, or std::nullopt if not present. % endif */ -% if field['requirement'] == 'soeREQUIRED': +% if field['requirement'] == 'SoeRequired': [[nodiscard]] ${field['typeData']['return_type']} get${field['name'][2:]}() const @@ -94,13 +94,13 @@ public: * MPT Support: ${field['mpt_support']} % endif * @note This is an untyped field (${field.get('cppType', 'unknown')}). -% if field['requirement'] == 'soeREQUIRED': +% if field['requirement'] == 'SoeRequired': * @return The field value. % else: * @return The field value, or std::nullopt if not present. % endif */ -% if field['requirement'] == 'soeREQUIRED': +% if field['requirement'] == 'SoeRequired': [[nodiscard]] ${field['typeData']['return_type']} get${field['name'][2:]}() const @@ -133,13 +133,13 @@ public: }; <% - required_fields = [f for f in fields if f['requirement'] == 'soeREQUIRED'] + required_fields = [f for f in fields if f['requirement'] == 'SoeRequired'] %>\ /** * @brief Builder for ${name} ledger entries. * * Provides a fluent interface for constructing ledger entries with method chaining. - * Uses Json::Value internally for flexible ledger entry construction. + * Uses STObject internally for flexible ledger entry construction. * Inherits common field setters from LedgerEntryBuilderBase. */ class ${name}Builder : public LedgerEntryBuilderBase<${name}Builder> diff --git a/cmake/scripts/codegen/templates/LedgerEntryTests.cpp.mako b/cmake/scripts/codegen/templates/LedgerEntryTests.cpp.mako index 35ce57f17b..011cf933dd 100644 --- a/cmake/scripts/codegen/templates/LedgerEntryTests.cpp.mako +++ b/cmake/scripts/codegen/templates/LedgerEntryTests.cpp.mako @@ -1,7 +1,7 @@ // Auto-generated unit tests for ledger entry ${name} <% - required_fields = [f for f in fields if f["requirement"] == "soeREQUIRED"] - optional_fields = [f for f in fields if f["requirement"] != "soeREQUIRED"] + required_fields = [f for f in fields if f["requirement"] == "SoeRequired"] + optional_fields = [f for f in fields if f["requirement"] != "SoeRequired"] def canonical_expr(field): return f"canonical_{field['stiSuffix']}()" diff --git a/cmake/scripts/codegen/templates/Transaction.h.mako b/cmake/scripts/codegen/templates/Transaction.h.mako index 62c51a5c97..d3b303d9d6 100644 --- a/cmake/scripts/codegen/templates/Transaction.h.mako +++ b/cmake/scripts/codegen/templates/Transaction.h.mako @@ -54,13 +54,13 @@ public: % if field.get('supports_mpt'): * @note This field supports MPT (Multi-Purpose Token) amounts. % endif -% if field['requirement'] == 'soeREQUIRED': +% if field['requirement'] == 'SoeRequired': * @return The field value. % else: * @return The field value, or std::nullopt if not present. % endif */ -% if field['requirement'] == 'soeREQUIRED': +% if field['requirement'] == 'SoeRequired': [[nodiscard]] ${field['typeData']['return_type']} get${field['name'][2:]}() const @@ -97,13 +97,13 @@ public: * @note This field supports MPT (Multi-Purpose Token) amounts. % endif * @note This is an untyped field. -% if field['requirement'] == 'soeREQUIRED': +% if field['requirement'] == 'SoeRequired': * @return The field value. % else: * @return The field value, or std::nullopt if not present. % endif */ -% if field['requirement'] == 'soeREQUIRED': +% if field['requirement'] == 'SoeRequired': [[nodiscard]] ${field['typeData']['return_type']} get${field['name'][2:]}() const @@ -136,13 +136,13 @@ public: }; <% - required_fields = [f for f in fields if f['requirement'] == 'soeREQUIRED'] + required_fields = [f for f in fields if f['requirement'] == 'SoeRequired'] %>\ /** * @brief Builder for ${name} transactions. * * Provides a fluent interface for constructing transactions with method chaining. - * Uses Json::Value internally for flexible transaction construction. + * Uses STObject internally for flexible transaction construction. * Inherits common field setters from TransactionBuilderBase. */ class ${name}Builder : public TransactionBuilderBase<${name}Builder> diff --git a/cmake/scripts/codegen/templates/TransactionTests.cpp.mako b/cmake/scripts/codegen/templates/TransactionTests.cpp.mako index b6bd5cd831..14ceaffd09 100644 --- a/cmake/scripts/codegen/templates/TransactionTests.cpp.mako +++ b/cmake/scripts/codegen/templates/TransactionTests.cpp.mako @@ -1,7 +1,7 @@ // Auto-generated unit tests for transaction ${name} <% - required_fields = [f for f in fields if f["requirement"] == "soeREQUIRED"] - optional_fields = [f for f in fields if f["requirement"] != "soeREQUIRED"] + required_fields = [f for f in fields if f["requirement"] == "SoeRequired"] + optional_fields = [f for f in fields if f["requirement"] != "SoeRequired"] def canonical_expr(field): return f"canonical_{field['stiSuffix']}()" @@ -33,7 +33,7 @@ TEST(Transactions${name}Tests, BuilderSettersRoundTrip) { // Generate a deterministic keypair for signing auto const [publicKey, secretKey] = - generateKeyPair(KeyType::secp256k1, generateSeed("test${name}")); + generateKeyPair(KeyType::Secp256k1, generateSeed("test${name}")); // Common transaction fields auto const accountValue = calcAccountID(publicKey); @@ -101,7 +101,7 @@ TEST(Transactions${name}Tests, BuilderFromStTxRoundTrip) { // Generate a deterministic keypair for signing auto const [publicKey, secretKey] = - generateKeyPair(KeyType::secp256k1, generateSeed("test${name}FromTx")); + generateKeyPair(KeyType::Secp256k1, generateSeed("test${name}FromTx")); // Common transaction fields auto const accountValue = calcAccountID(publicKey); @@ -168,7 +168,7 @@ TEST(Transactions${name}Tests, WrapperThrowsOnWrongTxType) { // Build a valid transaction of a different type auto const [pk, sk] = - generateKeyPair(KeyType::secp256k1, generateSeed("testWrongType")); + generateKeyPair(KeyType::Secp256k1, generateSeed("testWrongType")); auto const account = calcAccountID(pk); % if wrong_tx_include == "AccountSet": @@ -186,7 +186,7 @@ TEST(Transactions${name}Tests, BuilderThrowsOnWrongTxType) { // Build a valid transaction of a different type auto const [pk, sk] = - generateKeyPair(KeyType::secp256k1, generateSeed("testWrongTypeBuilder")); + generateKeyPair(KeyType::Secp256k1, generateSeed("testWrongTypeBuilder")); auto const account = calcAccountID(pk); % if wrong_tx_include == "AccountSet": @@ -205,7 +205,7 @@ TEST(Transactions${name}Tests, OptionalFieldsReturnNullopt) { // Generate a deterministic keypair for signing auto const [publicKey, secretKey] = - generateKeyPair(KeyType::secp256k1, generateSeed("test${name}Nullopt")); + generateKeyPair(KeyType::Secp256k1, generateSeed("test${name}Nullopt")); // Common transaction fields auto const accountValue = calcAccountID(publicKey); diff --git a/conan/profiles/ci b/conan/profiles/ci index c4c0898ad5..ae93187026 100644 --- a/conan/profiles/ci +++ b/conan/profiles/ci @@ -1 +1 @@ - include(sanitizers) +include(sanitizers) diff --git a/conan/profiles/sanitizers b/conan/profiles/sanitizers index 6d37425f43..4a05fda734 100644 --- a/conan/profiles/sanitizers +++ b/conan/profiles/sanitizers @@ -1,86 +1,122 @@ include(default) {% set compiler, version, compiler_exe = detect_api.detect_default_compiler() %} +{% set arch = detect_api.detect_arch() %} {% set sanitizers = os.getenv("SANITIZERS") %} -[conf] -{% if sanitizers %} - {% if compiler == "gcc" %} - {% if "address" in sanitizers or "thread" in sanitizers or "undefinedbehavior" in sanitizers %} - {% set sanitizer_list = [] %} - {% set defines = [] %} - {% set model_code = "" %} - {% set extra_cxxflags = ["-fno-omit-frame-pointer", "-O1", "-Wno-stringop-overflow"] %} +{% if not sanitizers %} +{# Sanitizers not configured; no additional settings needed #} +{% else %} - {% if "address" in sanitizers %} - {% set _ = sanitizer_list.append("address") %} - {% set model_code = "-mcmodel=large" %} - {% set _ = defines.append("BOOST_USE_ASAN")%} - {% set _ = defines.append("BOOST_USE_UCONTEXT")%} - {% elif "thread" in sanitizers %} - {% set _ = sanitizer_list.append("thread") %} - {% set model_code = "-mcmodel=medium" %} - {% set _ = extra_cxxflags.append("-Wno-tsan") %} - {% set _ = defines.append("BOOST_USE_TSAN")%} - {% set _ = defines.append("BOOST_USE_UCONTEXT")%} - {% endif %} +{% if compiler == "msvc" %} + {{ "Sanitizers are not supported on Windows/MSVC. Please unset the SANITIZERS environment variable." }} +{% endif %} - {% if "undefinedbehavior" in sanitizers %} - {% set _ = sanitizer_list.append("undefined") %} - {% set _ = sanitizer_list.append("float-divide-by-zero") %} - {% endif %} +{% set known_sanitizers = ["address", "thread", "undefinedbehavior"] %} +{% set provided_sanitizers = [] %} +{% for san in sanitizers.split(",") %} + {% set san = san.strip() %} + {% if san not in known_sanitizers %} + {{ "Unknown sanitizer in SANITIZERS: " ~ san }} + {% endif %} + {% set _ = provided_sanitizers.append(san) %} +{% endfor %} - {% set sanitizer_flags = "-fsanitize=" ~ ",".join(sanitizer_list) ~ " " ~ model_code %} +{% set enable_asan = "address" in provided_sanitizers %} +{% set enable_tsan = "thread" in provided_sanitizers %} +{% set enable_ubsan = "undefinedbehavior" in provided_sanitizers %} - tools.build:cxxflags+=['{{sanitizer_flags}} {{" ".join(extra_cxxflags)}}'] - tools.build:sharedlinkflags+=['{{sanitizer_flags}}'] - tools.build:exelinkflags+=['{{sanitizer_flags}}'] - tools.build:defines+={{defines}} - {% endif %} - {% elif compiler == "apple-clang" or compiler == "clang" %} - {% if "address" in sanitizers or "thread" in sanitizers or "undefinedbehavior" in sanitizers %} - {% set sanitizer_list = [] %} - {% set defines = [] %} - {% set extra_cxxflags = ["-fno-omit-frame-pointer", "-O1"] %} +{% if enable_asan and enable_tsan %} + {{ "AddressSanitizer and ThreadSanitizer are incompatible and cannot be enabled simultaneously." }} +{% endif %} - {% if "address" in sanitizers %} - {% set _ = sanitizer_list.append("address") %} - {% set _ = defines.append("BOOST_USE_ASAN")%} - {% set _ = defines.append("BOOST_USE_UCONTEXT")%} - {% elif "thread" in sanitizers %} - {% set _ = sanitizer_list.append("thread") %} - {% set _ = defines.append("BOOST_USE_TSAN")%} - {% set _ = defines.append("BOOST_USE_UCONTEXT")%} - {% endif %} +{% set sanitizer_types = [] %} +{% set defines = [] %} - {% if "undefinedbehavior" in sanitizers %} - {% set _ = sanitizer_list.append("undefined") %} - {% set _ = sanitizer_list.append("float-divide-by-zero") %} - {% set _ = sanitizer_list.append("unsigned-integer-overflow") %} - {% endif %} +{% if enable_asan %} + {% set _ = sanitizer_types.append("address") %} + {% set _ = defines.append("BOOST_USE_ASAN") %} + {% set _ = defines.append("BOOST_USE_UCONTEXT") %} +{% elif enable_tsan %} + {% set _ = sanitizer_types.append("thread") %} + {% set _ = defines.append("BOOST_USE_TSAN") %} + {% set _ = defines.append("BOOST_USE_UCONTEXT") %} +{% endif %} - {% set sanitizer_flags = "-fsanitize=" ~ ",".join(sanitizer_list) %} - - tools.build:cxxflags+=['{{sanitizer_flags}} {{" ".join(extra_cxxflags)}}'] - tools.build:sharedlinkflags+=['{{sanitizer_flags}}'] - tools.build:exelinkflags+=['{{sanitizer_flags}}'] - tools.build:defines+={{defines}} - {% endif %} +{% if enable_ubsan %} + {% set _ = sanitizer_types.append("undefined") %} + {% set _ = sanitizer_types.append("float-divide-by-zero") %} + {# Clang supports additional UB checks beyond the GCC baseline #} + {% if compiler == "clang" or compiler == "apple-clang" %} + {% set _ = sanitizer_types.append("unsigned-integer-overflow") %} {% endif %} {% endif %} +{# Frame pointer required for meaningful stack traces; -O1 for reasonable performance #} +{% set compile_flags = ["-fno-omit-frame-pointer", "-O1"] %} + +{% if compiler == "gcc" %} + {# Suppress false positive warnings with GCC #} + {% set _ = compile_flags.append("-Wno-stringop-overflow") %} + + {% set relocation_flags = [] %} + + {% if arch == "x86_64" and enable_asan %} + {# Large code model prevents relocation errors in instrumented ASAN binaries #} + {% set _ = compile_flags.append("-mcmodel=large") %} + {% set _ = relocation_flags.append("-mcmodel=large") %} + {% elif enable_tsan %} + {# GCC doesn't support atomic_thread_fence with TSAN; suppress warnings #} + {% set _ = compile_flags.append("-Wno-tsan") %} + {% if arch == "x86_64" %} + {# Medium code model for TSAN; large is incompatible #} + {% set _ = compile_flags.append("-mcmodel=medium") %} + {% set _ = relocation_flags.append("-mcmodel=medium") %} + {% endif %} + {% endif %} + + {% set fsanitize = "-fsanitize=" ~ ",".join(sanitizer_types) %} + {% set _ = compile_flags.append(fsanitize) %} + {% set _ = relocation_flags.append(fsanitize) %} + + {% set sanitizer_compiler_flags = " ".join(compile_flags) %} + {% set sanitizer_linker_flags = " ".join(relocation_flags) %} +{% elif compiler == "clang" or compiler == "apple-clang" %} + {% set fsanitize = "-fsanitize=" ~ ",".join(sanitizer_types) %} + {% set _ = compile_flags.append(fsanitize) %} + + {% set sanitizer_compiler_flags = " ".join(compile_flags) %} + {% set sanitizer_linker_flags = fsanitize %} +{% endif %} + +[conf] +tools.build:defines+={{defines}} +tools.build:cxxflags+=['{{sanitizer_compiler_flags}}'] +tools.build:sharedlinkflags+=['{{sanitizer_linker_flags}}'] +tools.build:exelinkflags+=['{{sanitizer_linker_flags}}'] + tools.info.package_id:confs+=["tools.build:cxxflags", "tools.build:exelinkflags", "tools.build:sharedlinkflags", "tools.build:defines"] +# &: means "apply only to the consumer/root package" +&:tools.cmake.cmaketoolchain:extra_variables={"SANITIZERS": "{{sanitizers}}", "SANITIZERS_COMPILER_FLAGS": "{{sanitizer_compiler_flags}}", "SANITIZERS_LINKER_FLAGS": "{{sanitizer_linker_flags}}"} + [options] -{% if sanitizers %} - {% if "address" in sanitizers %} - # Build Boost.Context with ucontext backend (not fcontext) so that - # ASAN fiber-switching annotations (__sanitizer_start/finish_switch_fiber) - # are compiled into the library. fcontext (assembly) has no ASAN support. - # define=BOOST_USE_ASAN=1 is critical: it must be defined when building - # Boost.Context itself so the ucontext backend compiles in the ASAN annotations. - boost/*:extra_b2_flags=context-impl=ucontext address-sanitizer=on define=BOOST_USE_ASAN=1 - boost/*:without_context=False - # Boost stacktrace fails to build with some sanitizers - boost/*:without_stacktrace=True - {% endif %} +{% if enable_asan %} + # Build Boost.Context with ucontext backend (not fcontext) so that + # ASAN fiber-switching annotations (__sanitizer_start/finish_switch_fiber) + # are compiled into the library. fcontext (assembly) has no ASAN support. + # define=BOOST_USE_ASAN=1 is critical: it must be defined when building + # Boost.Context itself so the ucontext backend compiles in the ASAN annotations. + boost/*:extra_b2_flags=context-impl=ucontext address-sanitizer=on define=BOOST_USE_ASAN=1 + boost/*:without_context=False + # Boost stacktrace fails to build with some sanitizers + boost/*:without_stacktrace=True +{% elif enable_tsan %} + # Build Boost.Context with ucontext backend for TSAN. fcontext (assembly) + # has no TSAN annotations, so without this the BOOST_USE_TSAN/BOOST_USE_UCONTEXT + # defines in [conf] would be ineffective. + boost/*:extra_b2_flags=context-impl=ucontext thread-sanitizer=on define=BOOST_USE_TSAN=1 + boost/*:without_context=False + boost/*:without_stacktrace=True +{% endif %} + {% endif %} diff --git a/conanfile.py b/conanfile.py index 4949516bfe..66de2e2d57 100644 --- a/conanfile.py +++ b/conanfile.py @@ -129,12 +129,6 @@ class Xrpl(ConanFile): if self.settings.compiler in ["clang", "gcc"]: self.options["boost"].without_cobalt = True - # Check if environment variable exists - if "SANITIZERS" in os.environ: - sanitizers = os.environ["SANITIZERS"] - if "address" in sanitizers.lower(): - self.default_options["fPIC"] = False - def requirements(self): self.requires("boost/1.90.0", force=True, transitive_headers=True) self.requires("date/3.0.4", transitive_headers=True) diff --git a/cspell.config.yaml b/cspell.config.yaml index 028f02191e..8b8589a8c9 100644 --- a/cspell.config.yaml +++ b/cspell.config.yaml @@ -71,6 +71,7 @@ words: - citardauq - clawback - clawbacks + - cmaketoolchain - coeffs - coldwallet - compr @@ -114,6 +115,7 @@ words: - gcovr - ghead - Gnutella + - godexsoft - gpgcheck - gpgkey - hotwallet @@ -151,6 +153,7 @@ words: - lseq - lsmf - ltype + - mathbunnyru - mcmodel - MEMORYSTATUSEX - MPTAMM diff --git a/docs/build/sanitizers.md b/docs/build/sanitizers.md index 7677775a3d..6e7284fd06 100644 --- a/docs/build/sanitizers.md +++ b/docs/build/sanitizers.md @@ -1,15 +1,17 @@ # Sanitizer Configuration for Xrpld -This document explains how to properly configure and run sanitizers (AddressSanitizer, undefinedbehaviorSanitizer, ThreadSanitizer) with the xrpld project. +This document explains how to properly configure and run sanitizers (`AddressSanitizer`, `UndefinedBehaviorSanitizer`, `ThreadSanitizer`) with the xrpld project. Corresponding suppression files are located in the `sanitizers/suppressions` directory. +> [!CAUTION] +> Do not mix Address and Thread sanitizers - they are incompatible. +> Also, we don't yet support MSVC sanitizers, so this is only for Clang/GCC builds. + - [Sanitizer Configuration for Xrpld](#sanitizer-configuration-for-xrpld) - [Building with Sanitizers](#building-with-sanitizers) - [Summary](#summary) - [Build steps:](#build-steps) - [Install dependencies](#install-dependencies) - - [Call CMake](#call-cmake) - - [Build](#build) - [Running Tests with Sanitizers](#running-tests-with-sanitizers) - [AddressSanitizer (ASAN)](#addresssanitizer-asan) - [ThreadSanitizer (TSan)](#threadsanitizer-tsan) @@ -33,9 +35,13 @@ Corresponding suppression files are located in the `sanitizers/suppressions` dir Follow the same instructions as mentioned in [BUILD.md](../../BUILD.md) but with the following changes: 1. Make sure you have a clean build directory. -2. Set the `SANITIZERS` environment variable before calling conan install and cmake. Only set it once. Make sure both conan and cmake read the same values. +2. Set the `SANITIZERS` environment variable before calling `conan install`. Only set it once. Example: `export SANITIZERS=address,undefinedbehavior` -3. Optionally use `--profile:all sanitizers` with Conan to build dependencies with sanitizer instrumentation. [!NOTE]Building with sanitizer-instrumented dependencies is slower but produces fewer false positives. +3. Use `--profile:all sanitizers` with Conan to build dependencies with sanitizer instrumentation. + + > [!NOTE] + > Building with sanitizer-instrumented dependencies is slower but produces fewer false positives. + 4. Set `ASAN_OPTIONS`, `LSAN_OPTIONS`, `UBSAN_OPTIONS` and `TSAN_OPTIONS` environment variables to configure sanitizer behavior when running executables. [More details below](#running-tests-with-sanitizers). --- @@ -51,36 +57,13 @@ cd .build #### Install dependencies -The `SANITIZERS` environment variable is used by both Conan and CMake. +The `SANITIZERS` environment variable is used during `conan install` command. ```bash -export SANITIZERS=address,undefinedbehavior -# Standard build (without instrumenting dependencies) -conan install .. --output-folder . --build missing --settings build_type=Debug - -# Or with sanitizer-instrumented dependencies (takes longer but fewer false positives) -conan install .. --output-folder . --profile:all sanitizers --build missing --settings build_type=Debug +SANITIZERS=address,undefinedbehavior conan install .. --output-folder . --build missing --settings build_type=Debug --profile:all sanitizers ``` -[!CAUTION] -Do not mix Address and Thread sanitizers - they are incompatible. - -Since you already set the `SANITIZERS` environment variable when running Conan, same values will be read for the next part. - -#### Call CMake - -```bash -cmake .. -G Ninja \ - -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake \ - -DCMAKE_BUILD_TYPE=Debug \ - -Dtests=ON -Dxrpld=ON -``` - -#### Build - -```bash -cmake --build . --parallel 4 -``` +Proceed with the rest of the build instructions as mentioned in [BUILD.md](../../BUILD.md). ## Running Tests with Sanitizers @@ -98,7 +81,7 @@ export LSAN_OPTIONS="include=sanitizers/suppressions/runtime-lsan-options.txt:su **Why `detect_container_overflow=0`?** -- Boost intrusive containers (used in `aged_unordered_container`) trigger false positives +- Boost intrusive containers (used in `AgedUnorderedContainer`) trigger false positives - Boost context switching (used in `Workers.cpp`) confuses ASAN's stack tracking - Since we usually don't build Boost (because we don't want to instrument Boost and detect issues in Boost code) with ASAN but use Boost containers in ASAN instrumented xrpld code, it generates false positives. - Building dependencies with ASAN instrumentation reduces false positives. But we don't want to instrument dependencies like Boost with ASAN because it is slow (to compile as well as run tests) and not necessary. diff --git a/docs/consensus.md b/docs/consensus.md index 0da23b708a..2850cf784e 100644 --- a/docs/consensus.md +++ b/docs/consensus.md @@ -477,7 +477,7 @@ struct Ledger // The parent ledger's close time NetClock::time_point parentCloseTime() const; - Json::Value getJson() const; + json::Value getJson() const; //... implementation specific }; diff --git a/include/xrpl/basics/BasicConfig.h b/include/xrpl/basics/BasicConfig.h index 0639fb6c4d..5a82f7d081 100644 --- a/include/xrpl/basics/BasicConfig.h +++ b/include/xrpl/basics/BasicConfig.h @@ -27,7 +27,7 @@ private: std::unordered_map lookup_; std::vector lines_; std::vector values_; - bool had_trailing_comments_ = false; + bool hadTrailingComments_ = false; using const_iterator = decltype(lookup_)::const_iterator; @@ -133,7 +133,7 @@ public: /// Returns a value if present, else another value. template [[nodiscard]] T - value_or(std::string const& name, T const& other) const + valueOr(std::string const& name, T const& other) const { auto const v = get(name); return v.has_value() ? *v : other; @@ -142,9 +142,9 @@ public: // indicates if trailing comments were seen // during the appending of any lines/values [[nodiscard]] bool - had_trailing_comments() const + hadTrailingComments() const { - return had_trailing_comments_; + return hadTrailingComments_; } friend std::ostream& @@ -273,9 +273,9 @@ public: // indicates if trailing comments were seen // in any loaded Sections [[nodiscard]] bool - had_trailing_comments() const + hadTrailingComments() const { - return std::ranges::any_of(map_, [](auto s) { return s.second.had_trailing_comments(); }); + return std::ranges::any_of(map_, [](auto s) { return s.second.hadTrailingComments(); }); } protected: @@ -294,17 +294,17 @@ template bool set(T& target, std::string const& name, Section const& section) { - bool found_and_valid = false; + bool foundAndValid = false; try { auto const val = section.get(name); - if ((found_and_valid = val.has_value())) + if ((foundAndValid = val.has_value())) target = *val; } catch (boost::bad_lexical_cast const&) // NOLINT(bugprone-empty-catch) { } - return found_and_valid; + return foundAndValid; } /** Set a value from a configuration Section @@ -316,10 +316,10 @@ template bool set(T& target, T const& defaultValue, std::string const& name, Section const& section) { - bool const found_and_valid = set(target, name, section); - if (!found_and_valid) + bool const foundAndValid = set(target, name, section); + if (!foundAndValid) target = defaultValue; - return found_and_valid; + return foundAndValid; } /** Retrieve a key/value pair from a section. @@ -333,7 +333,7 @@ get(Section const& section, std::string const& name, T const& defaultValue = T{} { try { - return section.value_or(name, defaultValue); + return section.valueOr(name, defaultValue); } catch (boost::bad_lexical_cast const&) // NOLINT(bugprone-empty-catch) { @@ -358,17 +358,17 @@ get(Section const& section, std::string const& name, char const* defaultValue) template bool -get_if_exists(Section const& section, std::string const& name, T& v) +getIfExists(Section const& section, std::string const& name, T& v) { return set(v, name, section); } template <> inline bool -get_if_exists(Section const& section, std::string const& name, bool& v) +getIfExists(Section const& section, std::string const& name, bool& v) { int intVal = 0; - auto stat = get_if_exists(section, name, intVal); + auto stat = getIfExists(section, name, intVal); if (stat) v = bool(intVal); return stat; diff --git a/include/xrpl/basics/CountedObject.h b/include/xrpl/basics/CountedObject.h index 379fd49837..275894673e 100644 --- a/include/xrpl/basics/CountedObject.h +++ b/include/xrpl/basics/CountedObject.h @@ -38,11 +38,11 @@ public: do { - head = instance.m_head.load(); + head = instance.head_.load(); next_ = head; - } while (instance.m_head.exchange(this) != head); + } while (instance.head_.exchange(this) != head); - ++instance.m_count; + ++instance.count_; } ~Counter() noexcept = default; @@ -88,8 +88,8 @@ private: ~CountedObjects() noexcept = default; private: - std::atomic m_count; - std::atomic m_head; + std::atomic count_; + std::atomic head_; }; //------------------------------------------------------------------------------ @@ -108,8 +108,8 @@ private: static auto& getCounter() noexcept { - static CountedObjects::Counter c{beast::type_name()}; - return c; + static CountedObjects::Counter kC{beast::typeName()}; + return kC; } CountedObject() noexcept diff --git a/include/xrpl/basics/DecayingSample.h b/include/xrpl/basics/DecayingSample.h index d4c7388046..d1861ebc4a 100644 --- a/include/xrpl/basics/DecayingSample.h +++ b/include/xrpl/basics/DecayingSample.h @@ -20,7 +20,7 @@ public: /** @param now Start time of DecayingSample. */ - explicit DecayingSample(time_point now) : m_value(value_type()), m_when(now) + explicit DecayingSample(time_point now) : value_(value_type()), when_(now) { } @@ -31,8 +31,8 @@ public: add(value_type value, time_point now) { decay(now); - m_value += value; - return m_value / Window; + value_ += value; + return value_ / Window; } /** Retrieve the current value in normalized units. @@ -42,7 +42,7 @@ public: value(time_point now) { decay(now); - return m_value / Window; + return value_ / Window; } private: @@ -50,36 +50,38 @@ private: void decay(time_point now) { - if (now == m_when) + if (now == when_) return; - if (m_value != value_type()) + if (value_ != value_type()) { std::size_t elapsed = - std::chrono::duration_cast(now - m_when).count(); + std::chrono::duration_cast(now - when_).count(); // A span larger than four times the window decays the // value to an insignificant amount so just reset it. // if (elapsed > 4 * Window) { - m_value = value_type(); + value_ = value_type(); } else { - while ((elapsed--) != 0u) - m_value -= (m_value + Window - 1) / Window; + for (; elapsed > 0; --elapsed) + { + value_ -= (value_ + Window - 1) / Window; + } } } - m_when = now; + when_ = now; } // Current value in exponential units - value_type m_value; + value_type value_; // Last time the aging function was applied - time_point m_when; + time_point when_; }; //------------------------------------------------------------------------------ diff --git a/include/xrpl/basics/Expected.h b/include/xrpl/basics/Expected.h index f4d8e5019a..74a0e76eef 100644 --- a/include/xrpl/basics/Expected.h +++ b/include/xrpl/basics/Expected.h @@ -16,9 +16,9 @@ namespace xrpl { */ // Exception thrown by an invalid access to Expected. -struct bad_expected_access : public std::runtime_error +struct BadExpectedAccess : public std::runtime_error { - bad_expected_access() : runtime_error("bad expected access") + BadExpectedAccess() : runtime_error("bad expected access") { } }; @@ -26,30 +26,33 @@ struct bad_expected_access : public std::runtime_error namespace detail { // Custom policy for Expected. Always throw on an invalid access. -struct throw_policy : public boost::outcome_v2::policy::base +struct ThrowPolicy : public boost::outcome_v2::policy::base { template static constexpr void + // NOLINTNEXTLINE(readability-identifier-naming) wide_value_check(Impl&& self) { if (!base::_has_value(std::forward(self))) - Throw(); + Throw(); } template static constexpr void + // NOLINTNEXTLINE(readability-identifier-naming) wide_error_check(Impl&& self) { if (!base::_has_error(std::forward(self))) - Throw(); + Throw(); } template static constexpr void + // NOLINTNEXTLINE(readability-identifier-naming) wide_exception_check(Impl&& self) { if (!base::_has_exception(std::forward(self))) - Throw(); + Throw(); } }; @@ -107,9 +110,9 @@ Unexpected(E (&)[N]) -> Unexpected; // Definition of Expected. All of the machinery comes from boost::result. template -class [[nodiscard]] Expected : private boost::outcome_v2::result +class [[nodiscard]] Expected : private boost::outcome_v2::result { - using Base = boost::outcome_v2::result; + using Base = boost::outcome_v2::result; public: template @@ -126,6 +129,7 @@ public: } [[nodiscard]] constexpr bool + // NOLINTNEXTLINE(readability-identifier-naming) has_value() const { return Base::has_value(); @@ -193,9 +197,9 @@ public: // (without a value) or the reason for the failure. template class [[nodiscard]] -Expected : private boost::outcome_v2::result +Expected : private boost::outcome_v2::result { - using Base = boost::outcome_v2::result; + using Base = boost::outcome_v2::result; public: // The default constructor makes a successful Expected. diff --git a/include/xrpl/basics/IntrusivePointer.h b/include/xrpl/basics/IntrusivePointer.h index 019e71a727..06ee3e5bb4 100644 --- a/include/xrpl/basics/IntrusivePointer.h +++ b/include/xrpl/basics/IntrusivePointer.h @@ -164,11 +164,11 @@ public: /** Return the strong count */ [[nodiscard]] std::size_t - use_count() const; + useCount() const; template friend SharedIntrusive - make_SharedIntrusive(Args&&... args); + makeSharedIntrusive(Args&&... args); template friend class SharedIntrusive; @@ -364,7 +364,7 @@ public: * return 0 */ [[nodiscard]] std::size_t - use_count() const; + useCount() const; /** Return true if there is a non-zero strong count. */ [[nodiscard]] bool @@ -406,8 +406,8 @@ private: // pointer. The low bit must be masked to zero when converting back to a // pointer. If the low bit is '1', this is a weak pointer. std::uintptr_t tp_{0}; - static constexpr std::uintptr_t tagMask = 1; - static constexpr std::uintptr_t ptrMask = ~tagMask; + static constexpr std::uintptr_t kTAG_MASK = 1; + static constexpr std::uintptr_t kPTR_MASK = ~kTAG_MASK; private: /** Return the raw pointer held by this object. @@ -415,7 +415,7 @@ private: [[nodiscard]] T* unsafeGetRawPtr() const; - enum class RefStrength { strong, weak }; + enum class RefStrength { Strong, Weak }; /** Set the raw pointer and tag bit directly. */ void @@ -442,7 +442,7 @@ private: */ template SharedIntrusive -make_SharedIntrusive(Args&&... args) +makeSharedIntrusive(Args&&... args) { auto p = new TT(std::forward(args)...); @@ -469,21 +469,21 @@ using SharedWeakUnionPtr = SharedWeakUnion; template SharedPtr -make_shared(A&&... args) +makeShared(A&&... args) { - return make_SharedIntrusive(std::forward(args)...); + return makeSharedIntrusive(std::forward(args)...); } template SharedPtr -static_pointer_cast(TT const& v) +staticPointerCast(TT const& v) { return SharedPtr(StaticCastTagSharedIntrusive{}, v); } template SharedPtr -dynamic_pointer_cast(TT const& v) +dynamicPointerCast(TT const& v) { return SharedPtr(DynamicCastTagSharedIntrusive{}, v); } diff --git a/include/xrpl/basics/IntrusivePointer.ipp b/include/xrpl/basics/IntrusivePointer.ipp index 59caf5a931..f229df8b27 100644 --- a/include/xrpl/basics/IntrusivePointer.ipp +++ b/include/xrpl/basics/IntrusivePointer.ipp @@ -43,14 +43,16 @@ SharedIntrusive::SharedIntrusive(SharedIntrusive const& rhs) } template -SharedIntrusive::SharedIntrusive(SharedIntrusive&& rhs) : ptr_{rhs.unsafeExchange(nullptr)} +SharedIntrusive::SharedIntrusive(SharedIntrusive&& rhs) + : ptr_{std::move(rhs).unsafeExchange(nullptr)} { } template template requires std::convertible_to -SharedIntrusive::SharedIntrusive(SharedIntrusive&& rhs) : ptr_{rhs.unsafeExchange(nullptr)} +SharedIntrusive::SharedIntrusive(SharedIntrusive&& rhs) + : ptr_{std::move(rhs).unsafeExchange(nullptr)} { } template @@ -93,7 +95,7 @@ SharedIntrusive::operator=(SharedIntrusive&& rhs) if (this == &rhs) return *this; - unsafeReleaseAndStore(rhs.unsafeExchange(nullptr)); + unsafeReleaseAndStore(std::move(rhs).unsafeExchange(nullptr)); return *this; } @@ -105,7 +107,7 @@ SharedIntrusive::operator=(SharedIntrusive&& rhs) { static_assert(!std::is_same_v, "This overload should not be instantiated for T == TT"); - unsafeReleaseAndStore(rhs.unsafeExchange(nullptr)); + unsafeReleaseAndStore(std::move(rhs).unsafeExchange(nullptr)); return *this; } @@ -157,7 +159,7 @@ SharedIntrusive::SharedIntrusive(StaticCastTagSharedIntrusive, SharedIntrusiv template template SharedIntrusive::SharedIntrusive(StaticCastTagSharedIntrusive, SharedIntrusive&& rhs) - : ptr_{static_cast(rhs.unsafeExchange(nullptr))} + : ptr_{static_cast(std::move(rhs).unsafeExchange(nullptr))} { } @@ -184,8 +186,10 @@ SharedIntrusive::SharedIntrusive(DynamicCastTagSharedIntrusive, SharedIntrusi { ptr_ = dynamic_cast(toSet); if (!ptr_) + { // need to set the pointer back or will leak - rhs.unsafeExchange(toSet); + std::move(rhs).unsafeExchange(toSet); + } } } @@ -226,10 +230,10 @@ SharedIntrusive::get() const template std::size_t -SharedIntrusive::use_count() const +SharedIntrusive::useCount() const { if (auto p = unsafeGetRawPtr()) - return p->use_count(); + return p->useCount(); return 0; } @@ -266,12 +270,12 @@ SharedIntrusive::unsafeReleaseAndStore(T* next) auto action = prev->releaseStrongRef(); switch (action) { - case noop: + case NoOp: break; - case destroy: + case Destroy: delete prev; break; - case partialDestroy: + case PartialDestroy: prev->partialDestructor(); partialDestructorFinished(&prev); // prev is null and may no longer be used @@ -345,7 +349,7 @@ template bool WeakIntrusive::expired() const { - return (!ptr_ || ptr_->expired()); + return ((ptr_ == nullptr) || ptr_->expired()); } template @@ -360,16 +364,16 @@ template void WeakIntrusive::unsafeReleaseNoStore() { - if (!ptr_) + if (ptr_ == nullptr) return; using enum ReleaseWeakRefAction; auto action = ptr_->releaseWeakRef(); switch (action) { - case noop: + case NoOp: break; - case destroy: + case Destroy: delete ptr_; break; } @@ -385,9 +389,13 @@ SharedWeakUnion::SharedWeakUnion(SharedWeakUnion const& rhs) : tp_{rhs.tp_} return; if (rhs.isStrong()) + { p->addStrongRef(); + } else + { p->addWeakRef(); + } } template @@ -398,7 +406,7 @@ SharedWeakUnion::SharedWeakUnion(SharedIntrusive const& rhs) auto p = rhs.unsafeGetRawPtr(); if (p) p->addStrongRef(); - unsafeSetRawPtr(p, RefStrength::strong); + unsafeSetRawPtr(p, RefStrength::Strong); } template @@ -414,8 +422,8 @@ SharedWeakUnion::SharedWeakUnion(SharedIntrusive&& rhs) { auto p = rhs.unsafeGetRawPtr(); if (p) - unsafeSetRawPtr(p, RefStrength::strong); - rhs.unsafeSetRawPtr(nullptr); + unsafeSetRawPtr(p, RefStrength::Strong); + std::move(rhs).unsafeSetRawPtr(nullptr); } template @@ -431,12 +439,12 @@ SharedWeakUnion::operator=(SharedWeakUnion const& rhs) if (rhs.isStrong()) { p->addStrongRef(); - unsafeSetRawPtr(p, RefStrength::strong); + unsafeSetRawPtr(p, RefStrength::Strong); } else { p->addWeakRef(); - unsafeSetRawPtr(p, RefStrength::weak); + unsafeSetRawPtr(p, RefStrength::Weak); } } else @@ -456,7 +464,7 @@ SharedWeakUnion::operator=(SharedIntrusive const& rhs) auto p = rhs.unsafeGetRawPtr(); if (p) p->addStrongRef(); - unsafeSetRawPtr(p, RefStrength::strong); + unsafeSetRawPtr(p, RefStrength::Strong); return *this; } @@ -467,8 +475,8 @@ SharedWeakUnion& SharedWeakUnion::operator=(SharedIntrusive&& rhs) { unsafeReleaseNoStore(); - unsafeSetRawPtr(rhs.unsafeGetRawPtr(), RefStrength::strong); - rhs.unsafeSetRawPtr(nullptr); + unsafeSetRawPtr(rhs.unsafeGetRawPtr(), RefStrength::Strong); + std::move(rhs).unsafeSetRawPtr(nullptr); return *this; } @@ -517,10 +525,10 @@ SharedWeakUnion::get() const template std::size_t -SharedWeakUnion::use_count() const +SharedWeakUnion::useCount() const { if (auto p = get()) - return p->use_count(); + return p->useCount(); return 0; } @@ -559,14 +567,14 @@ template bool SharedWeakUnion::isStrong() const { - return !(tp_ & tagMask); + return (tp_ & kTAG_MASK) == 0u; } template bool SharedWeakUnion::isWeak() const { - return tp_ & tagMask; + return (tp_ & kTAG_MASK) != 0u; } template @@ -581,10 +589,10 @@ SharedWeakUnion::convertToStrong() { [[maybe_unused]] auto action = p->releaseWeakRef(); XRPL_ASSERT( - (action == ReleaseWeakRefAction::noop), + (action == ReleaseWeakRefAction::NoOp), "xrpl::SharedWeakUnion::convertToStrong : " "action is noop"); - unsafeSetRawPtr(p, RefStrength::strong); + unsafeSetRawPtr(p, RefStrength::Strong); return true; } return false; @@ -605,9 +613,9 @@ SharedWeakUnion::convertToWeak() auto action = p->addWeakReleaseStrongRef(); switch (action) { - case noop: + case NoOp: break; - case destroy: + case Destroy: // We just added a weak ref. How could we destroy? // LCOV_EXCL_START UNREACHABLE( @@ -617,7 +625,7 @@ SharedWeakUnion::convertToWeak() unsafeSetRawPtr(nullptr); return true; // Should never happen // LCOV_EXCL_STOP - case partialDestroy: + case PartialDestroy: // This is a weird case. We just converted the last strong // pointer to a weak pointer. p->partialDestructor(); @@ -625,7 +633,7 @@ SharedWeakUnion::convertToWeak() // p is null and may no longer be used break; } - unsafeSetRawPtr(p, RefStrength::weak); + unsafeSetRawPtr(p, RefStrength::Weak); return true; } @@ -633,7 +641,7 @@ template T* SharedWeakUnion::unsafeGetRawPtr() const { - return reinterpret_cast(tp_ & ptrMask); + return reinterpret_cast(tp_ & kPTR_MASK); } template @@ -641,8 +649,8 @@ void SharedWeakUnion::unsafeSetRawPtr(T* p, RefStrength rs) { tp_ = reinterpret_cast(p); - if (tp_ && rs == RefStrength::weak) - tp_ |= tagMask; + if (tp_ && rs == RefStrength::Weak) + tp_ |= kTAG_MASK; } template @@ -666,12 +674,12 @@ SharedWeakUnion::unsafeReleaseNoStore() auto strongAction = p->releaseStrongRef(); switch (strongAction) { - case noop: + case NoOp: break; - case destroy: + case Destroy: delete p; break; - case partialDestroy: + case PartialDestroy: p->partialDestructor(); partialDestructorFinished(&p); // p is null and may no longer be used @@ -684,9 +692,9 @@ SharedWeakUnion::unsafeReleaseNoStore() auto weakAction = p->releaseWeakRef(); switch (weakAction) { - case noop: + case NoOp: break; - case destroy: + case Destroy: delete p; break; } diff --git a/include/xrpl/basics/IntrusiveRefCounts.h b/include/xrpl/basics/IntrusiveRefCounts.h index e7c31bde93..f0689197d7 100644 --- a/include/xrpl/basics/IntrusiveRefCounts.h +++ b/include/xrpl/basics/IntrusiveRefCounts.h @@ -18,7 +18,7 @@ namespace xrpl { destroy: Run the destructor. This action will occur when either the strong count or weak count is decremented and the other count is also zero. */ -enum class ReleaseStrongRefAction { noop, partialDestroy, destroy }; +enum class ReleaseStrongRefAction { NoOp, PartialDestroy, Destroy }; /** Action to perform when releasing a weak pointer. @@ -28,7 +28,7 @@ enum class ReleaseStrongRefAction { noop, partialDestroy, destroy }; destroy: Run the destructor. This action will occur when either the strong count or weak count is decremented and the other count is also zero. */ -enum class ReleaseWeakRefAction { noop, destroy }; +enum class ReleaseWeakRefAction { NoOp, Destroy }; /** Implement the strong count, weak count, and bit flags for an intrusive pointer. @@ -71,7 +71,7 @@ struct IntrusiveRefCounts expired() const noexcept; std::size_t - use_count() const noexcept; + useCount() const noexcept; // This function MUST be called after a partial destructor finishes running. // Calling this function may cause other threads to delete the object @@ -98,11 +98,11 @@ private: // enough for strong pointers and 14 bit counts are enough for weak // pointers. Use type aliases to make it easy to switch types. using CountType = std::uint16_t; - static constexpr size_t StrongCountNumBits = sizeof(CountType) * 8; - static constexpr size_t WeakCountNumBits = StrongCountNumBits - 2; + static constexpr size_t kSTRONG_COUNT_NUM_BITS = sizeof(CountType) * 8; + static constexpr size_t kWEAK_COUNT_NUM_BITS = kSTRONG_COUNT_NUM_BITS - 2; using FieldType = std::uint32_t; - static constexpr size_t FieldTypeBits = sizeof(FieldType) * 8; - static constexpr FieldType one = 1; + static constexpr size_t kFIELD_TYPE_BITS = sizeof(FieldType) * 8; + static constexpr FieldType kONE = 1; /** `refCounts` consists of four fields that are treated atomically: @@ -137,21 +137,21 @@ private: */ - mutable std::atomic refCounts{strongDelta}; + mutable std::atomic refCounts_{kSTRONG_DELTA}; /** Amount to change the strong count when adding or releasing a reference Note: The strong count is stored in the low `StrongCountNumBits` bits of refCounts */ - static constexpr FieldType strongDelta = 1; + static constexpr FieldType kSTRONG_DELTA = 1; /** Amount to change the weak count when adding or releasing a reference Note: The weak count is stored in the high `WeakCountNumBits` bits of refCounts */ - static constexpr FieldType weakDelta = (one << StrongCountNumBits); + static constexpr FieldType kWEAK_DELTA = (kONE << kSTRONG_COUNT_NUM_BITS); /** Flag that is set when the partialDestroy function has started running (or is about to start running). @@ -159,33 +159,34 @@ private: See description of the `refCounts` field for a fuller description of this field. */ - static constexpr FieldType partialDestroyStartedMask = (one << (FieldTypeBits - 1)); + static constexpr FieldType kPARTIAL_DESTROY_STARTED_MASK = (kONE << (kFIELD_TYPE_BITS - 1)); /** Flag that is set when the partialDestroy function has finished running See description of the `refCounts` field for a fuller description of this field. */ - static constexpr FieldType partialDestroyFinishedMask = (one << (FieldTypeBits - 2)); + static constexpr FieldType kPARTIAL_DESTROY_FINISHED_MASK = (kONE << (kFIELD_TYPE_BITS - 2)); /** Mask that will zero out all the `count` bits and leave the tag bits unchanged. */ - static constexpr FieldType tagMask = partialDestroyStartedMask | partialDestroyFinishedMask; + static constexpr FieldType kTAG_MASK = + kPARTIAL_DESTROY_STARTED_MASK | kPARTIAL_DESTROY_FINISHED_MASK; /** Mask that will zero out the `tag` bits and leave the count bits unchanged. */ - static constexpr FieldType valueMask = ~tagMask; + static constexpr FieldType kVALUE_MASK = ~kTAG_MASK; /** Mask that will zero out everything except the strong count. */ - static constexpr FieldType strongMask = ((one << StrongCountNumBits) - 1) & valueMask; + static constexpr FieldType kSTRONG_MASK = ((kONE << kSTRONG_COUNT_NUM_BITS) - 1) & kVALUE_MASK; /** Mask that will zero out everything except the weak count. */ - static constexpr FieldType weakMask = - (((one << WeakCountNumBits) - 1) << StrongCountNumBits) & valueMask; + static constexpr FieldType kWEAK_MASK = + (((kONE << kWEAK_COUNT_NUM_BITS) - 1) << kSTRONG_COUNT_NUM_BITS) & kVALUE_MASK; /** Unpack the count and tag fields from the packed atomic integer form. */ struct RefCountPair @@ -210,29 +211,29 @@ private: [[nodiscard]] FieldType combinedValue() const noexcept; - static constexpr CountType maxStrongValue = - static_cast((one << StrongCountNumBits) - 1); - static constexpr CountType maxWeakValue = - static_cast((one << WeakCountNumBits) - 1); + static constexpr CountType kMAX_STRONG_VALUE = + static_cast((kONE << kSTRONG_COUNT_NUM_BITS) - 1); + static constexpr CountType kMAX_WEAK_VALUE = + static_cast((kONE << kWEAK_COUNT_NUM_BITS) - 1); /** Put an extra margin to detect when running up against limits. This is only used in debug code, and is useful if we reduce the number of bits in the strong and weak counts (to 16 and 14 bits). */ - static constexpr CountType checkStrongMaxValue = maxStrongValue - 32; - static constexpr CountType checkWeakMaxValue = maxWeakValue - 32; + static constexpr CountType kCHECK_STRONG_MAX_VALUE = kMAX_STRONG_VALUE - 32; + static constexpr CountType kCHECK_WEAK_MAX_VALUE = kMAX_WEAK_VALUE - 32; }; }; inline void IntrusiveRefCounts::addStrongRef() const noexcept { - refCounts.fetch_add(strongDelta, std::memory_order_acq_rel); + refCounts_.fetch_add(kSTRONG_DELTA, std::memory_order_acq_rel); } inline void IntrusiveRefCounts::addWeakRef() const noexcept { - refCounts.fetch_add(weakDelta, std::memory_order_acq_rel); + refCounts_.fetch_add(kWEAK_DELTA, std::memory_order_acq_rel); } inline ReleaseStrongRefAction @@ -246,36 +247,36 @@ IntrusiveRefCounts::releaseStrongRef() const // conditional `fetch_or`. This loop will almost always run once. using enum ReleaseStrongRefAction; - auto prevIntVal = refCounts.load(std::memory_order_acquire); + auto prevIntVal = refCounts_.load(std::memory_order_acquire); while (true) { RefCountPair const prevVal{prevIntVal}; XRPL_ASSERT( - (prevVal.strong >= strongDelta), + (prevVal.strong >= kSTRONG_DELTA), "xrpl::IntrusiveRefCounts::releaseStrongRef : previous ref " "higher than new"); - auto nextIntVal = prevIntVal - strongDelta; - ReleaseStrongRefAction action = noop; + auto nextIntVal = prevIntVal - kSTRONG_DELTA; + ReleaseStrongRefAction action = NoOp; if (prevVal.strong == 1) { if (prevVal.weak == 0) { - action = destroy; + action = Destroy; } else { - nextIntVal |= partialDestroyStartedMask; - action = partialDestroy; + nextIntVal |= kPARTIAL_DESTROY_STARTED_MASK; + action = PartialDestroy; } } - if (refCounts.compare_exchange_weak(prevIntVal, nextIntVal, std::memory_order_acq_rel)) + if (refCounts_.compare_exchange_weak(prevIntVal, nextIntVal, std::memory_order_acq_rel)) { // Can't be in partial destroy because only decrementing the strong // count to zero can start a partial destroy, and that can't happen // twice. XRPL_ASSERT( - (action == noop) || !(prevIntVal & partialDestroyStartedMask), + (action == NoOp) || !(prevIntVal & kPARTIAL_DESTROY_STARTED_MASK), "xrpl::IntrusiveRefCounts::releaseStrongRef : not in partial " "destroy"); return action; @@ -288,9 +289,9 @@ IntrusiveRefCounts::addWeakReleaseStrongRef() const { using enum ReleaseStrongRefAction; - static_assert(weakDelta > strongDelta); - auto constexpr delta = weakDelta - strongDelta; - auto prevIntVal = refCounts.load(std::memory_order_acquire); + static_assert(kWEAK_DELTA > kSTRONG_DELTA); + auto constexpr kDELTA = kWEAK_DELTA - kSTRONG_DELTA; + auto prevIntVal = refCounts_.load(std::memory_order_acquire); // This loop will almost always run once. The loop is needed to atomically // change the counts and flags (the count could be atomically changed, but // the flags depend on the current value of the counts). @@ -311,24 +312,24 @@ IntrusiveRefCounts::addWeakReleaseStrongRef() const "xrpl::IntrusiveRefCounts::addWeakReleaseStrongRef : not in " "partial destroy"); - auto nextIntVal = prevIntVal + delta; - ReleaseStrongRefAction action = noop; + auto nextIntVal = prevIntVal + kDELTA; + ReleaseStrongRefAction action = NoOp; if (prevVal.strong == 1) { if (prevVal.weak == 0) { - action = noop; + action = NoOp; } else { - nextIntVal |= partialDestroyStartedMask; - action = partialDestroy; + nextIntVal |= kPARTIAL_DESTROY_STARTED_MASK; + action = PartialDestroy; } } - if (refCounts.compare_exchange_weak(prevIntVal, nextIntVal, std::memory_order_acq_rel)) + if (refCounts_.compare_exchange_weak(prevIntVal, nextIntVal, std::memory_order_acq_rel)) { XRPL_ASSERT( - (!(prevIntVal & partialDestroyStartedMask)), + (!(prevIntVal & kPARTIAL_DESTROY_STARTED_MASK)), "xrpl::IntrusiveRefCounts::addWeakReleaseStrongRef : not " "started partial destroy"); return action; @@ -339,7 +340,7 @@ IntrusiveRefCounts::addWeakReleaseStrongRef() const inline ReleaseWeakRefAction IntrusiveRefCounts::releaseWeakRef() const { - auto prevIntVal = refCounts.fetch_sub(weakDelta, std::memory_order_acq_rel); + auto prevIntVal = refCounts_.fetch_sub(kWEAK_DELTA, std::memory_order_acq_rel); RefCountPair prev = prevIntVal; if (prev.weak == 1 && prev.strong == 0) { @@ -348,19 +349,19 @@ IntrusiveRefCounts::releaseWeakRef() const // This case should only be hit if the partialDestroyStartedBit is // set non-atomically (and even then very rarely). The code is kept // in case we need to set the flag non-atomically for perf reasons. - refCounts.wait(prevIntVal, std::memory_order_acquire); - prevIntVal = refCounts.load(std::memory_order_acquire); + refCounts_.wait(prevIntVal, std::memory_order_acquire); + prevIntVal = refCounts_.load(std::memory_order_acquire); prev = RefCountPair{prevIntVal}; } if (prev.partialDestroyFinishedBit == 0u) { // partial destroy MUST finish before running a full destroy (when // using weak pointers) - refCounts.wait(prevIntVal - weakDelta, std::memory_order_acquire); + refCounts_.wait(prevIntVal - kWEAK_DELTA, std::memory_order_acquire); } - return ReleaseWeakRefAction::destroy; + return ReleaseWeakRefAction::Destroy; } - return ReleaseWeakRefAction::noop; + return ReleaseWeakRefAction::NoOp; } inline bool @@ -369,13 +370,13 @@ IntrusiveRefCounts::checkoutStrongRefFromWeak() const noexcept auto curValue = RefCountPair{1, 1}.combinedValue(); auto desiredValue = RefCountPair{2, 1}.combinedValue(); - while (!refCounts.compare_exchange_weak(curValue, desiredValue, std::memory_order_acq_rel)) + while (!refCounts_.compare_exchange_weak(curValue, desiredValue, std::memory_order_acq_rel)) { RefCountPair const prev{curValue}; if (prev.strong == 0u) return false; - desiredValue = curValue + strongDelta; + desiredValue = curValue + kSTRONG_DELTA; } return true; } @@ -383,38 +384,39 @@ IntrusiveRefCounts::checkoutStrongRefFromWeak() const noexcept inline bool IntrusiveRefCounts::expired() const noexcept { - RefCountPair const val = refCounts.load(std::memory_order_acquire); + RefCountPair const val = refCounts_.load(std::memory_order_acquire); return val.strong == 0; } inline std::size_t -IntrusiveRefCounts::use_count() const noexcept +IntrusiveRefCounts::useCount() const noexcept { - RefCountPair const val = refCounts.load(std::memory_order_acquire); + RefCountPair const val = refCounts_.load(std::memory_order_acquire); return val.strong; } inline IntrusiveRefCounts::~IntrusiveRefCounts() noexcept { #ifndef NDEBUG - auto v = refCounts.load(std::memory_order_acquire); + auto v = refCounts_.load(std::memory_order_acquire); XRPL_ASSERT( - (!(v & valueMask)), "xrpl::IntrusiveRefCounts::~IntrusiveRefCounts : count must be zero"); - auto t = v & tagMask; - XRPL_ASSERT((!t || t == tagMask), "xrpl::IntrusiveRefCounts::~IntrusiveRefCounts : valid tag"); + (!(v & kVALUE_MASK)), "xrpl::IntrusiveRefCounts::~IntrusiveRefCounts : count must be zero"); + auto t = v & kTAG_MASK; + XRPL_ASSERT( + (!t || t == kTAG_MASK), "xrpl::IntrusiveRefCounts::~IntrusiveRefCounts : valid tag"); #endif } //------------------------------------------------------------------------------ inline IntrusiveRefCounts::RefCountPair::RefCountPair(IntrusiveRefCounts::FieldType v) noexcept - : strong{static_cast(v & strongMask)} - , weak{static_cast((v & weakMask) >> StrongCountNumBits)} - , partialDestroyStartedBit{v & partialDestroyStartedMask} - , partialDestroyFinishedBit{v & partialDestroyFinishedMask} + : strong{static_cast(v & kSTRONG_MASK)} + , weak{static_cast((v & kWEAK_MASK) >> kSTRONG_COUNT_NUM_BITS)} + , partialDestroyStartedBit{v & kPARTIAL_DESTROY_STARTED_MASK} + , partialDestroyFinishedBit{v & kPARTIAL_DESTROY_FINISHED_MASK} { XRPL_ASSERT( - (strong < checkStrongMaxValue && weak < checkWeakMaxValue), + (strong < kCHECK_STRONG_MAX_VALUE && weak < kCHECK_WEAK_MAX_VALUE), "xrpl::IntrusiveRefCounts::RefCountPair(FieldType) : inputs inside " "range"); } @@ -425,7 +427,7 @@ inline IntrusiveRefCounts::RefCountPair::RefCountPair( : strong{s}, weak{w} { XRPL_ASSERT( - (strong < checkStrongMaxValue && weak < checkWeakMaxValue), + (strong < kCHECK_STRONG_MAX_VALUE && weak < kCHECK_WEAK_MAX_VALUE), "xrpl::IntrusiveRefCounts::RefCountPair(CountType, CountType) : " "inputs inside range"); } @@ -434,11 +436,11 @@ inline IntrusiveRefCounts::FieldType IntrusiveRefCounts::RefCountPair::combinedValue() const noexcept { XRPL_ASSERT( - (strong < checkStrongMaxValue && weak < checkWeakMaxValue), + (strong < kCHECK_STRONG_MAX_VALUE && weak < kCHECK_WEAK_MAX_VALUE), "xrpl::IntrusiveRefCounts::RefCountPair::combinedValue : inputs " "inside range"); return (static_cast(weak) - << IntrusiveRefCounts::StrongCountNumBits) | + << IntrusiveRefCounts::kSTRONG_COUNT_NUM_BITS) | static_cast(strong) | partialDestroyStartedBit | partialDestroyFinishedBit; } @@ -449,7 +451,7 @@ partialDestructorFinished(T** o) { T& self = **o; IntrusiveRefCounts::RefCountPair const p = - self.refCounts.fetch_or(IntrusiveRefCounts::partialDestroyFinishedMask); + self.refCounts_.fetch_or(IntrusiveRefCounts::kPARTIAL_DESTROY_FINISHED_MASK); XRPL_ASSERT( (!p.partialDestroyFinishedBit && p.partialDestroyStartedBit && !p.strong), "xrpl::partialDestructorFinished : not a weak ref"); @@ -458,7 +460,7 @@ partialDestructorFinished(T** o) // There was a weak count before the partial destructor ran (or we would // have run the full destructor) and now there isn't a weak count. Some // thread is waiting to run the destructor. - self.refCounts.notify_one(); + self.refCounts_.notify_one(); } // Set the pointer to null to emphasize that the object shouldn't be used // after calling this function as it may be destroyed in another thread. diff --git a/include/xrpl/basics/LocalValue.h b/include/xrpl/basics/LocalValue.h index 421ea7af23..f39df425a6 100644 --- a/include/xrpl/basics/LocalValue.h +++ b/include/xrpl/basics/LocalValue.h @@ -26,17 +26,17 @@ struct LocalValues template struct Value : BasicValue { - T t_; + T t; Value() = default; - explicit Value(T t) : t_(std::move(t)) + explicit Value(T t) : t(std::move(t)) { } void* get() override { - return &t_; + return &t; } }; @@ -55,8 +55,8 @@ template boost::thread_specific_ptr& getLocalValues() { - static boost::thread_specific_ptr tsp(&detail::LocalValues::cleanup); - return tsp; + static boost::thread_specific_ptr kTSP(&detail::LocalValues::cleanup); + return kTSP; } } // namespace detail diff --git a/include/xrpl/basics/Log.h b/include/xrpl/basics/Log.h index 58cca4f486..8e6532f7c0 100644 --- a/include/xrpl/basics/Log.h +++ b/include/xrpl/basics/Log.h @@ -15,16 +15,17 @@ namespace xrpl { // DEPRECATED use beast::severities::Severity instead +// NOLINTNEXTLINE(cppcoreguidelines-use-enum-class) enum LogSeverity { - lsINVALID = -1, // used to indicate an invalid severity - lsTRACE = 0, // Very low-level progress information, details inside + LSInvalid = -1, // used to indicate an invalid severity + LSTrace = 0, // Very low-level progress information, details inside // an operation - lsDEBUG = 1, // Function-level progress information, operations - lsINFO = 2, // Server-level progress information, major operations - lsWARNING = 3, // Conditions that warrant human attention, may indicate + LSDebug = 1, // Function-level progress information, operations + LSInfo = 2, // Server-level progress information, major operations + LSWarning = 3, // Conditions that warrant human attention, may indicate // a problem - lsERROR = 4, // A condition that indicates a problem - lsFATAL = 5 // A severe condition that indicates a server problem + LSError = 4, // A condition that indicates a problem + LSFatal = 5 // A severe condition that indicates a server problem }; /** Manages partitions for logging. */ @@ -129,8 +130,8 @@ private: /** @} */ private: - std::unique_ptr m_stream; - boost::filesystem::path m_path; + std::unique_ptr stream_; + boost::filesystem::path path_; }; std::mutex mutable mutex_; @@ -167,7 +168,7 @@ public: threshold(beast::severities::Severity thresh); std::vector> - partition_severities() const; + partitionSeverities() const; void write( @@ -207,11 +208,13 @@ public: fromString(std::string const& s); private: + // Need to be named before converting + // NOLINTNEXTLINE(cppcoreguidelines-use-enum-class) enum { // Maximum line length for log messages. // If the message exceeds this length it will be truncated with // ellipses. - maximumMessageCharacters = 12 * 1024 + MaximumMessageCharacters = 12 * 1024 }; static void diff --git a/include/xrpl/basics/Mutex.hpp b/include/xrpl/basics/Mutex.hpp index 5855ee2017..4432e27b4b 100644 --- a/include/xrpl/basics/Mutex.hpp +++ b/include/xrpl/basics/Mutex.hpp @@ -131,7 +131,7 @@ public: * @tparam LockType The type of lock to use * @return A lock on the mutex and a reference to the protected data */ - template