Skip to content

Commit 26712e3

Browse files
authored
Upgrade clang-tidy to LLVM 18, and fix (some) newly firing lints (CleverRaven#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::<T>(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<impl::item_on_person> * 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<faction_id> 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<abstract_var_info<std::basic_string<char>>>' into 'abstract_var_info<std::basic_string<char>>' and back into 'std::optional<abstract_var_info<std::basic_string<char>>>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion,-warnings-as-errors] 182 | return project_to<coords::omt>( 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<coords::omt>( 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 catchorg/Catch2@1078e7e 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<tripoint, coords::origin::reality_bubble, coords::scale::map_square>'), 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<E>( static_cast<I>( l ) | static_cast<I>( 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|<part_status_flag, void>' 3124 | static_cast<part_status_flag>( 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<E>( static_cast<I>( l ) | static_cast<I>( 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<void*>` 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_ ) ); |
1 parent 723f7d2 commit 26712e3

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

51 files changed

+230
-210
lines changed

.clang-tidy

+110-97
Original file line numberDiff line numberDiff line change
@@ -4,104 +4,117 @@
44
# this codebase and we do not intend to fix. The disabled checks appearing
55
# thereafter in a separate alphabetical list have yet to be triaged. We may
66
# fix their errors or recategorise them as checks we don't care about.
7-
#
8-
# Comments on the checks we have decided are not worthwhile:
9-
#
10-
# * bugprone-throw-keyword-missing
11-
# This check is too time consuming. Disable it for now to save CI time.
12-
#
13-
# * cert-dcl21-cpp (postfix operator++ and operator-- should return const objects)
14-
# This is an unconventional code style, and conflicts with
15-
# readability-const-return-type.
16-
#
17-
# * cert-env33-c (calls to system, popen)
18-
# Unlikely to catch bugs, and using system is convenient for portability.
19-
#
20-
# * cert-dcl37-c and cert-dcl-51-cpp (reserved identifiers)
21-
# These two checks are aliases for bugprone-reserved-identifier.
22-
# Don't repeatedly run the same check for three times.
23-
#
24-
# * cert-err58-cpp (exceptions from static variable declarations)
25-
# We have lots of memory allocations in static variable declarations, and
26-
# that's fine.
27-
#
28-
# * clang-analyzer-core.{DivideZero,NonNullParamChecker,UndefinedBinaryOperatorResult}
29-
# * clang-analyzer-cplusplus.NewDelete
30-
# They report too many false positives.
31-
#
32-
# * modernize-use-auto
33-
# We prefer an almost-always-avoid-auto style.
34-
#
35-
# * modernize-use-trailing-return-type
36-
# An arbitrary style convention we haven't adopted.
37-
#
38-
# * readability-identifier-naming
39-
# We are not enforcing a standard identifier naming scheme in the code base.
40-
# This check does not bring much value at the moment and consumes a lot of CPU time.
417

42-
Checks: "\
43-
bugprone-*,\
44-
cata-*,\
45-
cert-*,\
46-
-cert-dcl21-cpp,\
47-
-cert-env33-c,\
48-
-cert-dcl37-c,\
49-
-cert-dcl51-cpp,\
50-
-cert-err58-cpp,\
51-
-clang-analyzer-core.CallAndMessage,\
52-
-clang-analyzer-core.DivideZero,\
53-
-clang-analyzer-core.NonNullParamChecker,\
54-
-clang-analyzer-core.UndefinedBinaryOperatorResult,\
55-
-clang-analyzer-cplusplus.NewDelete,\
56-
clang-diagnostic-*,\
57-
cppcoreguidelines-slicing,\
58-
google-explicit-constructor,\
59-
llvm-namespace-comment,\
60-
misc-*,\
61-
modernize-*,\
62-
-modernize-use-auto,\
63-
-modernize-use-trailing-return-type,\
64-
performance-*,\
65-
readability-*,\
66-
-bugprone-assignment-in-if-condition,\
67-
-bugprone-easily-swappable-parameters,\
68-
-bugprone-empty-catch,\
69-
-bugprone-implicit-widening-of-multiplication-result,\
70-
-bugprone-narrowing-conversions,\
71-
-bugprone-switch-missing-default-case,\
72-
-bugprone-throw-keyword-missing,\
73-
-bugprone-unchecked-optional-access,\
74-
-bugprone-unhandled-exception-at-new,\
75-
-misc-confusable-identifiers,\
76-
-misc-const-correctness,\
77-
-misc-header-include-cycle,\
78-
-misc-include-cleaner,\
79-
-misc-no-recursion,\
80-
-misc-non-private-member-variables-in-classes,\
81-
-misc-use-anonymous-namespace,\
82-
-modernize-concat-nested-namespaces,\
83-
-modernize-macro-to-enum,\
84-
-modernize-pass-by-value,\
85-
-modernize-return-braced-init-list,\
86-
-modernize-use-default-member-init,\
87-
-modernize-use-nodiscard,\
88-
-performance-avoid-endl,\
89-
-performance-noexcept-swap,\
90-
-performance-no-automatic-move,\
91-
-readability-avoid-unconditional-preprocessor-if,\
92-
-readability-container-data-pointer,\
93-
-readability-convert-member-functions-to-static,\
94-
-readability-else-after-return,\
95-
-readability-function-cognitive-complexity,\
96-
-readability-identifier-length,\
97-
-readability-identifier-naming,\
98-
-readability-implicit-bool-conversion,\
99-
-readability-magic-numbers,\
100-
-readability-named-parameter,\
101-
-readability-simplify-boolean-expr,\
102-
-readability-suspicious-call-argument,\
103-
-readability-use-anyofallof,\
104-
"
8+
Checks: [
9+
bugprone-*,
10+
# This check is too time consuming. Disable it for now to save CI time.
11+
-bugprone-throw-keyword-missing,
12+
cata-*,
13+
cert-*,
14+
# * cert-dcl21-cpp (postfix operator++ and operator-- should return const objects)
15+
# This is an unconventional code style, and conflicts with
16+
# readability-const-return-type.
17+
-cert-dcl21-cpp,
18+
# * cert-env33-c (calls to system, popen)
19+
# Unlikely to catch bugs, and using system is convenient for portability.
20+
-cert-env33-c,
21+
# * cert-dcl37-c and cert-dcl-51-cpp (reserved identifiers)
22+
# These two checks are aliases for bugprone-reserved-identifier.
23+
# Don't repeatedly run the same check for three times.
24+
-cert-dcl37-c,
25+
-cert-dcl51-cpp,
26+
# * cert-err58-cpp (exceptions from static variable declarations)
27+
# We have lots of memory allocations in static variable declarations, and
28+
# that's fine.
29+
-cert-err58-cpp,
30+
# * clang-analyzer-core.{DivideZero,NonNullParamChecker,UndefinedBinaryOperatorResult}
31+
# * clang-analyzer-cplusplus.NewDelete
32+
# They report too many false positives.
33+
-clang-analyzer-core.CallAndMessage,
34+
-clang-analyzer-core.DivideZero,
35+
-clang-analyzer-core.NonNullParamChecker,
36+
-clang-analyzer-core.UndefinedBinaryOperatorResult,
37+
-clang-analyzer-cplusplus.NewDelete,
38+
# We often use enums as bitsets and do things like `enum::A | enum::B`
39+
# which is explicitly unsupported by this check, and it is officially recommended to disable
40+
# this lint for project that use such patterns.
41+
-clang-analyzer-optin.core.EnumCastOutOfRange,
42+
clang-diagnostic-*,
43+
cppcoreguidelines-slicing,
44+
google-explicit-constructor,
45+
llvm-namespace-comment,
46+
misc-*,
47+
# Extremely expensive, and we are not using `using` anyway, so it's not catching anything.
48+
-misc-unused-using-decls,
49+
modernize-*,
50+
# Rather expensive check, and it is unlikely that somebody
51+
# would *want* to use std::auto_ptr in %CURRENT_YEAR% (2025+) when unique_ptr is both better
52+
# and is a de-facto default in the codebase already.
53+
-modernize-replace-auto-ptr,
54+
# * modernize-use-auto
55+
# We prefer an almost-always-avoid-auto style.
56+
-modernize-use-auto,
57+
# * modernize-use-trailing-return-type
58+
# An arbitrary style convention we haven't adopted.
59+
-modernize-use-trailing-return-type,
60+
performance-*,
61+
readability-*,
62+
# * readability-identifier-naming
63+
# We are not enforcing a standard identifier naming scheme in the code base.
64+
# This check does not bring much value at the moment and consumes a lot of CPU time.
65+
-readability-identifier-length,
66+
-readability-identifier-naming,
67+
# disabled due to behaviour change between pre-module and post-module world.
68+
# Reevaluate in 2027(?) when the code is sufficiently migrated to C++20 modules
69+
# or when there is a decision not perform the migration
70+
-readability-redundant-inline-specifier,
71+
72+
# ==== Untriaged checks follow ====
73+
# Either fix the code and re-enable the check,
74+
# or add a good comment and move to the appropriate section above.
75+
# Silencing the existing errors with a //NOLINT does count as a "fix", as that still
76+
# prevents new issues from cropping up.
77+
-bugprone-assignment-in-if-condition,
78+
-bugprone-easily-swappable-parameters,
79+
-bugprone-empty-catch,
80+
-bugprone-implicit-widening-of-multiplication-result,
81+
-bugprone-narrowing-conversions,
82+
-bugprone-switch-missing-default-case,
83+
-bugprone-unchecked-optional-access,
84+
-bugprone-unhandled-exception-at-new,
85+
-bugprone-unused-local-non-trivial-variable, # great lint, but hard to fix
86+
-misc-confusable-identifiers,
87+
-misc-const-correctness,
88+
-misc-header-include-cycle,
89+
-misc-include-cleaner,
90+
-misc-no-recursion,
91+
-misc-non-private-member-variables-in-classes,
92+
-misc-use-anonymous-namespace,
93+
-modernize-concat-nested-namespaces,
94+
-modernize-macro-to-enum,
95+
-modernize-pass-by-value,
96+
-modernize-return-braced-init-list,
97+
-modernize-use-default-member-init,
98+
-modernize-use-nodiscard,
99+
-performance-avoid-endl,
100+
-performance-enum-size, # untriaged since upgrade to LLVM 18
101+
-performance-noexcept-swap,
102+
-performance-no-automatic-move,
103+
-readability-avoid-nested-conditional-operator, # great lint, but hard to fix
104+
-readability-avoid-unconditional-preprocessor-if,
105+
-readability-container-data-pointer,
106+
-readability-convert-member-functions-to-static,
107+
-readability-else-after-return,
108+
-readability-function-cognitive-complexity,
109+
-readability-implicit-bool-conversion,
110+
-readability-magic-numbers,
111+
-readability-named-parameter,
112+
-readability-redundant-casting, # probably good lint, just needs fixing
113+
-readability-simplify-boolean-expr,
114+
-readability-suspicious-call-argument,
115+
-readability-use-anyofallof,
116+
]
117+
105118
WarningsAsErrors: '*'
106119
HeaderFilterRegex: '(src|test|tools).*'
107120
FormatStyle: none

.github/workflows/clang-tidy.yml

+8-8
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
name: Clang-tidy 17
1+
name: Clang-tidy 18
22

33
on:
44
push:
@@ -36,13 +36,13 @@ jobs:
3636
fail-fast: true
3737
runs-on: ubuntu-24.04
3838
env:
39-
COMPILER: clang++-17
39+
COMPILER: clang++-18
4040
steps:
41-
- name: install LLVM 17
41+
- name: install LLVM 18
4242
if: ${{ needs.skip-duplicates.outputs.should_skip != 'true' && github.event_name != 'pull_request' || github.event.pull_request.draft == false }}
4343
run: |
4444
sudo apt-get update
45-
sudo apt install llvm-17 llvm-17-dev llvm-17-tools clang-17 clang-tidy-17 clang-tools-17 libclang-17-dev
45+
sudo apt install llvm-18 llvm-18-dev llvm-18-tools clang-18 clang-tidy-18 clang-tools-18 libclang-18-dev
4646
sudo apt install python3-pip ninja-build cmake
4747
pip3 install --user lit
4848
- name: checkout repository
@@ -74,8 +74,8 @@ jobs:
7474

7575
runs-on: ubuntu-24.04
7676
env:
77-
COMPILER: clang++-17
78-
CATA_CLANG_TIDY: clang-tidy-17
77+
COMPILER: clang++-18
78+
CATA_CLANG_TIDY: clang-tidy-18
7979
CATA_CLANG_TIDY_SUBSET: ${{ matrix.subset }}
8080
TILES: 1
8181
SOUND: 1
@@ -85,7 +85,7 @@ jobs:
8585
if: ${{ needs.skip-duplicates.outputs.should_skip != 'true' && github.event_name != 'pull_request' || github.event.pull_request.draft == false }}
8686
run: |
8787
sudo apt-get update
88-
sudo apt install clang-17 clang-tidy-17 cmake ccache jq
88+
sudo apt install clang-18 clang-tidy-18 cmake ccache jq
8989
sudo apt install libflac-dev libsdl2-dev libsdl2-ttf-dev libsdl2-image-dev libsdl2-mixer-dev libpulse-dev gettext
9090
- name: checkout repository
9191
uses: actions/checkout@v4
@@ -122,7 +122,7 @@ jobs:
122122
run: | # the folder may not exist if there is no file to analyze
123123
if [ -d clang-tidy-trace ]
124124
then
125-
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
125+
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
126126
else
127127
echo "clang-tidy-trace folder not found."
128128
fi

.vscode/settings.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,6 @@
8585
"C_Cpp.formatting": "disabled",
8686
"astyle.astylerc": "${workspaceRoot}/.astylerc",
8787
"files.associations": {
88-
"vector": "cpp"
88+
".clang-tidy": "yaml",
8989
}
9090
}

