Skip to content

Commit

Permalink
Change C-style casts to static_cast and remove unnecessary semicolons (
Browse files Browse the repository at this point in the history
…#668)

* Change C-style casts to static_cast

* Remove unnecessary semicolons

* Fix format

* Use .f

Co-authored-by: Thomas Madlener <[email protected]>

* Use .f

Co-authored-by: Thomas Madlener <[email protected]>

* Remove extra )

* Fix remaining issues

* Fix format

* Disable -Wold-style-cast for MurmurHash3.cpp

* Don't change the flags

* Fix test

---------

Co-authored-by: jmcarcell <[email protected]>
Co-authored-by: Thomas Madlener <[email protected]>
  • Loading branch information
3 people authored Sep 10, 2024
1 parent fb61989 commit 0777022
Show file tree
Hide file tree
Showing 18 changed files with 113 additions and 109 deletions.
2 changes: 1 addition & 1 deletion include/podio/CollectionIDTable.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class CollectionIDTable {
/// return registered names
const std::vector<std::string>& names() const {
return m_names;
};
}

/// return the ids
const std::vector<uint32_t>& ids() const {
Expand Down
2 changes: 1 addition & 1 deletion include/podio/DataSource.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ class DataSource : public ROOT::RDF::RDataSource {

std::string AsString() override {
return "Podio data source";
};
}

private:
/// Number of slots/threads
Expand Down
2 changes: 1 addition & 1 deletion include/podio/Frame.h
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ class Frame {
/// Get a const reference to the internally used GenericParameters
const podio::GenericParameters& parameters() const override {
return *m_parameters;
};
}

bool get(uint32_t collectionID, podio::CollectionBase*& collection) const override;

Expand Down
2 changes: 1 addition & 1 deletion include/podio/UserDataCollection.h
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ class UserDataCollection : public CollectionBase {
/// clear the collection and all internal states
void clear() override {
_vec.clear();
};
}

/// check if this collection is a subset collection - no subset possible
bool isSubsetCollection() const override {
Expand Down
9 changes: 5 additions & 4 deletions podioVersion.in.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
#define PODIO_PODIOVERSION_H

#include <cstdint>
#include <sstream>
#include <ostream>
#include <sstream>
#include <tuple>
#if __cplusplus >= 202002L
#include <compare>
Expand Down Expand Up @@ -64,7 +64,7 @@ struct Version {
std::stringstream ss;
ss << *this;
return ss.str();
};
}

friend std::ostream& operator<<(std::ostream&, const Version& v);
};
Expand All @@ -78,8 +78,9 @@ static constexpr Version build_version{podio_VERSION_MAJOR, podio_VERSION_MINOR,

/// Decode a version from a 64 bit unsigned
static constexpr Version decode_version(unsigned long version) noexcept {
return Version{(uint16_t)PODIO_MAJOR_VERSION(version), (uint16_t)PODIO_MINOR_VERSION(version),
(uint16_t)PODIO_PATCH_VERSION(version)};
return Version{static_cast<uint16_t>(PODIO_MAJOR_VERSION(version)),
static_cast<uint16_t>(PODIO_MINOR_VERSION(version)),
static_cast<uint16_t>(PODIO_PATCH_VERSION(version))};
}
} // namespace podio::version

Expand Down
2 changes: 1 addition & 1 deletion python/templates/Collection.cc.jinja2
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ void {{ collection_type }}::push_back(const Mutable{{ class.bare_type }}& object
auto obj = object.m_obj;
if (obj->id.index == podio::ObjectID::untracked) {
const auto size = m_storage.entries.size();
obj->id = {(int)size, m_collectionID};
obj->id = {static_cast<int>(size), m_collectionID};
m_storage.entries.push_back(obj.release());
{% if OneToManyRelations or VectorMembers %}
m_storage.createRelations(obj.get());
Expand Down
4 changes: 2 additions & 2 deletions python/templates/Collection.h.jinja2
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public:
void print(std::ostream& os=std::cout, bool flush=true) const final;

/// operator to allow pointer like calling of members a la LCIO
{{ class.bare_type }}Collection* operator->() { return ({{ class.bare_type }}Collection*) this; }
{{ class.bare_type }}Collection* operator->() { return static_cast<{{ class.bare_type }}Collection*>(this); }

/// Append a new object to the collection, and return this object.
Mutable{{ class.bare_type }} create();
Expand Down Expand Up @@ -136,7 +136,7 @@ public:
);
}
m_isValid = true;
};
}

