Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
greenc-FNAL wants to merge 3 commits into
base: main
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e1dc6ad
Address review comment
greenc-FNAL Apr 28, 2026
5a0c62d
Ignore some `clang-tidy` checks by directory
greenc-FNAL Apr 28, 2026
36020c1
Annotate known/verified instance of `static_cast` to derived
greenc-FNAL Apr 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions form/root_storage/.clang-tidy
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---

InheritParentConfig: true

Checks: >
-cppcoreguidelines-pro-type-const-cast,
-cppcoreguidelines-pro-type-reinterpret-cast
3 changes: 2 additions & 1 deletion form/root_storage/root_tbranch_read_container.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ bool ROOT_TBranch_Read_ContainerImp::read(int id, void const** data, std::type_i
}

if (dictInfo->Property() & EProperty::kIsFundamental) {
//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));
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);

Copilot uses AI. Check for mistakes.
branchBuffer = new char[fundInfo->Size()];
branchStatus = m_tree->SetBranchAddress(col_name().c_str(),
Expand Down
7 changes: 7 additions & 0 deletions plugins/python/src/.clang-tidy
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---

InheritParentConfig: true

Checks: >
-cppcoreguidelines-pro-type-const-cast,
-cppcoreguidelines-pro-type-reinterpret-cast
19 changes: 10 additions & 9 deletions plugins/python/src/configwrap.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,16 @@ struct phlex::experimental::py_config_map {
PyObject* phlex::experimental::wrap_configuration(configuration const& config)
{
py_config_map* pyconfig =
(py_config_map*)PhlexConfig_Type.tp_new(&PhlexConfig_Type, nullptr, nullptr);
reinterpret_cast<py_config_map*>(PhlexConfig_Type.tp_new(&PhlexConfig_Type, nullptr, nullptr));

pyconfig->ph_config = &config;

return (PyObject*)pyconfig;
return reinterpret_cast<PyObject*>(pyconfig);
}

static py_config_map* pcm_new(PyTypeObject* subtype, PyObject*, PyObject*)
{
py_config_map* pcm = (py_config_map*)subtype->tp_alloc(subtype, 0);
py_config_map* pcm = reinterpret_cast<py_config_map*>(subtype->tp_alloc(subtype, 0));
if (!pcm)
return nullptr;

Expand All @@ -41,7 +41,7 @@ static py_config_map* pcm_new(PyTypeObject* subtype, PyObject*, PyObject*)
static void pcm_dealloc(py_config_map* pcm)
{
Py_DECREF(pcm->ph_config_cache);
Py_TYPE(pcm)->tp_free((PyObject*)pcm);
Py_TYPE(pcm)->tp_free(reinterpret_cast<PyObject*>(pcm));
}

// Returns the array size as Py_ssize_t, or std::nullopt (and sets a Python
Expand Down Expand Up @@ -213,17 +213,18 @@ static PyObject* pcm_subscript(py_config_map* pycmap, PyObject* pykey)

// PyMappingMethods must be non-const; tp_as_mapping in PyTypeObject takes a non-const pointer.
// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
static PyMappingMethods pcm_as_mapping = {nullptr, (binaryfunc)pcm_subscript, nullptr};
static PyMappingMethods pcm_as_mapping = {
nullptr, reinterpret_cast<binaryfunc>(pcm_subscript), nullptr};

// clang-format off
// PyType_Ready() modifies PyTypeObject in-place; the Python C API requires non-const.
// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
PyTypeObject phlex::experimental::PhlexConfig_Type = {
PyVarObject_HEAD_INIT(&PyType_Type, 0)
(char*) "pyphlex.configuration", // tp_name
const_cast<char*>("pyphlex.configuration"), // tp_name
Copy link
Copy Markdown
Member

@knoepfel knoepfel Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

sizeof(py_config_map), // tp_basicsize
0, // tp_itemsize
(destructor)pcm_dealloc, // tp_dealloc
reinterpret_cast<destructor>(pcm_dealloc), // tp_dealloc
0, // tp_vectorcall_offset / tp_print
0, // tp_getattr
0, // tp_setattr
Expand All @@ -239,7 +240,7 @@ PyTypeObject phlex::experimental::PhlexConfig_Type = {
0, // tp_setattro
0, // tp_as_buffer
Py_TPFLAGS_DEFAULT, // tp_flags
(char*)"phlex configuration object-as-dictionary", // tp_doc
const_cast<char*>("phlex configuration object-as-dictionary"), // tp_doc
0, // tp_traverse
0, // tp_clear
0, // tp_richcompare
Expand All @@ -256,7 +257,7 @@ PyTypeObject phlex::experimental::PhlexConfig_Type = {
offsetof(py_config_map, ph_config_cache), // tp_dictoffset
0, // tp_init
0, // tp_alloc
(newfunc)pcm_new, // tp_new
reinterpret_cast<newfunc>(pcm_new), // tp_new
0, // tp_free
0, // tp_is_gc
0, // tp_bases
Expand Down
14 changes: 8 additions & 6 deletions plugins/python/src/dciwrap.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ PyObject* phlex::experimental::wrap_dci(data_cell_index const& dci)
py_data_cell_index* pydci = PyObject_New(py_data_cell_index, &PhlexDataCellIndex_Type);
pydci->ph_dci = &dci;

return (PyObject*)pydci;
return reinterpret_cast<PyObject*>(pydci);
}

// simple forwarding methods
Expand All @@ -30,16 +30,18 @@ static PyObject* dci_number(py_data_cell_index* pydci)

// PyMethodDef arrays must be non-const; tp_methods in PyTypeObject takes a non-const pointer.
// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
static PyMethodDef dci_methods[] = {
{(char*)"number", (PyCFunction)dci_number, METH_NOARGS, (char*)"index number"},
{(char*)nullptr, nullptr, 0, nullptr}};
static PyMethodDef dci_methods[] = {{const_cast<char*>("number"),
reinterpret_cast<PyCFunction>(dci_number),
METH_NOARGS,
const_cast<char*>("index number")},
{(char*)nullptr, nullptr, 0, nullptr}};

// clang-format off
// PyType_Ready() modifies PyTypeObject in-place; the Python C API requires non-const.
// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
PyTypeObject phlex::experimental::PhlexDataCellIndex_Type = {
PyVarObject_HEAD_INIT(&PyType_Type, 0)
(char*) "pyphlex.data_cell_index", // tp_name
const_cast<char*>("pyphlex.data_cell_index"), // tp_name
sizeof(py_data_cell_index), // tp_basicsize
0, // tp_itemsize
0, // tp_dealloc
Expand All @@ -58,7 +60,7 @@ PyTypeObject phlex::experimental::PhlexDataCellIndex_Type = {
0, // tp_setattro
0, // tp_as_buffer
Py_TPFLAGS_DEFAULT, // tp_flags
(char*)"phlex data_cell_index", // tp_doc
const_cast<char*>("phlex data_cell_index"), // tp_doc
0, // tp_traverse
0, // tp_clear
0, // tp_richcompare
Expand Down
16 changes: 8 additions & 8 deletions plugins/python/src/lifelinewrap.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ using namespace phlex::experimental;

static py_lifeline_t* ll_new(PyTypeObject* pytype, PyObject*, PyObject*)
{
py_lifeline_t* pyobj = (py_lifeline_t*)pytype->tp_alloc(pytype, 0);
py_lifeline_t* pyobj = reinterpret_cast<py_lifeline_t*>(pytype->tp_alloc(pytype, 0));
if (pyobj) {
pyobj->m_view = nullptr;
new (&pyobj->m_source) std::shared_ptr<void>{};
Expand Down Expand Up @@ -39,18 +39,18 @@ static void ll_dealloc(py_lifeline_t* pyobj)
typedef std::shared_ptr<void> generic_shared_t;
pyobj->m_source.~generic_shared_t();
// Use tp_free to pair with tp_alloc for GC-tracked Python objects.
Py_TYPE(pyobj)->tp_free((PyObject*)pyobj);
Py_TYPE(pyobj)->tp_free(reinterpret_cast<PyObject*>(pyobj));
}

// clang-format off
// PyType_Ready() modifies PyTypeObject in-place; the Python C API requires non-const.
// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
PyTypeObject phlex::experimental::PhlexLifeline_Type = {
PyVarObject_HEAD_INIT(&PyType_Type, 0)
(char*) "pyphlex.lifeline", // tp_name
const_cast<char*>("pyphlex.lifeline"), // tp_name
sizeof(py_lifeline_t), // tp_basicsize
0, // tp_itemsize
(destructor)ll_dealloc, // tp_dealloc
reinterpret_cast<destructor>(ll_dealloc), // tp_dealloc
0, // tp_vectorcall_offset / tp_print
0, // tp_getattr
0, // tp_setattr
Expand All @@ -66,9 +66,9 @@ PyTypeObject phlex::experimental::PhlexLifeline_Type = {
0, // tp_setattro
0, // tp_as_buffer
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC, // tp_flags
(char*)"internal", // tp_doc
(traverseproc)ll_traverse, // tp_traverse
(inquiry)ll_clear, // tp_clear
const_cast<char*>("internal"), // tp_doc
reinterpret_cast<traverseproc>(ll_traverse), // tp_traverse
reinterpret_cast<inquiry>(ll_clear), // tp_clear
0, // tp_richcompare
0, // tp_weaklistoffset
0, // tp_iter
Expand All @@ -83,7 +83,7 @@ PyTypeObject phlex::experimental::PhlexLifeline_Type = {
0, // tp_dictoffset
0, // tp_init
0, // tp_alloc
(newfunc)ll_new, // tp_new
reinterpret_cast<newfunc>(ll_new), // tp_new
0, // tp_free
0, // tp_is_gc
0, // tp_bases
Expand Down
Loading
Loading