build-scripts/clang-tidy-build.sh

+3-3
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@ cmake_extra_opts=()
1515
cmake_extra_opts+=("-DCATA_CLANG_TIDY_PLUGIN=ON")
1616
# Need to specify the particular LLVM / Clang versions to use, lest it
1717
# use the older LLVM that comes by default on Ubuntu.
18-
cmake_extra_opts+=("-DLLVM_DIR=/usr/lib/llvm-17/lib/cmake/llvm")
19-
cmake_extra_opts+=("-DClang_DIR=/usr/lib/llvm-17/lib/cmake/clang")
18+
cmake_extra_opts+=("-DLLVM_DIR=/usr/lib/llvm-18/lib/cmake/llvm")
19+
cmake_extra_opts+=("-DClang_DIR=/usr/lib/llvm-18/lib/cmake/clang")
2020

2121

2222
mkdir -p build
@@ -33,7 +33,7 @@ echo "Compiling clang-tidy plugin"
3333
make -j$num_jobs CataAnalyzerPlugin
3434
#export PATH=$PWD/tools/clang-tidy-plugin/clang-tidy-plugin-support/bin:$PATH
3535
# add FileCheck to the search path
36-
export PATH=/usr/lib/llvm-17/bin:$PATH
36+
export PATH=/usr/lib/llvm-18/bin:$PATH
3737
if ! which FileCheck
3838
then
3939
echo "Missing FileCheck"

src/activity_actor_definitions.h

+3-3
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ class aim_activity_actor : public activity_actor
5454
bool should_unload_RAS = false;
5555
bool snap_to_target = false;
5656
/* Item location for RAS weapon reload */
57-
item_location reload_loc = item_location();
57+
item_location reload_loc;
5858
bool shifting_view = false;
5959
tripoint_rel_ms initial_view_offset;
6060
/** Target UI requested to abort aiming */
@@ -1428,8 +1428,8 @@ class milk_activity_actor : public activity_actor
14281428

14291429
private:
14301430
int total_moves {};
1431-
std::vector<tripoint_abs_ms> monster_coords {};
1432-
std::vector<std::string> string_values {};
1431+
std::vector<tripoint_abs_ms> monster_coords;
1432+
std::vector<std::string> string_values;
14331433
};
14341434

14351435
class shearing_activity_actor : public activity_actor

src/activity_item_handling.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -446,7 +446,7 @@ void put_into_vehicle_or_drop( Character &you, item_drop_reason reason,
446446
{
447447
map &here = get_map();
448448

449-
return put_into_vehicle_or_drop( you, reason, items, &here, you.pos_bub( &here ) );
449+
put_into_vehicle_or_drop( you, reason, items, &here, you.pos_bub( &here ) );
450450
}
451451

452452
void put_into_vehicle_or_drop( Character &you, item_drop_reason reason,

src/cata_bitset.cpp

+2-1
Original file line numberDiff line numberDiff line change
@@ -43,5 +43,6 @@ void tiny_bitset::resize_heap( size_t requested_bits ) noexcept
4343

4444
void tiny_bitset::set_storage( block_t *data )
4545
{
46+
// NOLINTNEXTLINE(bugprone-multi-level-implicit-pointer-conversion)
4647
memcpy( &storage_, &data, sizeof( data ) );
47-
}
48+
}

src/cata_bitset.h

+1
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,7 @@ class tiny_bitset
178178
// The hashtag blessed UB-avoiding way of type punning a pointer
179179
// out of an integer.
180180
block_t *ret;
181+
// NOLINTNEXTLINE(bugprone-multi-level-implicit-pointer-conversion)
181182
memcpy( &ret, &storage_, sizeof( storage_ ) );
182183
return ret;
183184
}