uint32_t getID() const final {
return m_collectionID;
Expand Down
4 changes: 2 additions & 2 deletions python/templates/CollectionData.cc.jinja2
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,8 @@ podio::CollectionWriteBuffers {{ class_type }}::getCollectionBuffers(bool isSubs
{% endif -%}

return {
isSubsetColl ? nullptr : (void*)&m_data,
isSubsetColl ? nullptr : (void*)m_data.get(),
isSubsetColl ? nullptr : static_cast<void*>(&m_data),
isSubsetColl ? nullptr : static_cast<void*>(m_data.get()),
&m_refCollections, // only need to store the ObjectIDs of the referenced objects
&m_vecmem_info
};
Expand Down
2 changes: 1 addition & 1 deletion src/DataSource.cc
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ std::vector<void*> DataSource::GetColumnReadersImpl(std::string_view columnName,

std::vector<void*> columnReaders(m_nSlots);
for (size_t slotIndex = 0; slotIndex < m_nSlots; ++slotIndex) {
columnReaders[slotIndex] = (void*)&m_Collections[columnIndex][slotIndex];
columnReaders[slotIndex] = static_cast<void*>(&m_Collections[columnIndex][slotIndex]);
}

return columnReaders;
Expand Down
12 changes: 9 additions & 3 deletions src/MurmurHash3.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ inline uint64_t rotl64(uint64_t x, int8_t r) {

#endif // !defined(_MSC_VER)

// Disable -Wold-style-cast for this file
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wold-style-cast"

//-----------------------------------------------------------------------------
// Block read - if your platform needs to do endian-swapping or can only
// handle aligned reads, do the conversion here
Expand Down Expand Up @@ -131,7 +135,7 @@ void MurmurHash3_x86_32(const void* key, int len, uint32_t seed, void* out) {
k1 = ROTL32(k1, 15);
k1 *= c2;
h1 ^= k1;
};
}

//----------
// finalization
Expand Down Expand Up @@ -281,7 +285,7 @@ void MurmurHash3_x86_128(const void* key, const int len, uint32_t seed, void* ou
k1 = ROTL32(k1, 15);
k1 *= c2;
h1 ^= k1;
};
}

//----------
// finalization
Expand Down Expand Up @@ -418,7 +422,7 @@ void MurmurHash3_x64_128(const void* key, const int len, const uint32_t seed, vo
k1 = ROTL64(k1, 31);
k1 *= c2;
h1 ^= k1;
};
}

//----------
// finalization
Expand All @@ -440,3 +444,5 @@ void MurmurHash3_x64_128(const void* key, const int len, const uint32_t seed, vo
}

//-----------------------------------------------------------------------------

#pragma GCC diagnostic pop
6 changes: 3 additions & 3 deletions src/RNTupleWriter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,9 @@ void RNTupleWriter::writeFrame(const podio::Frame& frame, const std::string& cat
auto collBuffers = coll->getBuffers();
if (collBuffers.vecPtr) {
#if ROOT_VERSION_CODE >= ROOT_VERSION(6, 31, 0)
entry->BindRawPtr(name, (void*)collBuffers.vecPtr);
entry->BindRawPtr(name, static_cast<void*>(collBuffers.vecPtr));
#else
entry->CaptureValueUnsafe(name, (void*)collBuffers.vecPtr);
entry->CaptureValueUnsafe(name, static_cast<void*>(collBuffers.vecPtr));
#endif
}

Expand Down Expand Up @@ -148,7 +148,7 @@ void RNTupleWriter::writeFrame(const podio::Frame& frame, const std::string& cat
for (auto& [type, vec] : (*vmInfo)) {
const auto typeName = "vector<" + type + ">";
const auto brName = root_utils::vecBranch(name, relVecNames.vectorMembers[i]);
auto ptr = *(std::vector<int>**)vec;
auto ptr = *static_cast<std::vector<int>**>(vec);
#if ROOT_VERSION_CODE >= ROOT_VERSION(6, 31, 0)
entry->BindRawPtr(brName, ptr);
#else
Expand Down
74 changes: 36 additions & 38 deletions tests/read_frame.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,59 +20,59 @@

void processExtensions(const podio::Frame& event, int iEvent, podio::version::Version) {
const auto& extColl = event.get<extension::ContainedTypeCollection>("extension_Contained");
ASSERT(extColl.isValid(), "extension_Contained collection should be present");
ASSERT(extColl.size() == 1, "extension_Contained collection should have one element");
ASSERT(extColl.isValid(), "extension_Contained collection should be present")
ASSERT(extColl.size() == 1, "extension_Contained collection should have one element")
auto extElem = extColl[0];
const auto& polVec = extElem.getAVector();
ASSERT(polVec.r == iEvent * 123.f, "polVec.r value not as expected");
ASSERT(polVec.phi == 0.15f, "polVec.phi value not as expected");
ASSERT(polVec.rho == 3.14f, "polVec.phi value not as expected");
ASSERT(polVec.r == iEvent * 123.f, "polVec.r value not as expected")
ASSERT(polVec.phi == 0.15f, "polVec.phi value not as expected")
ASSERT(polVec.rho == 3.14f, "polVec.phi value not as expected")

const auto& extCompColl = event.get<extension::ExternalComponentTypeCollection>("extension_ExternalComponent");
ASSERT(extCompColl.isValid(), "extension_ExternalComponent collection should be present");
ASSERT(extCompColl.size() == 1, "extension_ExternalComponent should have one element");
ASSERT(extCompColl.isValid(), "extension_ExternalComponent collection should be present")
ASSERT(extCompColl.size() == 1, "extension_ExternalComponent should have one element")
auto extCompElem = extCompColl[0];
ASSERT((extCompElem.getAStruct().p == std::array{iEvent, iEvent - 2, iEvent + 4, iEvent * 8}),
"aStruct.p value not as expected");
ASSERT(extCompElem.getAComponent().aStruct.data.x == 42 * iEvent, "aComponent.aStruct.x value not as expected");
ASSERT(extCompElem.getAComponent().nspStruct.x == iEvent, "aComponent.nspStruct.x value not as expected");
ASSERT(extCompElem.getAComponent().nspStruct.y == iEvent * 2, "aComponent.nspStruct.y value not as expected");
"aStruct.p value not as expected")
ASSERT(extCompElem.getAComponent().aStruct.data.x == 42 * iEvent, "aComponent.aStruct.x value not as expected")
ASSERT(extCompElem.getAComponent().nspStruct.x == iEvent, "aComponent.nspStruct.x value not as expected")
ASSERT(extCompElem.getAComponent().nspStruct.y == iEvent * 2, "aComponent.nspStruct.y value not as expected")

const auto& extRelColl = event.get<extension::ExternalRelationTypeCollection>("extension_ExternalRelation");
ASSERT(extRelColl.isValid(), "extension_ExternalRelation collection should be present");
ASSERT(extRelColl.size() == 3, "extension_ExternalRelation collection should contain 3 elements");
ASSERT(extRelColl.isValid(), "extension_ExternalRelation collection should be present")
ASSERT(extRelColl.size() == 3, "extension_ExternalRelation collection should contain 3 elements")

const auto& hits = event.get<ExampleHitCollection>("hits");
auto elem0 = extRelColl[0];
ASSERT(elem0.getWeight() == iEvent * 100.f, "weight of first element not as expected");
ASSERT(elem0.getSingleHit() == hits[0], "single hit relation not as expected");
ASSERT(elem0.getWeight() == iEvent * 100.f, "weight of first element not as expected")
ASSERT(elem0.getSingleHit() == hits[0], "single hit relation not as expected")

const auto& clusters = event.get<ExampleClusterCollection>("clusters");
auto elem1 = extRelColl[1];
const auto relClusters = elem1.getClusters();
ASSERT(relClusters.size() == 2, "element should have two related clusters");
ASSERT(relClusters[0] == clusters[1], "first related cluster not as expected");
ASSERT(relClusters[1] == clusters[0], "first related cluster not as expected");
ASSERT(relClusters.size() == 2, "element should have two related clusters")
ASSERT(relClusters[0] == clusters[1], "first related cluster not as expected")
ASSERT(relClusters[1] == clusters[0], "first related cluster not as expected")

auto elem2 = extRelColl[2];
const auto& structs = elem2.getSomeStructs();
ASSERT(structs.size() == 3, "element should have 3 struct vector members");
ASSERT(structs[0].y == 0, "struct value not as expected");
ASSERT(structs[1].y == iEvent, "struct value not as expected");
ASSERT(structs[2].y == 2 * iEvent, "struct value not as expected");
ASSERT(structs.size() == 3, "element should have 3 struct vector members")
ASSERT(structs[0].y == 0, "struct value not as expected")
ASSERT(structs[1].y == iEvent, "struct value not as expected")
ASSERT(structs[2].y == 2 * iEvent, "struct value not as expected")
}

void checkVecMemSubsetColl(const podio::Frame& event) {
const auto& subsetColl = event.get<ExampleWithVectorMemberCollection>("VectorMemberSubsetColl");
const auto& origColl = event.get<ExampleWithVectorMemberCollection>("WithVectorMember");
ASSERT(subsetColl.isSubsetCollection(), "subset collection not read back as a subset collection");
ASSERT(subsetColl.size() == 1, "subset collection should have size 1");
ASSERT(subsetColl[0] == origColl[0], "subset coll does not have the right contents");
ASSERT(subsetColl.isSubsetCollection(), "subset collection not read back as a subset collection")
ASSERT(subsetColl.size() == 1, "subset collection should have size 1")
ASSERT(subsetColl[0] == origColl[0], "subset coll does not have the right contents")
}

void checkInterfaceCollection(const podio::Frame& event) {
const auto& interfaceColl = event.get<ExampleWithInterfaceRelationCollection>("interface_examples");
ASSERT(interfaceColl.size() == 2, "interface_examples should have two elements");
ASSERT(interfaceColl.size() == 2, "interface_examples should have two elements")

const auto& hits = event.get<ExampleHitCollection>("hits");
const auto& particles = event.get<ExampleMCCollection>("mcparticles");
Expand All @@ -81,22 +81,20 @@ void checkInterfaceCollection(const podio::Frame& event) {
const auto iface0 = interfaceColl[0];
const auto iface1 = interfaceColl[1];

ASSERT(iface0.aSingleEnergyType() == hits[0], "OneToOneRelation aSingleEnergy not persisted as expected");
ASSERT(iface1.aSingleEnergyType() == clusters[0], "OneToOneRelation aSingleEnergy not persisted as expected");
ASSERT(iface0.aSingleEnergyType() == hits[0], "OneToOneRelation aSingleEnergy not persisted as expected")
ASSERT(iface1.aSingleEnergyType() == clusters[0], "OneToOneRelation aSingleEnergy not persisted as expected")

const auto iface0Rels = iface0.manyEnergies();
ASSERT(iface0Rels.size() == 3,
"OneToManyRelation to interface does not have the expected number of related elements");
ASSERT(iface0Rels[0] == hits[0], "OneToManyRelations to interface not persisted correctly");
ASSERT(iface0Rels[1] == clusters[0], "OneToManyRelations to interface not persisted correctly");
ASSERT(iface0Rels[2] == particles[0], "OneToManyRelations to interface not persisted correctly");
ASSERT(iface0Rels.size() == 3, "OneToManyRelation to interface does not have the expected number of related elements")
ASSERT(iface0Rels[0] == hits[0], "OneToManyRelations to interface not persisted correctly")
ASSERT(iface0Rels[1] == clusters[0], "OneToManyRelations to interface not persisted correctly")
ASSERT(iface0Rels[2] == particles[0], "OneToManyRelations to interface not persisted correctly")

const auto iface1Rels = iface1.manyEnergies();
ASSERT(iface1Rels.size() == 3,
"OneToManyRelation to interface does not have the expected number of related elements");
ASSERT(iface1Rels[0] == particles[0], "OneToManyRelations to interface not persisted correctly");
ASSERT(iface1Rels[1] == hits[0], "OneToManyRelations to interface not persisted correctly");
ASSERT(iface1Rels[2] == clusters[0], "OneToManyRelations to interface not persisted correctly");
ASSERT(iface1Rels.size() == 3, "OneToManyRelation to interface does not have the expected number of related elements")
ASSERT(iface1Rels[0] == particles[0], "OneToManyRelations to interface not persisted correctly")
ASSERT(iface1Rels[1] == hits[0], "OneToManyRelations to interface not persisted correctly")
ASSERT(iface1Rels[2] == clusters[0], "OneToManyRelations to interface not persisted correctly")
}

template <typename ReaderT>
Expand Down
10 changes: 4 additions & 6 deletions tests/read_test.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,10 @@

// STL
#include <cassert>
#include <exception>
#include <iostream>
#include <limits>
#include <sstream>
#include <stdexcept>
#include <type_traits>
#include <vector>

template <typename FixedWidthT>
Expand All @@ -40,8 +38,8 @@ bool check_fixed_width_value(FixedWidthT actual, FixedWidthT expected, const std

void processEvent(const podio::Frame& event, int eventNum, podio::version::Version fileVersion) {
const float evtWeight = event.getParameter<float>("UserEventWeight").value();
if (evtWeight != (float)100. * eventNum) {
std::cout << " read UserEventWeight: " << evtWeight << " - expected : " << (float)100. * eventNum << std::endl;
if (evtWeight != 100.f * eventNum) {
std::cout << " read UserEventWeight: " << evtWeight << " - expected : " << 100.f * eventNum << std::endl;
throw std::runtime_error("Couldn't read event meta data parameters 'UserEventWeight'");
}

Expand Down Expand Up @@ -196,7 +194,7 @@ void processEvent(const podio::Frame& event, int eventNum, podio::version::Versi
}

for (auto pr : mcpRefs) {
if ((unsigned)pr.getObjectID().collectionID == mcpRefs.getID()) {
if (static_cast<unsigned>(pr.getObjectID().collectionID) == mcpRefs.getID()) {
throw std::runtime_error(
"objects of a reference collection should have a different collectionID than the reference collection");
}
Expand Down Expand Up @@ -380,7 +378,7 @@ void processEvent(const podio::Frame& event, int eventNum, podio::version::Versi
if (fileVersion >= podio::version::Version{0, 13, 2}) {
auto& usrInts = event.get<podio::UserDataCollection<uint64_t>>("userInts");

if (usrInts.size() != (unsigned)eventNum + 1) {
if (usrInts.size() != static_cast<unsigned>(eventNum + 1)) {
throw std::runtime_error("Could not read all userInts properly (expected: " + std::to_string(eventNum + 1) +
", actual: " + std::to_string(usrInts.size()) + ")");
}
Expand Down
2 changes: 1 addition & 1 deletion tests/root_io/read_and_write_associated.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ void writeCollection() {
item1.Number(i);
info.push_back(item1);

event.putParameter("UserEventWeight", (float)100. * i);
event.putParameter("UserEventWeight", 100.f * i);
std::cout << " event number: " << i << std::endl;
event.putParameter("UserEventName", std::to_string(i));

Expand Down
Loading

0 comments on commit 0777022

Please sign in to comment.