`clang-tidy`: resolve `cppguidelines-pro-type-*cast*` by greenc-FNAL · Pull Request #554 · Framework-R-D/phlex

greenc-FNAL · 2026-04-28T22:14:09Z

Address review comment
Ignore some clang-tidy checks by directory
Annotate known/verified instance of static_cast to derived

Ignore: - `cppcoreguidelines-pro-type-const-cast` - `cppcoreguidelines-pro-type-reinterpret-cast` in `form/root_storage` and `plugins/python/src` due to ROOT and Python interaction limitations, respectively.

Copilot

Pull request overview

This PR updates Python and ROOT interop code to address clang-tidy cppcoreguidelines-pro-type-*cast* findings by replacing C-style casts, adding targeted NOLINT annotations, and introducing per-directory .clang-tidy overrides.

Changes:

Replace many C-style casts with reinterpret_cast/const_cast in Python wrapper code.
Add per-directory .clang-tidy configs to disable cppcoreguidelines-pro-type-const-cast and cppcoreguidelines-pro-type-reinterpret-cast for interop-heavy directories.
Add a justified NOLINT for a verified static_cast downcast in ROOT storage code.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
plugins/python/src/modulewrap.cpp	Reworks casts around Python object conversions and method/type table definitions to address `clang-tidy` findings.
plugins/python/src/lifelinewrap.cpp	Updates allocation/deallocation and `PyTypeObject` initialization casts to satisfy cast-related checks.
plugins/python/src/dciwrap.cpp	Updates object wrapping and method/type table casts away from C-style.
plugins/python/src/configwrap.cpp	Updates casts in configuration wrapper and `PyTypeObject`/mapping method setup.
plugins/python/src/.clang-tidy	Disables `pro-type-const-cast` and `pro-type-reinterpret-cast` checks for Python plugin sources.
form/root_storage/root_tbranch_read_container.cpp	Documents and suppresses a verified safe downcast for fundamental ROOT types.
form/root_storage/.clang-tidy	Disables `pro-type-const-cast` and `pro-type-reinterpret-cast` checks for ROOT storage sources.

Copilot · 2026-04-28T22:19:27Z

  static char const* kwnames[] = {
    "callable", "input_family", "output_product_suffixes", "concurrency", "name", nullptr};
  PyObject *callable = 0, *input = 0, *output = 0, *concurrency = 0, *pyname = 0;