src/cata_tiles.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -287,7 +287,7 @@ tileset::find_tile_type_by_season( const std::string &id, season_type season ) c
287287
}
288288
const tileset::season_tile_value &res = iter->second;
289289
if( res.season_tile ) {
290-
return *res.season_tile;
290+
return res.season_tile;
291291
} else if( res.default_tile ) { // can skip this check, but just in case
292292
return tile_lookup_res( iter->first, *res.default_tile );
293293
}

src/character_modifier.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ struct character_modifier {
6060
mod_type limbscore_modop = MULT;
6161
std::vector<std::pair<character_modifier_id, mod_id>> src;
6262
body_part_type::type limbtype = body_part_type::type::num_types;
63-
translation desc = translation();
63+
translation desc;
6464
mod_type modtype = mod_type::NONE;
6565
float max_val = 0.0f;
6666
float min_val = 0.0f;

src/climbing.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ void climbing_aid::down_t::deserialize( const JsonObject &jo )
214214
jo.throw_error( str_cat( "failed to read optional member \"menu_hotkey\"" ) );
215215
}
216216
}
217-
if( menu_hotkey_str.length() ) {
217+
if( !menu_hotkey_str.empty() ) {
218218
menu_hotkey = std::uint8_t( menu_hotkey_str[ 0 ] );
219219
}
220220

src/construction.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ class nc_color;
3131

3232
struct partial_con {
3333
int counter = 0;
34-
std::list<item> components = {};
34+
std::list<item> components;
3535
construction_id id = construction_id( -1 );
3636
};
3737

0 commit comments

Comments
 (0)