From 26712e3801bef9f6bf6d663b6213717cb34809ac Mon Sep 17 00:00:00 2001 From: moxian Date: Wed, 12 Feb 2025 23:57:44 -0800 Subject: [PATCH] Upgrade clang-tidy to LLVM 18, and fix (some) newly firing lints (#79633) * Upgrade clang-tidy to llvm-18 - fix compilation issues - update the CI - drive-by change: bump the displayed "most time consuming checks" from top 10 to top 30 * Make .clang-tidy use structured list for checks instead of a single string that relies on `\` with no trailing whitespace for line joining. Both because the \ thing is bugprone, but mostly because i want to add extra comments about *why* the checks are disabled * Shuffle comments around to be closer to the thing they are commenting on * Disable newly discovered firing checks This partially defeats the whole point of update, but maybe that's still worth it. Improved existing checks: modernize-make-shared readability-redundant-member-init Newly introduced checks: bugprone-chained-comparison bugprone-multi-level-implicit-pointer-conversion bugprone-optional-value-conversion bugprone-unused-local-non-trivial-variable clang-analyzer-optin.core.EnumCastOutOfRange performance-enum-size readability-avoid-nested-conditional-operator readability-avoid-return-with-void-value readability-redundant-casting readability-redundant-inline-specifier readability-reference-to-constructed-temporary * Reenable and fix modernize-make-shared https://clang.llvm.org/extra/clang-tidy/checks/modernize/make-shared.html the new check is the x.reset(new T(args)) => x=make_shared::(args) form example error: ##[error]/home/runner/work/Cataclysm-DDA/Cataclysm-DDA/src/item_location.cpp:838:13: error: use std::make_shared instead [modernize-make-shared,-warnings-as-errors] 9 | ptr.reset( new impl::item_on_person( who_id, idx ) ); | ~^~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~ ~ | = std::make_shared * Re-enable and fix readability-redundant-member-init lint https://clang.llvm.org/extra/clang-tidy/checks/readability/redundant-member-init.html Sample error: ##[error]/home/runner/work/Cataclysm-DDA/Cataclysm-DDA/src/effect_source.h:47:41: error: initializer for member 'fac' is redundant [readability-redundant-member-init,-warnings-as-errors] 47 | std::optional fac = faction_id(); | ~~^~~~~~~~~~~~ * Enable and fix bugprone-optional-value-conversion lint https://clang.llvm.org/extra/clang-tidy/checks/bugprone/optional-value-conversion.html Sample error: ##[error]/home/runner/work/Cataclysm-DDA/Cataclysm-DDA/src/mission_util.cpp:182:67: error: conversion from 'std::optional>>' into 'abstract_var_info>' and back into 'std::optional>>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion,-warnings-as-errors] 182 | return project_to( get_tripoint_ms_from_var( params.target_var.value(), d, false ) ); | ^ /home/runner/work/Cataclysm-DDA/Cataclysm-DDA/src/mission_util.cpp:182:85: note: remove call to 'value' to silence this warning 182 | return project_to( get_tripoint_ms_from_var( params.target_var.value(), d, false ) ); | ~^~~~~~~ * Fix and reenable bugprone-chained-comparison https://clang.llvm.org/extra/clang-tidy/checks/bugprone/chained-comparison.html It only crops up in tests due to how catch2 works. Catch2 has silenced the lint on their end in https://github.com/catchorg/Catch2/commit/1078e7e95b3a06d4dadc75188de48bc4afffb955 This commit mirrors the upstream change Example error: ##[error]/home/runner/work/Cataclysm-DDA/Cataclysm-DDA/tests/zones_custom_test.cpp:56:9: error: chained comparison 'v0 <= v1 == v2' may generate unintended results, use parentheses to specify order of evaluation or a logical operator to separate comparison expressions [bugprone-chained-comparison,-warnings-as-errors] 56 | REQUIRE( zmgr.get_near_zone_type_for_item( hammer, where ) == zone_type_LOOT_CUSTOM ); | ^ /home/runner/work/Cataclysm-DDA/Cataclysm-DDA/tests/catch/catch.hpp:17636:24: note: expanded from macro 'REQUIRE' 17636 | #define REQUIRE( ... ) INTERNAL_CATCH_TEST( "REQUIRE", Catch::ResultDisposition::Normal, __VA_ARGS__ ) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /home/runner/work/Cataclysm-DDA/Cataclysm-DDA/tests/catch/catch.hpp:2706:47: note: expanded from macro 'INTERNAL_CATCH_TEST' 2706 | catchAssertionHandler.handleExpr( Catch::Decomposer() <= __VA_ARGS__ ); \ | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /home/runner/work/Cataclysm-DDA/Cataclysm-DDA/tests/zones_custom_test.cpp:56:9: note: operand 'v0' is here 56 | REQUIRE( zmgr.get_near_zone_type_for_item( hammer, where ) == zone_type_LOOT_CUSTOM ); | ^ /home/runner/work/Cataclysm-DDA/Cataclysm-DDA/tests/catch/catch.hpp:17636:24: note: expanded from macro 'REQUIRE' 17636 | #define REQUIRE( ... ) INTERNAL_CATCH_TEST( "REQUIRE", Catch::ResultDisposition::Normal, __VA_ARGS__ ) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /home/runner/work/Cataclysm-DDA/Cataclysm-DDA/tests/catch/catch.hpp:2706:47: note: expanded from macro 'INTERNAL_CATCH_TEST' 2706 | catchAssertionHandler.handleExpr( Catch::Decomposer() <= __VA_ARGS__ ); \ | ^~~~~~~~~~~~~~~~~~~ /home/runner/work/Cataclysm-DDA/Cataclysm-DDA/tests/zones_custom_test.cpp:56:18: note: operand 'v1' is here 56 | REQUIRE( zmgr.get_near_zone_type_for_item( hammer, where ) == zone_type_LOOT_CUSTOM ); | ^ /home/runner/work/Cataclysm-DDA/Cataclysm-DDA/tests/catch/catch.hpp:17636:90: note: expanded from macro 'REQUIRE' 17636 | #define REQUIRE( ... ) INTERNAL_CATCH_TEST( "REQUIRE", Catch::ResultDisposition::Normal, __VA_ARGS__ ) | ^~~~~~~~~~~ /home/runner/work/Cataclysm-DDA/Cataclysm-DDA/tests/catch/catch.hpp:2706:70: note: expanded from macro 'INTERNAL_CATCH_TEST' 2706 | catchAssertionHandler.handleExpr( Catch::Decomposer() <= __VA_ARGS__ ); \ | ^~~~~~~~~~~ /home/runner/work/Cataclysm-DDA/Cataclysm-DDA/tests/zones_custom_test.cpp:56:71: note: operand 'v2' is here 56 | REQUIRE( zmgr.get_near_zone_type_for_item( hammer, where ) == zone_type_LOOT_CUSTOM ); | ^ /home/runner/work/Cataclysm-DDA/Cataclysm-DDA/tests/catch/catch.hpp:17636:90: note: expanded from macro 'REQUIRE' 17636 | #define REQUIRE( ... ) INTERNAL_CATCH_TEST( "REQUIRE", Catch::ResultDisposition::Normal, __VA_ARGS__ ) | ^~~~~~~~~~~ /home/runner/work/Cataclysm-DDA/Cataclysm-DDA/tests/catch/catch.hpp:2706:70: note: expanded from macro 'INTERNAL_CATCH_TEST' 2706 | catchAssertionHandler.handleExpr( Catch::Decomposer() <= __VA_ARGS__ ); \ | ^~~~~~~~~~~ * Fix and re-enable readability-reference-to-constructed-temporary https://clang.llvm.org/extra/clang-tidy/checks/readability/reference-to-constructed-temporary.html Example error: ##[error]/home/runner/work/Cataclysm-DDA/Cataclysm-DDA/src/melee.cpp:1809:32: error: reference variable 'dest' extends the lifetime of a just-constructed temporary object 'const tripoint_bub_ms' (aka 'const coords::coord_point_ob'), consider changing reference to value [readability-reference-to-constructed-temporary,-warnings-as-errors] 1809 | const tripoint_bub_ms &dest{ new_, b.z()}; | * Fix and reenable readability-avoid-return-with-void-value https://clang.llvm.org/extra/clang-tidy/checks/readability/avoid-return-with-void-value.html Example error: ##[error]/home/runner/work/Cataclysm-DDA/Cataclysm-DDA/src/map.h:2397:13: error: return statement within a void function should not have a specified return value [readability-avoid-return-with-void-value,-warnings-as-errors] 2397 | return map::i_clear( rebase_bub( p ) ); | * Triage readability-redundant-inline-specifier as not desirable https://clang.llvm.org/extra/clang-tidy/checks/readability/redundant-inline-specifier.html Sample error: ##[error]/home/runner/work/Cataclysm-DDA/Cataclysm-DDA/src/point.h:33:5: error: function 'is_invalid' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier,-warnings-as-errors] 33 | inline bool is_invalid() const { | ^~~~~~ Reasoning to keep disabled in the comments * Triage clang-analyzer-optin.core.EnumCastOutOfRange as not desirable https://clang.llvm.org/extra/clang-tidy/checks/clang-analyzer/optin.core.EnumCastOutOfRange.html Rationale in comments. Example error: ##[error]/home/runner/work/Cataclysm-DDA/Cataclysm-DDA/src/enum_traits.h:99:12: error: The value '7' provided to the cast expression is not in the valid range of values for 'part_status_flag' [clang-analyzer-optin.core.EnumCastOutOfRange,-warnings-as-errors] 99 | return static_cast( static_cast( l ) | static_cast( r ) ); | ^ /home/runner/work/Cataclysm-DDA/Cataclysm-DDA/src/vehicle.h:100:12: note: enum declared here 100 | enum class part_status_flag : int { | ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~ 101 | any = 0, | ~~~~~~~~ 102 | working = 1 << 0, | ~~~~~~~~~~~~~~~~~ 103 | available = 1 << 1, | ~~~~~~~~~~~~~~~~~~~ 104 | enabled = 1 << 2 | ~~~~~~~~~~~~~~~~ 105 | }; | ~ /home/runner/work/Cataclysm-DDA/Cataclysm-DDA/src/vehicle.cpp:3124:44: note: Calling 'operator|' 3124 | static_cast( part_status_flag::enabled | | ^~~~~~~~~~~~~~~~~~~~~~~~~~~ 3125 | part_status_flag::working | | ~~~~~~~~~~~~~~~~~~~~~~~~~~~ 3126 | part_status_flag::available ) ); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~ /home/runner/work/Cataclysm-DDA/Cataclysm-DDA/src/enum_traits.h:99:12: note: The value '7' provided to the cast expression is not in the valid range of values for 'part_status_flag' 99 | return static_cast( static_cast( l ) | static_cast( r ) ); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ * Mark the remaining lints as "maybe some other day" Lints in question: bugprone-multi-level-implicit-pointer-conversion bugprone-unused-local-non-trivial-variable performance-enum-size readability-avoid-nested-conditional-operator readability-redundant-casting They look reasonable but I can't really deal with them right now. * Fix readability-container-size-empty (and keep it enabled) https://clang.llvm.org/extra/clang-tidy/checks/readability/container-size-empty.html Error: /home/runner/work/Cataclysm-DDA/Cataclysm-DDA/src/climbing.cpp:222:13: error: the 'empty' method should be used to check for emptiness instead of 'length' [readability-container-size-empty,-warnings-as-errors] 222 | if( menu_hotkey_str.length() ) { | ^~~~~~~~~~~~~~~~~~~~~~~~ | !menu_hotkey_str.empty() * Disable two lints for performance reasons. Lints in question: misc-unused-using-decls modernize-replace-auto-ptr These are in top-10 most expensive checks. For context: here's the profiling data when running clang-tidy on ~most of the codebase (as seen in the CI in one of the versions of the current PR): { "time.clang-tidy.misc-unused-using-decls.wall": 1094.7915439605713, "time.clang-tidy.bugprone-stringview-nullptr.wall": 866.0817050933838, "time.clang-tidy.cata-redundant-parentheses.wall": 839.2004644870758, "time.clang-tidy.modernize-type-traits.wall": 814.9849390983582, "time.clang-tidy.bugprone-use-after-move.wall": 694.5256803035736, "time.clang-tidy.modernize-use-transparent-functors.wall": 670.7836837768555, "time.clang-tidy.bugprone-standalone-empty.wall": 600.5903759002686, "time.clang-tidy.modernize-replace-auto-ptr.wall": 580.4591262340546, "time.clang-tidy.modernize-deprecated-ios-base-aliases.wall": 566.2826192378998, "time.clang-tidy.bugprone-reserved-identifier.wall": 558.5143051147461, "time.clang-tidy.modernize-avoid-c-arrays.wall": 545.1163799762726, "time.clang-tidy.modernize-use-using.wall": 539.6424849033356, "time.clang-tidy.performance-unnecessary-value-param.wall": 471.33301615715027, "time.clang-tidy.readability-non-const-parameter.wall": 440.45192885398865, "time.clang-tidy.cert-dcl58-cpp.wall": 421.88030886650085, "time.clang-tidy.cata-translate-string-literal.wall": 390.8441689014435, "time.clang-tidy.cata-static-initialization-order.wall": 388.08465909957886, "time.clang-tidy.readability-redundant-declaration.wall": 381.59097266197205, "time.clang-tidy.cert-dcl16-c.wall": 362.8544645309448, "time.clang-tidy.readability-container-size-empty.wall": 358.62095737457275, "time.clang-tidy.modernize-use-nullptr.wall": 352.8454167842865, "time.clang-tidy.bugprone-suspicious-string-compare.wall": 350.5792667865753, "time.clang-tidy.misc-misleading-identifier.wall": 346.3509900569916, "time.clang-tidy.misc-definitions-in-headers.wall": 330.5536653995514, "time.clang-tidy.readability-redundant-control-flow.wall": 329.31720495224, "time.clang-tidy.bugprone-infinite-loop.wall": 319.4184067249298, "time.clang-tidy.bugprone-unused-return-value.wall": 306.18062829971313, "time.clang-tidy.performance-move-const-arg.wall": 300.217401266098, "time.clang-tidy.bugprone-assert-side-effect.wall": 298.9822633266449, "time.clang-tidy.bugprone-multiple-statement-macro.wall": 297.0318651199341 } * Fix and re-enable bugprone-multi-level-implicit-pointer-conversion https://clang.llvm.org/extra/clang-tidy/checks/bugprone/multi-level-implicit-pointer-conversion.html Two points of occurence, both are about passing T** to memX() functions. The usage is intended, and correct, so silence. Although perhaps we could add an explicit `reinterpret_cast` there, but I'm not sure it's valuable. ##[error]/home/runner/work/Cataclysm-DDA/Cataclysm-DDA/src/cata_bitset.h:182:25: error: multilevel pointer conversion from 'block_t **' (aka 'unsigned long **') to 'void *', please use explicit cast [bugprone-multi-level-implicit-pointer-conversion,-warnings-as-errors] 182 | memcpy( &ret, &storage_, sizeof( storage_ ) ); | --- .clang-tidy | 207 ++++++++++-------- .github/workflows/clang-tidy.yml | 16 +- .vscode/settings.json | 2 +- build-scripts/clang-tidy-build.sh | 6 +- src/activity_actor_definitions.h | 6 +- src/activity_item_handling.cpp | 2 +- src/cata_bitset.cpp | 3 +- src/cata_bitset.h | 1 + src/cata_tiles.cpp | 2 +- src/character_modifier.h | 2 +- src/climbing.cpp | 2 +- src/construction.h | 2 +- src/creature.cpp | 18 +- src/cursesport.cpp | 14 +- src/debug.cpp | 2 +- src/dialogue.h | 8 +- src/dialogue_chatbin.h | 4 +- src/dialogue_helpers.h | 2 +- src/effect_source.h | 6 +- src/game.cpp | 2 +- src/handle_action.cpp | 2 +- src/iexamine.cpp | 2 +- src/item_location.cpp | 15 +- src/iuse_actor.h | 2 +- src/map.cpp | 2 +- src/map.h | 8 +- src/math_parser_diag.cpp | 18 +- src/melee.cpp | 2 +- src/mission_util.cpp | 2 +- src/morale.cpp | 16 +- src/npc.h | 4 +- src/npctalk.h | 8 +- src/overmap_ui.h | 2 +- src/overmapbuffer.cpp | 2 +- src/text_style_check.h | 2 + src/trap.cpp | 4 +- src/veh_type.h | 2 +- src/wish.cpp | 2 +- tests/catch/catch.hpp | 2 +- .../CombineLocalsIntoPointCheck.cpp | 2 +- tools/clang-tidy-plugin/HeaderGuardCheck.cpp | 2 +- tools/clang-tidy-plugin/NoLongCheck.cpp | 2 +- .../SimplifyPointConstructorsCheck.cpp | 2 +- .../StaticIntIdConstantsCheck.cpp | 2 +- .../StaticStringIdConstantsCheck.cpp | 8 +- tools/clang-tidy-plugin/TestFilenameCheck.cpp | 2 +- .../clang-tidy-plugin/UnusedStaticsCheck.cpp | 2 +- .../UseLocalizedSortingCheck.cpp | 2 +- .../UsePointArithmeticCheck.cpp | 2 +- tools/clang-tidy-plugin/Utils.cpp | 8 +- tools/clang-tidy-plugin/Utils.h | 2 +- 51 files changed, 230 insertions(+), 210 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 6cdfddae7c577..85e157c0ae773 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -4,104 +4,117 @@ # this codebase and we do not intend to fix. The disabled checks appearing # thereafter in a separate alphabetical list have yet to be triaged. We may # fix their errors or recategorise them as checks we don't care about. -# -# Comments on the checks we have decided are not worthwhile: -# -# * bugprone-throw-keyword-missing -# This check is too time consuming. Disable it for now to save CI time. -# -# * cert-dcl21-cpp (postfix operator++ and operator-- should return const objects) -# This is an unconventional code style, and conflicts with -# readability-const-return-type. -# -# * cert-env33-c (calls to system, popen) -# Unlikely to catch bugs, and using system is convenient for portability. -# -# * cert-dcl37-c and cert-dcl-51-cpp (reserved identifiers) -# These two checks are aliases for bugprone-reserved-identifier. -# Don't repeatedly run the same check for three times. -# -# * cert-err58-cpp (exceptions from static variable declarations) -# We have lots of memory allocations in static variable declarations, and -# that's fine. -# -# * clang-analyzer-core.{DivideZero,NonNullParamChecker,UndefinedBinaryOperatorResult} -# * clang-analyzer-cplusplus.NewDelete -# They report too many false positives. -# -# * modernize-use-auto -# We prefer an almost-always-avoid-auto style. -# -# * modernize-use-trailing-return-type -# An arbitrary style convention we haven't adopted. -# -# * readability-identifier-naming -# We are not enforcing a standard identifier naming scheme in the code base. -# This check does not bring much value at the moment and consumes a lot of CPU time. -Checks: "\ -bugprone-*,\ -cata-*,\ -cert-*,\ --cert-dcl21-cpp,\ --cert-env33-c,\ --cert-dcl37-c,\ --cert-dcl51-cpp,\ --cert-err58-cpp,\ --clang-analyzer-core.CallAndMessage,\ --clang-analyzer-core.DivideZero,\ --clang-analyzer-core.NonNullParamChecker,\ --clang-analyzer-core.UndefinedBinaryOperatorResult,\ --clang-analyzer-cplusplus.NewDelete,\ -clang-diagnostic-*,\ -cppcoreguidelines-slicing,\ -google-explicit-constructor,\ -llvm-namespace-comment,\ -misc-*,\ -modernize-*,\ --modernize-use-auto,\ --modernize-use-trailing-return-type,\ -performance-*,\ -readability-*,\ --bugprone-assignment-in-if-condition,\ --bugprone-easily-swappable-parameters,\ --bugprone-empty-catch,\ --bugprone-implicit-widening-of-multiplication-result,\ --bugprone-narrowing-conversions,\ --bugprone-switch-missing-default-case,\ --bugprone-throw-keyword-missing,\ --bugprone-unchecked-optional-access,\ --bugprone-unhandled-exception-at-new,\ --misc-confusable-identifiers,\ --misc-const-correctness,\ --misc-header-include-cycle,\ --misc-include-cleaner,\ --misc-no-recursion,\ --misc-non-private-member-variables-in-classes,\ --misc-use-anonymous-namespace,\ --modernize-concat-nested-namespaces,\ --modernize-macro-to-enum,\ --modernize-pass-by-value,\ --modernize-return-braced-init-list,\ --modernize-use-default-member-init,\ --modernize-use-nodiscard,\ --performance-avoid-endl,\ --performance-noexcept-swap,\ --performance-no-automatic-move,\ --readability-avoid-unconditional-preprocessor-if,\ --readability-container-data-pointer,\ --readability-convert-member-functions-to-static,\ --readability-else-after-return,\ --readability-function-cognitive-complexity,\ --readability-identifier-length,\ --readability-identifier-naming,\ --readability-implicit-bool-conversion,\ --readability-magic-numbers,\ --readability-named-parameter,\ --readability-simplify-boolean-expr,\ --readability-suspicious-call-argument,\ --readability-use-anyofallof,\ -" +Checks: [ + bugprone-*, + # This check is too time consuming. Disable it for now to save CI time. + -bugprone-throw-keyword-missing, + cata-*, + cert-*, + # * cert-dcl21-cpp (postfix operator++ and operator-- should return const objects) + # This is an unconventional code style, and conflicts with + # readability-const-return-type. + -cert-dcl21-cpp, + # * cert-env33-c (calls to system, popen) + # Unlikely to catch bugs, and using system is convenient for portability. + -cert-env33-c, + # * cert-dcl37-c and cert-dcl-51-cpp (reserved identifiers) + # These two checks are aliases for bugprone-reserved-identifier. + # Don't repeatedly run the same check for three times. + -cert-dcl37-c, + -cert-dcl51-cpp, + # * cert-err58-cpp (exceptions from static variable declarations) + # We have lots of memory allocations in static variable declarations, and + # that's fine. + -cert-err58-cpp, + # * clang-analyzer-core.{DivideZero,NonNullParamChecker,UndefinedBinaryOperatorResult} + # * clang-analyzer-cplusplus.NewDelete + # They report too many false positives. + -clang-analyzer-core.CallAndMessage, + -clang-analyzer-core.DivideZero, + -clang-analyzer-core.NonNullParamChecker, + -clang-analyzer-core.UndefinedBinaryOperatorResult, + -clang-analyzer-cplusplus.NewDelete, + # We often use enums as bitsets and do things like `enum::A | enum::B` + # which is explicitly unsupported by this check, and it is officially recommended to disable + # this lint for project that use such patterns. + -clang-analyzer-optin.core.EnumCastOutOfRange, + clang-diagnostic-*, + cppcoreguidelines-slicing, + google-explicit-constructor, + llvm-namespace-comment, + misc-*, + # Extremely expensive, and we are not using `using` anyway, so it's not catching anything. + -misc-unused-using-decls, + modernize-*, + # Rather expensive check, and it is unlikely that somebody + # would *want* to use std::auto_ptr in %CURRENT_YEAR% (2025+) when unique_ptr is both better + # and is a de-facto default in the codebase already. + -modernize-replace-auto-ptr, + # * modernize-use-auto + # We prefer an almost-always-avoid-auto style. + -modernize-use-auto, + # * modernize-use-trailing-return-type + # An arbitrary style convention we haven't adopted. + -modernize-use-trailing-return-type, + performance-*, + readability-*, + # * readability-identifier-naming + # We are not enforcing a standard identifier naming scheme in the code base. + # This check does not bring much value at the moment and consumes a lot of CPU time. + -readability-identifier-length, + -readability-identifier-naming, + # disabled due to behaviour change between pre-module and post-module world. + # Reevaluate in 2027(?) when the code is sufficiently migrated to C++20 modules + # or when there is a decision not perform the migration + -readability-redundant-inline-specifier, + + # ==== Untriaged checks follow ==== + # Either fix the code and re-enable the check, + # or add a good comment and move to the appropriate section above. + # Silencing the existing errors with a //NOLINT does count as a "fix", as that still + # prevents new issues from cropping up. + -bugprone-assignment-in-if-condition, + -bugprone-easily-swappable-parameters, + -bugprone-empty-catch, + -bugprone-implicit-widening-of-multiplication-result, + -bugprone-narrowing-conversions, + -bugprone-switch-missing-default-case, + -bugprone-unchecked-optional-access, + -bugprone-unhandled-exception-at-new, + -bugprone-unused-local-non-trivial-variable, # great lint, but hard to fix + -misc-confusable-identifiers, + -misc-const-correctness, + -misc-header-include-cycle, + -misc-include-cleaner, + -misc-no-recursion, + -misc-non-private-member-variables-in-classes, + -misc-use-anonymous-namespace, + -modernize-concat-nested-namespaces, + -modernize-macro-to-enum, + -modernize-pass-by-value, + -modernize-return-braced-init-list, + -modernize-use-default-member-init, + -modernize-use-nodiscard, + -performance-avoid-endl, + -performance-enum-size, # untriaged since upgrade to LLVM 18 + -performance-noexcept-swap, + -performance-no-automatic-move, + -readability-avoid-nested-conditional-operator, # great lint, but hard to fix + -readability-avoid-unconditional-preprocessor-if, + -readability-container-data-pointer, + -readability-convert-member-functions-to-static, + -readability-else-after-return, + -readability-function-cognitive-complexity, + -readability-implicit-bool-conversion, + -readability-magic-numbers, + -readability-named-parameter, + -readability-redundant-casting, # probably good lint, just needs fixing + -readability-simplify-boolean-expr, + -readability-suspicious-call-argument, + -readability-use-anyofallof, +] + WarningsAsErrors: '*' HeaderFilterRegex: '(src|test|tools).*' FormatStyle: none diff --git a/.github/workflows/clang-tidy.yml b/.github/workflows/clang-tidy.yml index 41eba657a5fec..dcb2c1f6bf930 100644 --- a/.github/workflows/clang-tidy.yml +++ b/.github/workflows/clang-tidy.yml @@ -1,4 +1,4 @@ -name: Clang-tidy 17 +name: Clang-tidy 18 on: push: @@ -36,13 +36,13 @@ jobs: fail-fast: true runs-on: ubuntu-24.04 env: - COMPILER: clang++-17 + COMPILER: clang++-18 steps: - - name: install LLVM 17 + - name: install LLVM 18 if: ${{ needs.skip-duplicates.outputs.should_skip != 'true' && github.event_name != 'pull_request' || github.event.pull_request.draft == false }} run: | sudo apt-get update - sudo apt install llvm-17 llvm-17-dev llvm-17-tools clang-17 clang-tidy-17 clang-tools-17 libclang-17-dev + sudo apt install llvm-18 llvm-18-dev llvm-18-tools clang-18 clang-tidy-18 clang-tools-18 libclang-18-dev sudo apt install python3-pip ninja-build cmake pip3 install --user lit - name: checkout repository @@ -74,8 +74,8 @@ jobs: runs-on: ubuntu-24.04 env: - COMPILER: clang++-17 - CATA_CLANG_TIDY: clang-tidy-17 + COMPILER: clang++-18 + CATA_CLANG_TIDY: clang-tidy-18 CATA_CLANG_TIDY_SUBSET: ${{ matrix.subset }} TILES: 1 SOUND: 1 @@ -85,7 +85,7 @@ jobs: if: ${{ needs.skip-duplicates.outputs.should_skip != 'true' && github.event_name != 'pull_request' || github.event.pull_request.draft == false }} run: | sudo apt-get update - sudo apt install clang-17 clang-tidy-17 cmake ccache jq + sudo apt install clang-18 clang-tidy-18 cmake ccache jq sudo apt install libflac-dev libsdl2-dev libsdl2-ttf-dev libsdl2-image-dev libsdl2-mixer-dev libpulse-dev gettext - name: checkout repository uses: actions/checkout@v4 @@ -122,7 +122,7 @@ jobs: run: | # the folder may not exist if there is no file to analyze if [ -d clang-tidy-trace ] then - jq -n 'reduce(inputs.profile | to_entries[]) as {$key,$value} ({}; .[$key] += $value) | with_entries(select(.key|contains(".wall"))) | to_entries | sort_by(.value) | reverse | .[0:10] | from_entries' clang-tidy-trace/*.json + jq -n 'reduce(inputs.profile | to_entries[]) as {$key,$value} ({}; .[$key] += $value) | with_entries(select(.key|contains(".wall"))) | to_entries | sort_by(.value) | reverse | .[0:30] | from_entries' clang-tidy-trace/*.json else echo "clang-tidy-trace folder not found." fi diff --git a/.vscode/settings.json b/.vscode/settings.json index 95294f0d6160a..807bba2e1fee1 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -85,6 +85,6 @@ "C_Cpp.formatting": "disabled", "astyle.astylerc": "${workspaceRoot}/.astylerc", "files.associations": { - "vector": "cpp" + ".clang-tidy": "yaml", } } diff --git a/build-scripts/clang-tidy-build.sh b/build-scripts/clang-tidy-build.sh index 729500be89532..478010786db0e 100644 --- a/build-scripts/clang-tidy-build.sh +++ b/build-scripts/clang-tidy-build.sh @@ -15,8 +15,8 @@ cmake_extra_opts=() cmake_extra_opts+=("-DCATA_CLANG_TIDY_PLUGIN=ON") # Need to specify the particular LLVM / Clang versions to use, lest it # use the older LLVM that comes by default on Ubuntu. -cmake_extra_opts+=("-DLLVM_DIR=/usr/lib/llvm-17/lib/cmake/llvm") -cmake_extra_opts+=("-DClang_DIR=/usr/lib/llvm-17/lib/cmake/clang") +cmake_extra_opts+=("-DLLVM_DIR=/usr/lib/llvm-18/lib/cmake/llvm") +cmake_extra_opts+=("-DClang_DIR=/usr/lib/llvm-18/lib/cmake/clang") mkdir -p build @@ -33,7 +33,7 @@ echo "Compiling clang-tidy plugin" make -j$num_jobs CataAnalyzerPlugin #export PATH=$PWD/tools/clang-tidy-plugin/clang-tidy-plugin-support/bin:$PATH # add FileCheck to the search path -export PATH=/usr/lib/llvm-17/bin:$PATH +export PATH=/usr/lib/llvm-18/bin:$PATH if ! which FileCheck then echo "Missing FileCheck" diff --git a/src/activity_actor_definitions.h b/src/activity_actor_definitions.h index b1fba622ec8ec..94ca23656e725 100644 --- a/src/activity_actor_definitions.h +++ b/src/activity_actor_definitions.h @@ -54,7 +54,7 @@ class aim_activity_actor : public activity_actor bool should_unload_RAS = false; bool snap_to_target = false; /* Item location for RAS weapon reload */ - item_location reload_loc = item_location(); + item_location reload_loc; bool shifting_view = false; tripoint_rel_ms initial_view_offset; /** Target UI requested to abort aiming */ @@ -1428,8 +1428,8 @@ class milk_activity_actor : public activity_actor private: int total_moves {}; - std::vector monster_coords {}; - std::vector string_values {}; + std::vector monster_coords; + std::vector string_values; }; class shearing_activity_actor : public activity_actor diff --git a/src/activity_item_handling.cpp b/src/activity_item_handling.cpp index c16c0abc06cf8..da3bd75c57215 100644 --- a/src/activity_item_handling.cpp +++ b/src/activity_item_handling.cpp @@ -446,7 +446,7 @@ void put_into_vehicle_or_drop( Character &you, item_drop_reason reason, { map &here = get_map(); - return put_into_vehicle_or_drop( you, reason, items, &here, you.pos_bub( &here ) ); + put_into_vehicle_or_drop( you, reason, items, &here, you.pos_bub( &here ) ); } void put_into_vehicle_or_drop( Character &you, item_drop_reason reason, diff --git a/src/cata_bitset.cpp b/src/cata_bitset.cpp index 7d4d336c265cf..3d1a23de4d7bb 100644 --- a/src/cata_bitset.cpp +++ b/src/cata_bitset.cpp @@ -43,5 +43,6 @@ void tiny_bitset::resize_heap( size_t requested_bits ) noexcept void tiny_bitset::set_storage( block_t *data ) { + // NOLINTNEXTLINE(bugprone-multi-level-implicit-pointer-conversion) memcpy( &storage_, &data, sizeof( data ) ); -} \ No newline at end of file +} diff --git a/src/cata_bitset.h b/src/cata_bitset.h index 933c8f750ca64..680f5323c4d4b 100644 --- a/src/cata_bitset.h +++ b/src/cata_bitset.h @@ -178,6 +178,7 @@ class tiny_bitset // The hashtag blessed UB-avoiding way of type punning a pointer // out of an integer. block_t *ret; + // NOLINTNEXTLINE(bugprone-multi-level-implicit-pointer-conversion) memcpy( &ret, &storage_, sizeof( storage_ ) ); return ret; } diff --git a/src/cata_tiles.cpp b/src/cata_tiles.cpp index 1cd1da77c3b52..59a82a211d056 100644 --- a/src/cata_tiles.cpp +++ b/src/cata_tiles.cpp @@ -287,7 +287,7 @@ tileset::find_tile_type_by_season( const std::string &id, season_type season ) c } const tileset::season_tile_value &res = iter->second; if( res.season_tile ) { - return *res.season_tile; + return res.season_tile; } else if( res.default_tile ) { // can skip this check, but just in case return tile_lookup_res( iter->first, *res.default_tile ); } diff --git a/src/character_modifier.h b/src/character_modifier.h index 580924956f1cd..2c614944244db 100644 --- a/src/character_modifier.h +++ b/src/character_modifier.h @@ -60,7 +60,7 @@ struct character_modifier { mod_type limbscore_modop = MULT; std::vector> src; body_part_type::type limbtype = body_part_type::type::num_types; - translation desc = translation(); + translation desc; mod_type modtype = mod_type::NONE; float max_val = 0.0f; float min_val = 0.0f; diff --git a/src/climbing.cpp b/src/climbing.cpp index 664a96121992d..0243a539b66ed 100644 --- a/src/climbing.cpp +++ b/src/climbing.cpp @@ -214,7 +214,7 @@ void climbing_aid::down_t::deserialize( const JsonObject &jo ) jo.throw_error( str_cat( "failed to read optional member \"menu_hotkey\"" ) ); } } - if( menu_hotkey_str.length() ) { + if( !menu_hotkey_str.empty() ) { menu_hotkey = std::uint8_t( menu_hotkey_str[ 0 ] ); } diff --git a/src/construction.h b/src/construction.h index d1d64448db42a..2413f331a6581 100644 --- a/src/construction.h +++ b/src/construction.h @@ -31,7 +31,7 @@ class nc_color; struct partial_con { int counter = 0; - std::list components = {}; + std::list components; construction_id id = construction_id( -1 ); }; diff --git a/src/creature.cpp b/src/creature.cpp index 983a03da82b6f..f1aac05bd55d1 100644 --- a/src/creature.cpp +++ b/src/creature.cpp @@ -1953,7 +1953,7 @@ void Creature::schedule_effect_removal( const efftype_id &eff_id, const bodypart } void Creature::schedule_effect_removal( const efftype_id &eff_id ) { - return schedule_effect_removal( eff_id, bodypart_str_id::NULL_ID() ); + schedule_effect_removal( eff_id, bodypart_str_id::NULL_ID() ); } bool Creature::has_effect( const efftype_id &eff_id, const bodypart_id &bp ) const @@ -3436,44 +3436,44 @@ void Creature::adjust_taken_damage_by_enchantments_post_absorbed( damage_unit &d void Creature::add_msg_if_player( const translation &msg ) const { - return add_msg_if_player( msg.translated() ); + add_msg_if_player( msg.translated() ); } void Creature::add_msg_if_player( const game_message_params ¶ms, const translation &msg ) const { - return add_msg_if_player( params, msg.translated() ); + add_msg_if_player( params, msg.translated() ); } void Creature::add_msg_if_npc( const translation &msg ) const { - return add_msg_if_npc( msg.translated() ); + add_msg_if_npc( msg.translated() ); } void Creature::add_msg_if_npc( const game_message_params ¶ms, const translation &msg ) const { - return add_msg_if_npc( params, msg.translated() ); + add_msg_if_npc( params, msg.translated() ); } void Creature::add_msg_player_or_npc( const translation &pc, const translation &npc ) const { - return add_msg_player_or_npc( pc.translated(), npc.translated() ); + add_msg_player_or_npc( pc.translated(), npc.translated() ); } void Creature::add_msg_player_or_npc( const game_message_params ¶ms, const translation &pc, const translation &npc ) const { - return add_msg_player_or_npc( params, pc.translated(), npc.translated() ); + add_msg_player_or_npc( params, pc.translated(), npc.translated() ); } void Creature::add_msg_player_or_say( const translation &pc, const translation &npc ) const { - return add_msg_player_or_say( pc.translated(), npc.translated() ); + add_msg_player_or_say( pc.translated(), npc.translated() ); } void Creature::add_msg_player_or_say( const game_message_params ¶ms, const translation &pc, const translation &npc ) const { - return add_msg_player_or_say( params, pc.translated(), npc.translated() ); + add_msg_player_or_say( params, pc.translated(), npc.translated() ); } std::vector Creature::dispersion_for_even_chance_of_good_hit = { { diff --git a/src/cursesport.cpp b/src/cursesport.cpp index b04e2196cf493..a5a9793992207 100644 --- a/src/cursesport.cpp +++ b/src/cursesport.cpp @@ -189,7 +189,7 @@ void catacurses::wrefresh( const window &win ) //Refreshes the main window, causing it to redraw on top. void catacurses::refresh() { - return wrefresh( stdscr ); + wrefresh( stdscr ); } void catacurses::doupdate() @@ -349,7 +349,7 @@ void catacurses::wprintw( const window &win, const std::string &text ) return; } - return printstring( win.get(), text ); + printstring( win.get(), text ); } //Prints a formatted string to a window, moves the cursor @@ -358,7 +358,7 @@ void catacurses::mvwprintw( const window &win, const point &p, const std::string if( !wmove_internal( win, p ) ) { return; } - return printstring( win.get(), text ); + printstring( win.get(), text ); } //Resizes the underlying terminal after a Window's console resize(maybe?) Not used in TILES @@ -387,7 +387,7 @@ void catacurses::werase( const window &win_ ) //erases the main window of all text and attributes void catacurses::erase() { - return werase( stdscr ); + werase( stdscr ); } //pairs up a foreground and background color and puts it into the array of pairs @@ -410,7 +410,7 @@ void catacurses::wmove( const window &win_, const point &p ) //Clears the main window I'm not sure if its suppose to do this? void catacurses::clear() { - return wclear( stdscr ); + wclear( stdscr ); } //adds a character to the window @@ -419,7 +419,7 @@ void catacurses::mvwaddch( const window &win, const point &p, const chtype ch ) if( !wmove_internal( win, p ) ) { return; } - return waddch( win, ch ); + waddch( win, ch ); } //clears a window @@ -510,7 +510,7 @@ void catacurses::wattroff( const window &win_, nc_color ) void catacurses::waddch( const window &win, const chtype ch ) { - return printstring( win.get(), string_from_int( ch ) ); + printstring( win.get(), string_from_int( ch ) ); } static constexpr int A_BLINK = 0x00000800; /* Added characters are blinking. */ diff --git a/src/debug.cpp b/src/debug.cpp index c7c2aa881d8ff..350340c50e4df 100644 --- a/src/debug.cpp +++ b/src/debug.cpp @@ -1426,7 +1426,7 @@ void debug_write_backtrace( std::ostream &out ) if( !addresses.empty() ) { call_addr2line( last_binary_name, addresses ); } - free( funcNames ); + free( funcNames ); // NOLINT( bugprone-multi-level-implicit-pointer-conversion ) # endif #endif } diff --git a/src/dialogue.h b/src/dialogue.h index 1b56b10db96df..d95d323a8359a 100644 --- a/src/dialogue.h +++ b/src/dialogue.h @@ -197,10 +197,10 @@ struct talk_response { bool ignore_conditionals = false; mission *mission_selected = nullptr; - skill_id skill = skill_id(); - matype_id style = matype_id(); - spell_id dialogue_spell = spell_id(); - proficiency_id proficiency = proficiency_id(); + skill_id skill; + matype_id style; + spell_id dialogue_spell; + proficiency_id proficiency; talk_effect_t success; talk_effect_t failure; diff --git a/src/dialogue_chatbin.h b/src/dialogue_chatbin.h index bddaa8a45c5b1..615beda3cb2a1 100644 --- a/src/dialogue_chatbin.h +++ b/src/dialogue_chatbin.h @@ -40,11 +40,11 @@ struct dialogue_chatbin { /** * The skill this dialogue offers to train. */ - skill_id skill = skill_id(); + skill_id skill; /** * The martial art style this dialogue offers to train. */ - matype_id style = matype_id(); + matype_id style; /** * The spell this dialogue offers to train */ diff --git a/src/dialogue_helpers.h b/src/dialogue_helpers.h index e1bbb65887349..4490731324e1f 100644 --- a/src/dialogue_helpers.h +++ b/src/dialogue_helpers.h @@ -89,7 +89,7 @@ struct talk_effect_fun_t { if( !function ) { return; } - return function( d ); + function( d ); } }; diff --git a/src/effect_source.h b/src/effect_source.h index cf08e4134d615..4ac416faf9d6e 100644 --- a/src/effect_source.h +++ b/src/effect_source.h @@ -42,9 +42,9 @@ class effect_source void deserialize( const JsonObject &data ); private: - std::optional character = character_id(); - std::optional fac = faction_id(); - std::optional mfac = mfaction_id(); + std::optional character; + std::optional fac; + std::optional mfac; }; #endif // CATA_SRC_EFFECT_SOURCE_H diff --git a/src/game.cpp b/src/game.cpp index 330cb56ad8bba..b6f74ae29641a 100644 --- a/src/game.cpp +++ b/src/game.cpp @@ -6412,7 +6412,7 @@ void game::pickup() return; } // Pick up items only from the selected tile - u.pick_up( game_menus::inv::pickup( *where_ ) ); + u.pick_up( game_menus::inv::pickup( where_ ) ); } void game::pickup_all() diff --git a/src/handle_action.cpp b/src/handle_action.cpp index 532571b5d542b..9f1a64189da66 100644 --- a/src/handle_action.cpp +++ b/src/handle_action.cpp @@ -569,7 +569,7 @@ static void pldrive( const tripoint_rel_ms &p ) static void pldrive( point_rel_ms d ) { - return pldrive( tripoint_rel_ms( d, 0 ) ); + pldrive( tripoint_rel_ms( d, 0 ) ); } static void open() diff --git a/src/iexamine.cpp b/src/iexamine.cpp index 577aaea6b3c67..426951ba28093 100644 --- a/src/iexamine.cpp +++ b/src/iexamine.cpp @@ -5139,7 +5139,7 @@ static void reload_furniture( Character &you, const tripoint_bub_ms &examp, bool void iexamine::reload_furniture( Character &you, const tripoint_bub_ms &examp ) { - return reload_furniture( you, examp, true ); + reload_furniture( you, examp, true ); } void iexamine::curtains( Character &you, const tripoint_bub_ms &examp ) diff --git a/src/item_location.cpp b/src/item_location.cpp index 80dec57b98473..0a7c8bbf917f5 100644 --- a/src/item_location.cpp +++ b/src/item_location.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -841,16 +842,16 @@ void item_location::deserialize( const JsonObject &obj ) // character item locations were assumed to be on g->u who_id = get_player_character().getID(); } - ptr.reset( new impl::item_on_person( who_id, idx ) ); + ptr = std::make_shared( who_id, idx ); } else if( type == "map" ) { - ptr.reset( new impl::item_on_map( map_cursor( pos ), idx ) ); + ptr = std::make_shared( map_cursor( pos ), idx ); } else if( type == "vehicle" ) { vehicle *const veh = veh_pointer_or_null( get_map().veh_at( pos ) ); int part = obj.get_int( "part" ); if( veh && part >= 0 && part < veh->part_count() ) { - ptr.reset( new impl::item_on_vehicle( vehicle_cursor( *veh, part ), idx ) ); + ptr = std::make_shared( vehicle_cursor( *veh, part ), idx ); } } else if( type == "in_container" ) { item_location parent; @@ -858,18 +859,18 @@ void item_location::deserialize( const JsonObject &obj ) if( !parent.ptr->valid() ) { if( parent == nowhere ) { debugmsg( "parent location doesn't exist. Item_location has lost its target over a save/load cycle." ); - ptr.reset( new impl::nowhere ); + ptr = std::make_shared( ); return; } debugmsg( "parent location does not point to valid item" ); - ptr.reset( new impl::item_on_map( map_cursor( parent.pos_bub() ), idx ) ); // drop on ground + ptr = std::make_shared( map_cursor( parent.pos_bub() ), idx ); // drop on ground return; } const std::list parent_contents = parent->all_items_container_top(); if( idx > -1 && idx < static_cast( parent_contents.size() ) ) { auto iter = parent_contents.begin(); std::advance( iter, idx ); - ptr.reset( new impl::item_in_container( parent, *iter ) ); + ptr = std::make_shared( parent, *iter ); } else { // probably pointing to the wrong item debugmsg( "contents index greater than contents size" ); @@ -1059,7 +1060,7 @@ void item_location::remove_item() return; } ptr->remove_item(); - ptr.reset( new impl::nowhere() ); + ptr = std::make_shared( ); } void item_location::on_contents_changed() diff --git a/src/iuse_actor.h b/src/iuse_actor.h index 02aa253f0f52c..dfd0f7f618d8b 100644 --- a/src/iuse_actor.h +++ b/src/iuse_actor.h @@ -1161,7 +1161,7 @@ class link_up_actor : public iuse_actor translation menu_text; std::set targets = { link_state::no_link, link_state::vehicle_port }; - std::set can_extend = {}; + std::set can_extend; std::optional link_to_veh_app( Character *p, item &it, bool to_ports ) const; std::optional link_tow_cable( Character *p, item &it, bool to_towing ) const; diff --git a/src/map.cpp b/src/map.cpp index a0c8fb0485191..bc3aabcf698d6 100644 --- a/src/map.cpp +++ b/src/map.cpp @@ -1945,7 +1945,7 @@ void map::set_map_damage( const tripoint_bub_ms &p, int dmg ) debugmsg( "Called set_map_damage for unloaded submap" ); return; } - return current_submap->set_map_damage( l, dmg ); + current_submap->set_map_damage( l, dmg ); } uint8_t map::get_known_connections( const tripoint_bub_ms &p, diff --git a/src/map.h b/src/map.h index 0bed532f08ef5..dc63f16debaf2 100644 --- a/src/map.h +++ b/src/map.h @@ -2395,20 +2395,20 @@ class tinymap : private map map::i_rem( rebase_bub( p ), it ); } void i_clear( const tripoint_omt_ms &p ) { - return map::i_clear( rebase_bub( p ) ); + map::i_clear( rebase_bub( p ) ); } bool add_field( const tripoint_omt_ms &p, const field_type_id &type_id, int intensity = INT_MAX, const time_duration &age = 0_turns, bool hit_player = true ) { return map::add_field( rebase_bub( p ), type_id, intensity, age, hit_player ); } void delete_field( const tripoint_omt_ms &p, const field_type_id &field_to_remove ) { - return map::delete_field( rebase_bub( p ), field_to_remove ); + map::delete_field( rebase_bub( p ), field_to_remove ); } bool has_flag( ter_furn_flag flag, const tripoint_omt_ms &p ) const { return map::has_flag( flag, rebase_bub( p ) ); } void destroy( const tripoint_omt_ms &p, bool silent = false ) { - return map::destroy( rebase_bub( p ), silent ); + map::destroy( rebase_bub( p ), silent ); } const trap &tr_at( const tripoint_omt_ms &p ) const { return map::tr_at( rebase_bub( p ) ); @@ -2436,7 +2436,7 @@ class tinymap : private map } void add_splatter_trail( const field_type_id &type, const tripoint_omt_ms &from, const tripoint_omt_ms &to ) { - return map::add_splatter_trail( type, rebase_bub( from ), rebase_bub( to ) ); + map::add_splatter_trail( type, rebase_bub( from ), rebase_bub( to ) ); } void collapse_at( const tripoint_omt_ms &p, bool silent, bool was_supporting = false, diff --git a/src/math_parser_diag.cpp b/src/math_parser_diag.cpp index 1499de245330d..c0a22dd5393c9 100644 --- a/src/math_parser_diag.cpp +++ b/src/math_parser_diag.cpp @@ -165,7 +165,7 @@ diag_assign_dbl_f addiction_turns_ass( char scope, std::vector const diag_kwargs const & /* kwargs */ ) { return[ beta = is_beta( scope ), add_value = params[0]]( dialogue const & d, double val ) { - return d.actor( beta )->set_addiction_turns( addiction_id( add_value.str( d ) ), val ); + d.actor( beta )->set_addiction_turns( addiction_id( add_value.str( d ) ), val ); }; } @@ -182,7 +182,7 @@ diag_assign_dbl_f health_ass( char scope, std::vector const & /* par { return [beta = is_beta( scope )]( dialogue const & d, double val ) { const int current_health = d.actor( beta )->get_health(); - return d.actor( beta )->mod_livestyle( val - current_health ); + d.actor( beta )->mod_livestyle( val - current_health ); }; } @@ -1111,7 +1111,7 @@ diag_assign_dbl_f skill_ass( char scope, std::vector const ¶ms, diag_kwargs const & /* kwargs */ ) { return [beta = is_beta( scope ), sid = params[0] ]( dialogue const & d, double val ) { - return d.actor( beta )->set_skill_level( skill_id( sid.str( d ) ), val ); + d.actor( beta )->set_skill_level( skill_id( sid.str( d ) ), val ); }; } @@ -1144,7 +1144,7 @@ diag_assign_dbl_f skill_exp_ass( char scope, std::vector const ¶ throw math::runtime_error( R"(Unknown format type "%s" for skill_exp)", format ); } bool raw = format == "raw"; - return d.actor( beta )->set_skill_exp( skill, val, raw ); + d.actor( beta )->set_skill_exp( skill, val, raw ); }; } @@ -1215,7 +1215,7 @@ diag_assign_dbl_f spell_exp_ass( char scope, std::vector const ¶ diag_kwargs const & /* kwargs */ ) { return[beta = is_beta( scope ), sid = params[0]]( dialogue const & d, double val ) { - return d.actor( beta )->set_spell_exp( spell_id( sid.str( d ) ), val ); + d.actor( beta )->set_spell_exp( spell_id( sid.str( d ) ), val ); }; } @@ -1638,7 +1638,7 @@ diag_assign_dbl_f npc_anger_ass( char scope, std::vector const & /* diag_kwargs const & /* kwargs */ ) { return[beta = is_beta( scope )]( dialogue const & d, double val ) { - return d.actor( beta )->set_npc_anger( val ); + d.actor( beta )->set_npc_anger( val ); }; } @@ -1646,7 +1646,7 @@ diag_assign_dbl_f npc_fear_ass( char scope, std::vector const & /* p diag_kwargs const & /* kwargs */ ) { return[beta = is_beta( scope )]( dialogue const & d, double val ) { - return d.actor( beta )->set_npc_fear( val ); + d.actor( beta )->set_npc_fear( val ); }; } @@ -1654,7 +1654,7 @@ diag_assign_dbl_f npc_value_ass( char scope, std::vector const & /* diag_kwargs const & /* kwargs */ ) { return[beta = is_beta( scope )]( dialogue const & d, double val ) { - return d.actor( beta )->set_npc_value( val ); + d.actor( beta )->set_npc_value( val ); }; } @@ -1662,7 +1662,7 @@ diag_assign_dbl_f npc_trust_ass( char scope, std::vector const & /* diag_kwargs const & /* kwargs */ ) { return[beta = is_beta( scope )]( dialogue const & d, double val ) { - return d.actor( beta )->set_npc_trust( val ); + d.actor( beta )->set_npc_trust( val ); }; } diff --git a/src/melee.cpp b/src/melee.cpp index 182d66f374e21..97d560050a890 100644 --- a/src/melee.cpp +++ b/src/melee.cpp @@ -1816,7 +1816,7 @@ void Character::perform_technique( const ma_technique &technique, Creature &t, new_.y() = b.y(); } - const tripoint_bub_ms &dest{ new_, b.z()}; + const tripoint_bub_ms dest{ new_, b.z()}; if( g->is_empty( dest ) ) { t.setpos( dest ); } diff --git a/src/mission_util.cpp b/src/mission_util.cpp index 1326e99734aca..93794b46b73e2 100644 --- a/src/mission_util.cpp +++ b/src/mission_util.cpp @@ -180,7 +180,7 @@ static std::optional find_or_create_om_terrain( tripoint_abs_omt target_pos = tripoint_abs_omt::invalid; if( params.target_var.has_value() ) { - return project_to( get_tripoint_ms_from_var( params.target_var.value(), d, false ) ); + return project_to( get_tripoint_ms_from_var( params.target_var, d, false ) ); } omt_find_params find_params; diff --git a/src/morale.cpp b/src/morale.cpp index ace8640eb3c29..6a6ff03c00f1b 100644 --- a/src/morale.cpp +++ b/src/morale.cpp @@ -292,31 +292,31 @@ player_morale::player_morale() : mutations[trait_OPTIMISTIC] = mutation_data( [set_optimist]( player_morale * pm ) { - return set_optimist( pm, 9 ); + set_optimist( pm, 9 ); }, [set_optimist]( player_morale * pm ) { - return set_optimist( pm, 0 ); + set_optimist( pm, 0 ); } ); mutations[trait_BADTEMPER] = mutation_data( [set_badtemper]( player_morale * pm ) { - return set_badtemper( pm, -9 ); + set_badtemper( pm, -9 ); }, [set_badtemper]( player_morale * pm ) { - return set_badtemper( pm, 0 ); + set_badtemper( pm, 0 ); } ); mutations[trait_NUMB] = mutation_data( [set_numb]( player_morale * pm ) { - return set_numb( pm, -1 ); + set_numb( pm, -1 ); }, [set_numb]( player_morale * pm ) { - return set_numb( pm, 0 ); + set_numb( pm, 0 ); } ); mutations[trait_STYLISH] = mutation_data( [set_stylish]( player_morale * pm ) { - return set_stylish( pm, true ); + set_stylish( pm, true ); }, [set_stylish]( player_morale * pm ) { - return set_stylish( pm, false ); + set_stylish( pm, false ); } ); mutations[trait_FLOWERS] = mutation_data( update_constrained ); mutations[trait_ROOTS1] = mutation_data( update_constrained ); diff --git a/src/npc.h b/src/npc.h index a6addfff76858..82aecd66432a6 100644 --- a/src/npc.h +++ b/src/npc.h @@ -306,6 +306,7 @@ const std::unordered_map cbm_reserve_strs = { { } }; +// NOLINTBEGIN(optin.core.EnumCastOutOfRange) enum class ally_rule : int { DEFAULT = 0, use_guns = 1, @@ -327,6 +328,7 @@ enum class ally_rule : int { lock_doors = 65536, avoid_locks = 131072 }; +// NOLINTEND(optin.core.EnumCastOutOfRange) struct ally_rule_data { ally_rule rule; @@ -1343,7 +1345,7 @@ class npc : public Character int last_seen_player_turn = 0; // Timeout to forgetting //Safe reference to an item at a specific location in case it gets deleted before pickup - item_location wanted_item = {}; + item_location wanted_item; tripoint_bub_ms wanted_item_pos; // The square containing an item we want // These are the coordinates that a guard will return to inside of their goal tripoint diff --git a/src/npctalk.h b/src/npctalk.h index e8594dfff655a..e9c971dd51157 100644 --- a/src/npctalk.h +++ b/src/npctalk.h @@ -18,10 +18,10 @@ namespace talk_function { struct teach_domain { - skill_id skill = skill_id(); - matype_id style = matype_id(); - spell_id spell = spell_id(); - proficiency_id prof = proficiency_id(); + skill_id skill; + matype_id style; + spell_id spell; + proficiency_id prof; }; void nothing( npc & ); diff --git a/src/overmap_ui.h b/src/overmap_ui.h index 2f7bc6d4f9d33..f935e9bf7f64e 100644 --- a/src/overmap_ui.h +++ b/src/overmap_ui.h @@ -129,7 +129,7 @@ struct overmap_draw_data_t { // draw zone location. tripoint_abs_omt select = tripoint_abs_omt( -1, -1, -1 ); int iZoneIndex = -1; - std::vector display_path = {}; + std::vector display_path; //center of UI view; usually player OMT position tripoint_abs_omt origin_pos = tripoint_abs_omt( -1, -1, -1 ); /** diff --git a/src/overmapbuffer.cpp b/src/overmapbuffer.cpp index 25f1fad4957b3..2d39fc80d20a5 100644 --- a/src/overmapbuffer.cpp +++ b/src/overmapbuffer.cpp @@ -821,7 +821,7 @@ const oter_id &overmapbuffer::ter_existing( const tripoint_abs_omt &p ) void overmapbuffer::ter_set( const tripoint_abs_omt &p, const oter_id &id ) { const overmap_with_local_coords om_loc = get_om_global( p ); - return om_loc.om->ter_set( om_loc.local, id ); + om_loc.om->ter_set( om_loc.local, id ); } std::optional *overmapbuffer::mapgen_args( const tripoint_abs_omt &p ) diff --git a/src/text_style_check.h b/src/text_style_check.h index 136af2372fd3d..c76a8e4fee841 100644 --- a/src/text_style_check.h +++ b/src/text_style_check.h @@ -55,6 +55,7 @@ void text_style_check( Iter beg, Iter end, // remove unnecessary spaces before the symbol size_t fix_before_max = 0; } spaces; + // NOLINTBEGIN(readability-redundant-member-init) struct { bool yes = false; std::string str {}; @@ -62,6 +63,7 @@ void text_style_check( Iter beg, Iter end, std::string sym_desc {}; std::string replace_desc {}; } replace; + // NOLINTEND(readability-redundant-member-init) }; // always put the longest (in u32) symbols at the front, since we'll iterate // and search for them in this order. diff --git a/src/trap.cpp b/src/trap.cpp index 3f2fd088204be..58c1ceb7bbc39 100644 --- a/src/trap.cpp +++ b/src/trap.cpp @@ -338,12 +338,12 @@ void trap::trigger( const tripoint_bub_ms &pos ) const void trap::trigger( const tripoint_bub_ms &pos, Creature &creature ) const { - return trigger( pos, &creature, nullptr ); + trigger( pos, &creature, nullptr ); } void trap::trigger( const tripoint_bub_ms &pos, item &item ) const { - return trigger( pos, nullptr, &item ); + trigger( pos, nullptr, &item ); } void trap::trigger( const tripoint_bub_ms &pos, Creature *creature, item *item ) const diff --git a/src/veh_type.h b/src/veh_type.h index e7c0e19a973e5..3765b5d088eb7 100644 --- a/src/veh_type.h +++ b/src/veh_type.h @@ -360,7 +360,7 @@ class vpart_info item_group_id breaks_into_group = item_group_id( "EMPTY_GROUP" ); /** Flat decrease of damage of a given type. */ - std::unordered_map damage_reduction = {}; + std::unordered_map damage_reduction; /** Tool qualities this vehicle part can provide when installed */ std::map qualities; diff --git a/src/wish.cpp b/src/wish.cpp index 2896da4161bb0..e97fbe6248c6d 100644 --- a/src/wish.cpp +++ b/src/wish.cpp @@ -826,7 +826,7 @@ void debug_menu::wishmonster( const std::optional &p ) wmenu.query(); if( wmenu.ret >= 0 ) { const mtype_id &mon_type = mtypes[ wmenu.ret ]->id; - if( std::optional spawn = p ? p.value() : g->look_around() ) { + if( std::optional spawn = p.has_value() ? p : g->look_around() ) { int num_spawned = 0; for( const tripoint_bub_ms &destination : closest_points_first( *spawn, cb.group ) ) { monster *const mon = g->place_critter_at( mon_type, destination ); diff --git a/tests/catch/catch.hpp b/tests/catch/catch.hpp index 860b5bf6fddbf..1c09a9e2114b6 100644 --- a/tests/catch/catch.hpp +++ b/tests/catch/catch.hpp @@ -2703,7 +2703,7 @@ namespace Catch { INTERNAL_CATCH_TRY { \ CATCH_INTERNAL_START_WARNINGS_SUPPRESSION \ CATCH_INTERNAL_SUPPRESS_PARENTHESES_WARNINGS \ - catchAssertionHandler.handleExpr( Catch::Decomposer() <= __VA_ARGS__ ); \ + catchAssertionHandler.handleExpr( Catch::Decomposer() <= __VA_ARGS__ ); /* NOLINT(bugprone-chained-comparison) */ \ CATCH_INTERNAL_STOP_WARNINGS_SUPPRESSION \ } INTERNAL_CATCH_CATCH( catchAssertionHandler ) \ INTERNAL_CATCH_REACT( catchAssertionHandler ) \ diff --git a/tools/clang-tidy-plugin/CombineLocalsIntoPointCheck.cpp b/tools/clang-tidy-plugin/CombineLocalsIntoPointCheck.cpp index ac66c67363602..a5e4498d597b8 100644 --- a/tools/clang-tidy-plugin/CombineLocalsIntoPointCheck.cpp +++ b/tools/clang-tidy-plugin/CombineLocalsIntoPointCheck.cpp @@ -91,7 +91,7 @@ static void CheckDecl( CombineLocalsIntoPointCheck &Check, const MatchFinder::Ma const VarDecl *ZDecl = dyn_cast_or_null( NextDecl ); // Avoid altering bool variables - if( StringRef( XDecl->getType().getAsString() ).endswith( "ool" ) ) { + if( StringRef( XDecl->getType().getAsString() ).ends_with( "ool" ) ) { return; } diff --git a/tools/clang-tidy-plugin/HeaderGuardCheck.cpp b/tools/clang-tidy-plugin/HeaderGuardCheck.cpp index a17d0b8a969f8..c3c6511de253b 100644 --- a/tools/clang-tidy-plugin/HeaderGuardCheck.cpp +++ b/tools/clang-tidy-plugin/HeaderGuardCheck.cpp @@ -107,7 +107,7 @@ class HeaderGuardPPCallbacks : public PPCallbacks std::string GetFileName( SourceLocation Loc ) { SourceManager &SM = PP->getSourceManager(); FileID Id = SM.getFileID( Loc ); - if( const FileEntry *Entry = SM.getFileEntryForID( Id ) ) { + if( const OptionalFileEntryRef Entry = SM.getFileEntryRefForID( Id ) ) { return cleanPath( Entry->getName() ); } else { return {}; diff --git a/tools/clang-tidy-plugin/NoLongCheck.cpp b/tools/clang-tidy-plugin/NoLongCheck.cpp index d9a13659ebef9..a7d81feed52c7 100644 --- a/tools/clang-tidy-plugin/NoLongCheck.cpp +++ b/tools/clang-tidy-plugin/NoLongCheck.cpp @@ -123,7 +123,7 @@ static void CheckDecl( NoLongCheck &Check, const MatchFinder::MatchResult &Resul if( alternatives.empty() ) { return; } - if( MatchedDecl->getName().startswith( "__" ) ) { + if( MatchedDecl->getName().starts_with( "__" ) ) { // Can happen for e.g. compiler-generated code inside an implicitly // generated function return; diff --git a/tools/clang-tidy-plugin/SimplifyPointConstructorsCheck.cpp b/tools/clang-tidy-plugin/SimplifyPointConstructorsCheck.cpp index 65b82d3cf3117..6da0d635e8679 100644 --- a/tools/clang-tidy-plugin/SimplifyPointConstructorsCheck.cpp +++ b/tools/clang-tidy-plugin/SimplifyPointConstructorsCheck.cpp @@ -54,7 +54,7 @@ struct ExpressionCategory { ExpressionCategory( const MatchFinder::MatchResult &Result, const Expr *E ) : Replacement( getText( Result, E ) ) { - if( StringRef( Replacement ).endswith( "->" ) ) { + if( StringRef( Replacement ).ends_with( "->" ) ) { Replacement.erase( Replacement.end() - 2, Replacement.end() ); } QualType EType = E->getType(); diff --git a/tools/clang-tidy-plugin/StaticIntIdConstantsCheck.cpp b/tools/clang-tidy-plugin/StaticIntIdConstantsCheck.cpp index cddce4e8d8146..dd1c2fdcb9c92 100644 --- a/tools/clang-tidy-plugin/StaticIntIdConstantsCheck.cpp +++ b/tools/clang-tidy-plugin/StaticIntIdConstantsCheck.cpp @@ -40,7 +40,7 @@ static void CheckConstructor( StaticIntIdConstantsCheck &Check, } StringRef VarName = IntIdVarDecl->getName(); - if( VarName.endswith( "null" ) || VarName.endswith( "NULL" ) ) { + if( VarName.ends_with( "null" ) || VarName.ends_with( "NULL" ) ) { // Null constants are OK because they probably don't vary return; } diff --git a/tools/clang-tidy-plugin/StaticStringIdConstantsCheck.cpp b/tools/clang-tidy-plugin/StaticStringIdConstantsCheck.cpp index a33e2b545422a..2ccd5ff880367 100644 --- a/tools/clang-tidy-plugin/StaticStringIdConstantsCheck.cpp +++ b/tools/clang-tidy-plugin/StaticStringIdConstantsCheck.cpp @@ -103,7 +103,7 @@ static std::string GetPrefixFor( const CXXRecordDecl *Type, StaticStringIdConsta for( const char *Suffix : { "_type", "_info" } ) { - if( StringRef( TypeName ).endswith( Suffix ) ) { + if( StringRef( TypeName ).ends_with( Suffix ) ) { TypeName.erase( TypeName.end() - strlen( Suffix ), TypeName.end() ); } } @@ -123,7 +123,7 @@ static std::string GetCanonicalName( const CXXRecordDecl *Type, const StringRef } static const std::string anon_prefix = "_anonymous_namespace___"; - if( StringRef( Result ).startswith( anon_prefix ) ) { + if( StringRef( Result ).starts_with( anon_prefix ) ) { Result.erase( Result.begin(), Result.begin() + anon_prefix.size() ); } @@ -200,7 +200,7 @@ void StaticStringIdConstantsCheck::CheckConstructor( const MatchFinder::MatchRes std::string CurrentName = VarDeclParent->getNameAsString(); if( CurrentName != CanonicalName && !PreviousDeclIsExtern && - !StringRef( CurrentName ).startswith( "fuel_type_" ) ) { + !StringRef( CurrentName ).starts_with( "fuel_type_" ) ) { SourceRange Range = DeclarationNameInfo( VarDeclParent->getDeclName(), VarDeclParent->getLocation() ).getSourceRange(); @@ -528,7 +528,7 @@ static void CheckDeclRef( StaticStringIdConstantsCheck &Check, std::string CurrentName = VarDeclParent->getNameAsString(); if( CurrentName != CanonicalName && - !StringRef( CurrentName ).startswith( "fuel_type_" ) ) { + !StringRef( CurrentName ).starts_with( "fuel_type_" ) ) { Check.diag( Ref->getBeginLoc(), "Use of string_id %0 should be named '%1'." diff --git a/tools/clang-tidy-plugin/TestFilenameCheck.cpp b/tools/clang-tidy-plugin/TestFilenameCheck.cpp index 813f812dca4b1..75c26370d871b 100644 --- a/tools/clang-tidy-plugin/TestFilenameCheck.cpp +++ b/tools/clang-tidy-plugin/TestFilenameCheck.cpp @@ -36,7 +36,7 @@ class TestFilenameCallbacks : public PPCallbacks if( MacroName == "TEST_CASE" ) { StringRef Filename = SM->getBufferName( Range.getBegin() ); - bool IsTestFilename = Filename.endswith( "_test.cpp" ); + bool IsTestFilename = Filename.ends_with( "_test.cpp" ); if( !IsTestFilename ) { Check->diag( Range.getBegin(), diff --git a/tools/clang-tidy-plugin/UnusedStaticsCheck.cpp b/tools/clang-tidy-plugin/UnusedStaticsCheck.cpp index c9ec67a481b5b..ac2970b40d7b1 100644 --- a/tools/clang-tidy-plugin/UnusedStaticsCheck.cpp +++ b/tools/clang-tidy-plugin/UnusedStaticsCheck.cpp @@ -48,7 +48,7 @@ void UnusedStaticsCheck::check( const MatchFinder::MatchResult &Result ) // Ignore cases that are not static linkage Linkage Lnk = ThisDecl->getFormalLinkage(); - if( Lnk != InternalLinkage && Lnk != UniqueExternalLinkage ) { + if( Lnk != Linkage::Internal && Lnk != Linkage::UniqueExternal ) { return; } diff --git a/tools/clang-tidy-plugin/UseLocalizedSortingCheck.cpp b/tools/clang-tidy-plugin/UseLocalizedSortingCheck.cpp index d86a04f6d417e..7360930eb229e 100644 --- a/tools/clang-tidy-plugin/UseLocalizedSortingCheck.cpp +++ b/tools/clang-tidy-plugin/UseLocalizedSortingCheck.cpp @@ -128,7 +128,7 @@ static void CheckOpCall( UseLocalizedSortingCheck &Check, const MatchFinder::Mat } StringRef Arg0Text = getText( Result, Arg0Expr ); - if( Arg0Text.endswith( "id" ) ) { + if( Arg0Text.ends_with( "id" ) ) { return; } diff --git a/tools/clang-tidy-plugin/UsePointArithmeticCheck.cpp b/tools/clang-tidy-plugin/UsePointArithmeticCheck.cpp index 4dd8cc64a2b78..89f4d6bc5de7e 100644 --- a/tools/clang-tidy-plugin/UsePointArithmeticCheck.cpp +++ b/tools/clang-tidy-plugin/UsePointArithmeticCheck.cpp @@ -105,7 +105,7 @@ struct ExpressionComponent { } void complete_init() { - if( StringRef( objectRef ).endswith( "->" ) ) { + if( StringRef( objectRef ).ends_with( "->" ) ) { objectRef.erase( objectRef.end() - 2, objectRef.end() ); } } diff --git a/tools/clang-tidy-plugin/Utils.cpp b/tools/clang-tidy-plugin/Utils.cpp index fde9b829610f7..7ab45c1c8060a 100644 --- a/tools/clang-tidy-plugin/Utils.cpp +++ b/tools/clang-tidy-plugin/Utils.cpp @@ -16,19 +16,19 @@ bool isPointMethod( const FunctionDecl *d ) NameConvention::NameConvention( StringRef xName ) { - if( xName.endswith( "x" ) ) { + if( xName.ends_with( "x" ) ) { root = xName.drop_back().str(); capital = false; atEnd = true; - } else if( xName.endswith( "X" ) ) { + } else if( xName.ends_with( "X" ) ) { root = xName.drop_back().str(); capital = true; atEnd = true; - } else if( xName.startswith( "x" ) ) { + } else if( xName.starts_with( "x" ) ) { root = xName.drop_front().str(); capital = false; atEnd = false; - } else if( xName.startswith( "X" ) ) { + } else if( xName.starts_with( "X" ) ) { root = xName.drop_front().str(); capital = true; atEnd = false; diff --git a/tools/clang-tidy-plugin/Utils.h b/tools/clang-tidy-plugin/Utils.h index c88b4b936208f..0c22f6f21ffcb 100644 --- a/tools/clang-tidy-plugin/Utils.h +++ b/tools/clang-tidy-plugin/Utils.h @@ -89,7 +89,7 @@ inline bool isInHeader( const SourceLocation &loc, const SourceManager &SM ) StringRef Filename = SM.getFilename( loc ); // The .h.tmp.cpp catches the test case; that's the style of filename used // by lit. - return !SM.isInMainFile( loc ) || Filename.endswith( ".h.tmp.cpp" ); + return !SM.isInMainFile( loc ) || Filename.ends_with( ".h.tmp.cpp" ); } inline bool isPointType( const CXXRecordDecl *R )