-  if (!PyArg_ParseTupleAndKeywords(
-        args, kwds, "OO|OOO", (char**)kwnames, &callable, &input, &output, &concurrency, &pyname)) {
+  if (!PyArg_ParseTupleAndKeywords(args,
+                                   kwds,
+                                   "OO|OOO",
+                                   const_cast<char**>(kwnames),
+                                   &callable,


In parse_args, kwnames is an array of const char* but it is passed to PyArg_ParseTupleAndKeywords via const_cast<char**>. That cast is type-unsafe (it implies the callee may mutate both the pointers and the pointed-to characters). Prefer declaring a static char* kwlist with writable storage (e.g., static char kw0[] = "callable"; ...; static char* kwnames[] = {kw0, ... , nullptr};) and pass kwnames without casting.

Copilot · 2026-04-28T22:19:27Z

  {                                                                                                \
    PyGILRAII gil;                                                                                 \
-    cpptype i = (cpptype)frompy((PyObject*)pyobj);                                                 \
+    cpptype i = (cpptype)frompy(reinterpret_cast<PyObject*>(pyobj));                               \


BASIC_CONVERTER still uses a C-style cast to cpptype (cpptype i = (cpptype)frompy(...)). This will continue to trigger cppcoreguidelines-pro-type-cstyle-cast and is easy to replace with static_cast<cpptype>(...).

Suggested change

cpptype i = (cpptype)frompy(reinterpret_cast<PyObject*>(pyobj)); \

cpptype i = static_cast<cpptype>(frompy(reinterpret_cast<PyObject*>(pyobj))); \

Copilot · 2026-04-28T22:19:27Z

    PyObject* np_view = PyArray_SimpleNewFromData(1,                 /* 1-D array */               \
                                                  dims,              /* dimension sizes */         \
                                                  nptype,            /* numpy C type */            \
                                                  (void*)(v->data()) /* raw buffer */              \
    );                                                                                             \


In VECTOR_CONVERTER, the (void*)(v->data()) cast is unnecessary (object pointers implicitly convert to void* in C++) and also trips cppcoreguidelines-pro-type-cstyle-cast. Prefer passing v->data() directly or using static_cast<void*>(v->data()) if an explicit cast is desired.

Copilot · 2026-04-28T22:19:28Z

      for (Py_ssize_t i = 0; i < total; ++i) {                                                     \
-        PyObject* item = PyList_GetItem((PyObject*)pyobj, i);                                      \
+        PyObject* item = PyList_GetItem(reinterpret_cast<PyObject*>(pyobj), i);                    \
        vec->push_back((cpptype)frompy(item));                                                     \


In NUMPY_ARRAY_CONVERTER, vec->push_back((cpptype)frompy(item)); is another remaining C-style cast that will be flagged by cppcoreguidelines-pro-type-cstyle-cast. Prefer static_cast<cpptype>(frompy(item)).

Suggested change

vec->push_back((cpptype)frompy(item)); \

vec->push_back(static_cast<cpptype>(frompy(item))); \

Copilot · 2026-04-28T22:19:28Z

-    //Assume this is a fundamental type like int or double
+    // We can avoid a `dynamic_cast` to `TDataType` here after checking EProperty::kIsFundamental
+    // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast)
    auto fundInfo = static_cast<TDataType*>(TDictionary::GetDictionary(type));


dictInfo is already retrieved just above and proven fundamental, but the code calls TDictionary::GetDictionary(type) again to initialize fundInfo. This is redundant and could be replaced with static_cast<TDataType*>(dictInfo) to avoid a second lookup and make the downcast intent clearer.

Suggested change

auto fundInfo = static_cast<TDataType*>(TDictionary::GetDictionary(type));

auto fundInfo = static_cast<TDataType*>(dictInfo);

codecov · 2026-04-28T22:27:18Z

Codecov Report

❌ Patch coverage is 95.23810% with 2 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
plugins/python/src/modulewrap.cpp	94.44%	1 Missing and 1 partial ⚠️

@@            Coverage Diff             @@
##             main     #554      +/-   ##
==========================================
+ Coverage   85.27%   85.36%   +0.08%     
==========================================
  Files         144      144              
  Lines        3675     3683       +8     
  Branches      632      632              
==========================================
+ Hits         3134     3144      +10     
  Misses        324      324              
+ Partials      217      215       -2

Flag	Coverage Δ
unittests	`85.36% <95.23%> (+0.08%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
form/root_storage/root_tbranch_read_container.cpp	`78.84% <ø> (ø)`
plugins/python/src/configwrap.cpp	`75.60% <100.00%> (ø)`
plugins/python/src/dciwrap.cpp	`100.00% <100.00%> (ø)`
plugins/python/src/lifelinewrap.cpp	`56.25% <100.00%> (ø)`
plugins/python/src/modulewrap.cpp	`80.37% <94.44%> (+0.23%)`	⬆️

... and 1 file with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 67e9a8f...36020c1. Read the comment docs.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

knoepfel · 2026-04-29T13:58:08Z

 PyTypeObject phlex::experimental::PhlexConfig_Type = {
  PyVarObject_HEAD_INIT(&PyType_Type, 0)
-  (char*) "pyphlex.configuration",                   // tp_name
+  const_cast<char*>("pyphlex.configuration"),        // tp_name


If I remember correctly, I think that each of these hard-casts (now const_casts) can simply be replaced by the string literal:

Suggested change

const_cast<char*>("pyphlex.configuration"), // tp_name

"pyphlex.configuration", // tp_name

Can we try making that change here and elsewhere? If that works, we'd be able to re-enable the cppcoreguidelines-pro-type-const-cast check for this directory.

greenc-FNAL added 3 commits April 28, 2026 16:28

Address review comment

e1dc6ad

- Per #553 (comment)

Ignore some clang-tidy checks by directory

5a0c62d

Ignore: - `cppcoreguidelines-pro-type-const-cast` - `cppcoreguidelines-pro-type-reinterpret-cast` in `form/root_storage` and `plugins/python/src` due to ROOT and Python interaction limitations, respectively.

Annotate known/verified instance of static_cast to derived

36020c1

Copilot AI review requested due to automatic review settings April 28, 2026 22:14

Copilot started reviewing on behalf of greenc-FNAL April 28, 2026 22:14 View session

Copilot AI reviewed Apr 28, 2026

View reviewed changes

knoepfel reviewed Apr 29, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`clang-tidy`: resolve `cppguidelines-pro-type-cast`#554

`clang-tidy`: resolve `cppguidelines-pro-type-cast`#554
greenc-FNAL wants to merge 3 commits intomainfrom
maintenance/clang-tidy/cppcoreguidelines-pro-type-cast

greenc-FNAL commented Apr 28, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 28, 2026

Uh oh!

Copilot AI Apr 28, 2026

Uh oh!

Copilot AI Apr 28, 2026

Uh oh!

Copilot AI Apr 28, 2026

Uh oh!

Copilot AI Apr 28, 2026

Uh oh!

codecov Bot commented Apr 28, 2026

Uh oh!

knoepfel Apr 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	cpptype i = (cpptype)frompy(reinterpret_cast<PyObject*>(pyobj)); \
	cpptype i = static_cast<cpptype>(frompy(reinterpret_cast<PyObject*>(pyobj))); \

	vec->push_back((cpptype)frompy(item)); \
	vec->push_back(static_cast<cpptype>(frompy(item))); \

	auto fundInfo = static_cast<TDataType*>(TDictionary::GetDictionary(type));
	auto fundInfo = static_cast<TDataType*>(dictInfo);

	const_cast<char*>("pyphlex.configuration"), // tp_name
	"pyphlex.configuration", // tp_name

Conversation

greenc-FNAL commented Apr 28, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Apr 28, 2026

Codecov Report

Uh oh!

knoepfel Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants