Skip to content

Commit 7aba406

Browse files
committed
Fix #356: CI can now fail on UBSan findings and clang-tidy warnings
Two CI jobs were structurally unable to fail on the defect class they exist to catch: GCC UBSan recovers by default (prints and exits 0, so the sanitizer leg only enforced the ASan half), and clang-tidy exits 0 on warnings with WarningsAsErrors unset. - Sanitizer blocks (library + parser) add -fno-sanitize-recover= undefined. Verified locally: the full suite passes under fatal ASan+UBSan (the real UB this would have caught was fixed in #349 and #361 first). - The clang-tidy workflow adds --warnings-as-errors='*', and the 15-warning baseline is fixed in the same change (no-op std::move on const-ref args and a trivially-copyable variant, missing override on two rebuild-visitor dtors, std::move on a forwarding reference, a cloned constexpr branch merged, a value param made const-ref, and a NOLINT that sat on the wrong line), so the job starts green. Signed-off-by: petlenz <peterlenz89.pl@gmail.com>
1 parent 82f5e3c commit 7aba406

11 files changed

Lines changed: 28 additions & 30 deletions

File tree

.github/workflows/clang-tidy-check.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,4 +30,4 @@ jobs:
3030
- name: Run clang-tidy
3131
run: |
3232
find src -name '*.cpp' -print0 \
33-
| xargs -0 clang-tidy-18 -p build
33+
| xargs -0 clang-tidy-18 -p build --warnings-as-errors='*'

CMakeLists.txt

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -194,9 +194,10 @@ endif()
194194
# Sanitizers
195195
if(NUMSIM_CAS_SANITIZERS)
196196
target_compile_options(${PROJECT_NAME} PUBLIC
197-
-fsanitize=address,undefined -fno-omit-frame-pointer)
197+
-fsanitize=address,undefined -fno-sanitize-recover=undefined
198+
-fno-omit-frame-pointer)
198199
target_link_options(${PROJECT_NAME} PUBLIC
199-
-fsanitize=address,undefined)
200+
-fsanitize=address,undefined -fno-sanitize-recover=undefined)
200201
endif()
201202

202203
# Optional convenience for Windows
@@ -274,9 +275,10 @@ if(NUMSIM_CAS_BUILD_PARSER)
274275

275276
if(NUMSIM_CAS_SANITIZERS)
276277
target_compile_options(NumSim_CAS_Parser PUBLIC
277-
-fsanitize=address,undefined -fno-omit-frame-pointer)
278+
-fsanitize=address,undefined -fno-sanitize-recover=undefined
279+
-fno-omit-frame-pointer)
278280
target_link_options(NumSim_CAS_Parser PUBLIC
279-
-fsanitize=address,undefined)
281+
-fsanitize=address,undefined -fno-sanitize-recover=undefined)
280282
endif()
281283

282284
if(WIN32)

include/numsim_cas/core/scalar_number.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ class scalar_number {
9696
scalar_number const &exp);
9797

9898
private:
99-
explicit scalar_number(variant_t vv) : v_(std::move(vv)) {
99+
explicit scalar_number(variant_t vv) : v_(vv) {
100100
if (auto *r = std::get_if<rational_t>(&v_)) {
101101
if (r->den == 1)
102102
v_ = r->num;

include/numsim_cas/parser/parse_error.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ class parse_error : public cas_error {
3737
public:
3838
/// Construct with optional source context. Pass an empty `source` and
3939
/// `byte_offset = 0` for errors raised outside the parser.
40-
parse_error(std::string message, std::size_t byte_offset,
40+
parse_error(std::string const &message, std::size_t byte_offset,
4141
std::string_view source);
4242

4343
/// Byte offset into the source where the error was detected, clamped

include/numsim_cas/scalar/visitors/scalar_rebuild_visitor.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ class scalar_rebuild_visitor : public scalar_visitor_const_t {
1414
public:
1515
using expr_holder_t = expression_holder<scalar_expression>;
1616

17-
virtual ~scalar_rebuild_visitor() = default;
17+
~scalar_rebuild_visitor() override = default;
1818

1919
virtual expr_holder_t apply(expr_holder_t const &expr) {
2020
if (expr.is_valid()) {

include/numsim_cas/tensor/wrappers/tensor_inv.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@ class tensor_inv final : public unary_op<tensor_node_base_t<tensor_inv>> {
1212
using base = unary_op<tensor_node_base_t<tensor_inv>>;
1313

1414
template <typename Expr>
15-
explicit tensor_inv(
16-
Expr &&_expr) // NOLINT(bugprone-forwarding-reference-overload)
15+
// NOLINTNEXTLINE(bugprone-forwarding-reference-overload)
16+
explicit tensor_inv(Expr &&_expr)
1717
: base(std::forward<Expr>(_expr), _expr.get().dim(), _expr.get().rank()) {
1818
// ── Rank gate (#292) ──────────────────────────────────────────
1919
// Mirror the inv() factory's rank gate (tensor_functions.h:467)

include/numsim_cas/tensor_to_scalar/visitors/tensor_to_scalar_rebuild_visitor.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ class tensor_to_scalar_rebuild_visitor
1919
using scalar_holder_t = expression_holder<scalar_expression>;
2020
using tensor_holder_t = expression_holder<tensor_expression>;
2121

22-
virtual ~tensor_to_scalar_rebuild_visitor() = default;
22+
~tensor_to_scalar_rebuild_visitor() override = default;
2323

2424
virtual t2s_holder_t apply(t2s_holder_t const &expr) {
2525
if (expr.is_valid()) {

include/numsim_cas/tensor_to_scalar/visitors/tensor_to_scalar_substitution.h

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,8 @@ class tensor_to_scalar_substitution final
4545
}
4646

4747
tensor_holder_t apply_tensor(tensor_holder_t const &expr) override {
48-
if constexpr (std::is_same_v<TargetBase, tensor_expression>) {
49-
return substitute(expr, m_old, m_new);
50-
} else if constexpr (std::is_same_v<TargetBase, scalar_expression>) {
48+
if constexpr (std::is_same_v<TargetBase, tensor_expression> ||
49+
std::is_same_v<TargetBase, scalar_expression>) {
5150
return substitute(expr, m_old, m_new);
5251
} else {
5352
return expr;

src/numsim_cas/parser/parse_error.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ std::string format_message(std::string_view body, std::string_view source,
6363

6464
} // namespace
6565

66-
parse_error::parse_error(std::string message, std::size_t byte_offset,
66+
parse_error::parse_error(std::string const &message, std::size_t byte_offset,
6767
std::string_view source)
6868
: cas_error(format_message(message, source, byte_offset)) {
6969
if (!source.empty()) {

src/numsim_cas/parser/parser.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ syntax_error translate_pegtl_error(pegtl::parse_error const &e,
3838
// returns a string_view from message() which can't directly
3939
// construct a std::string by '='.
4040
std::string msg(e.message());
41-
return syntax_error(std::move(msg), byte, source);
41+
return syntax_error(msg, byte, source);
4242
}
4343

4444
// #355 — PEGTL parses by C++ recursion; deeply nested input overflows the
@@ -150,7 +150,7 @@ parsed_expression parse(std::string_view source, symbol_table &syms) {
150150
"bracket-list literal '[...]' cannot be a top-level expression",
151151
source.size(), source);
152152
} else {
153-
return std::move(v);
153+
return std::forward<decltype(v)>(v);
154154
}
155155
},
156156
std::move(state.values.front()));

0 commit comments

Comments
 (0)