Skip to content

Commit 190ac7f

Browse files
authored
clang-tidy: resolve bugprone-unchecked-optional-access (#495)
`clang-tidy` does not recognize Catch2's `REQUIRE` macro as protecting the target `std::optional` against non-existent-value access; use `// NOLINT` to silence the warning. `clang-tidy` correctly noted that there was a (theoretical) path to unprotected access to an `std::optional`. We now throw an `std::logic_error` and protect it from coverage validation as a `should never happen` untestable case. * Widen coverage exclusion to the full "can never happen" block
1 parent 88851ae commit 190ac7f

File tree

4 files changed

+12
-7
lines changed

4 files changed

+12
-7
lines changed

docs/dev/clang-tidy-fixes-2026-04.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@
1616
- [PR #499](https://github.com/Framework-R-D/phlex/pull/499)
1717
- [x] [bugprone-throwing-static-initialization](https://clang.llvm.org/extra/clang-tidy/checks/bugprone/throwing-static-initialization.html) (7)
1818
- [PR #472](https://github.com/Framework-R-D/phlex/pull/472)
19-
- [ ] [bugprone-unchecked-optional-access](https://clang.llvm.org/extra/clang-tidy/checks/bugprone/unchecked-optional-access.html) (15)
19+
- [x] [bugprone-unchecked-optional-access](https://clang.llvm.org/extra/clang-tidy/checks/bugprone/unchecked-optional-access.html) (15)
20+
- [PR #495](https://github.com/Framework-R-D/phlex/pull/495)
2021
- [ ] [cert-dcl50-cpp](https://clang.llvm.org/extra/clang-tidy/checks/cert/dcl50-cpp.html) (2)
2122
- [ ] [cert-err33-c](https://clang.llvm.org/extra/clang-tidy/checks/cert/err33-c.html) (4)
2223
- [ ] [cert-msc30-c](https://clang.llvm.org/extra/clang-tidy/checks/cert/msc30-c.html) (6)

plugins/python/src/modulewrap.cpp

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1182,11 +1182,13 @@ static PyObject* sc_provide(py_phlex_source* src, PyObject* args, PyObject* kwds
11821182
// translate and validate the output query
11831183
auto opq = validate_query(output);
11841184
if (!opq.has_value()) {
1185-
// validate_query will have set a python exception
1186-
std::string msg;
1187-
if (msg_from_py_error(msg, false)) {
1188-
throw std::runtime_error("output specification error: " + msg);
1189-
}
1185+
// LCOV_EXCL_START
1186+
// validate_query _will_ have set a python exception with details about the
1187+
// error, so just propagate it as a C++ exception
1188+
std::string msg{"(unknown)"};
1189+
msg_from_py_error(msg, false);
1190+
throw std::runtime_error("output specification error: " + msg);
1191+
// LCOV_EXCL_STOP
11901192
}
11911193

11921194
// insert provider node (TODO: as in transform and observe, we'll leak the

test/form/form_basics_test.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,8 @@ TEST_CASE("form::experimental::config tests", "[form]")
207207
cfg.addItem("prod1", "file1.root", 1);
208208

209209
auto item = cfg.findItem("prod1");
210-
REQUIRE(item.has_value());
210+
REQUIRE(item);
211+
// NOLINTNEXTLINE(bugprone-unchecked-optional-access) -- `REQUIRE` protects against incorrect access
211212
CHECK(item->product_name == "prod1");
212213

213214
CHECK_FALSE(cfg.findItem("nonexistent").has_value());

test/identifier.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ TEST_CASE("Identifier from JSON", "[identifier]")
3333
boost::json::object parsed_json = boost::json::parse(R"( {"identifier": "b" } )").as_object();
3434
auto b_from_json = phlex::detail::value_if_exists(parsed_json, "identifier");
3535
REQUIRE(b_from_json);
36+
// NOLINTNEXTLINE(bugprone-unchecked-optional-access) -- `REQUIRE` protects against incorrect access
3637
CHECK(b == *b_from_json);
3738
}
3839

0 commit comments

Comments
 (0)