Skip to content
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

[CORE] Apply code checks/format #46999

Merged
merged 1 commit into from
Dec 20, 2024
Merged

[CORE] Apply code checks/format #46999

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 1 addition & 2 deletions DataFormats/Provenance/src/ProductProvenance.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@ namespace edm {

ProductProvenance::ProductProvenance(BranchID bid) : branchID_(bid), parentageID_() {}

ProductProvenance::ProductProvenance(BranchID bid, ParentageID edid)
: branchID_(bid), parentageID_(std::move(edid)) {}
ProductProvenance::ProductProvenance(BranchID bid, ParentageID edid) : branchID_(bid), parentageID_(edid) {}

ProductProvenance::ProductProvenance(BranchID bid, std::vector<BranchID> const& parents)
: branchID_(bid), parentageID_() {
Expand Down
2 changes: 1 addition & 1 deletion FWCore/Framework/bin/cmsRun.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ int main(int argc, const char* argv[]) {
//NOTE: JobReport must have a lifetime shorter than jobReportStreamPtr so that when the JobReport destructor
// is called jobReportStreamPtr is still valid
auto jobRepPtr = std::make_unique<edm::JobReport>(jobReportStreamPtr.get());
jobRep.reset(new edm::serviceregistry::ServiceWrapper<edm::JobReport>(std::move(jobRepPtr)));
jobRep = std::make_shared<edm::serviceregistry::ServiceWrapper<edm::JobReport>>(std::move(jobRepPtr));
edm::ServiceToken jobReportToken = edm::ServiceRegistry::createContaining(jobRep);

if (!fileName.empty()) {
Expand Down
5 changes: 2 additions & 3 deletions FWCore/Framework/src/EventPrincipal.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,8 +201,7 @@ namespace edm {
}
auto phb = getProductResolverByIndex(index);

productProvenanceRetrieverPtr()->insertIntoSet(
ProductProvenance(phb->branchDescription().branchID(), std::move(parentage)));
productProvenanceRetrieverPtr()->insertIntoSet(ProductProvenance(phb->branchDescription().branchID(), parentage));

assert(phb);
// ProductResolver assumes ownership
Expand All @@ -214,7 +213,7 @@ namespace edm {
std::optional<ProductProvenance> productProvenance) const {
assert(!bd.produced());
if (productProvenance) {
productProvenanceRetrieverPtr()->insertIntoSet(std::move(*productProvenance));
productProvenanceRetrieverPtr()->insertIntoSet(*productProvenance);
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to note for future reference, ProductProvenance is as efficient to copy as it is to move.

}
auto phb = getExistingProduct(bd.branchID());
assert(phb);
Expand Down
15 changes: 8 additions & 7 deletions FWCore/MessageService/src/ELoutput.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,11 @@
// #define ELoutputTRACE_LOG
// #define ELoutput_EMIT_TRACE

#include <iostream>
#include <fstream>
#include <cstring>
#include <cassert>
#include <cstring>
#include <fstream>
#include <iostream>
#include <memory>

namespace {
// ----------------------------------------------------------------------
Expand Down Expand Up @@ -375,14 +376,14 @@ namespace edm {
needAspace = false;
}
}
if (wantModule && (xid.module.length() > 0)) {
if (wantModule && (!xid.module.empty())) {
if (needAspace) {
emitToken(" ");
needAspace = false;
}
emitToken(xid.module + " ");
}
if (wantSubroutine && (xid.subroutine.length() > 0)) {
if (wantSubroutine && (!xid.subroutine.empty())) {
if (needAspace) {
emitToken(" ");
needAspace = false;
Expand Down Expand Up @@ -516,7 +517,7 @@ namespace edm {
std::cerr << "[][][] in emit: lineLength is " << lineLength << '\n';
#endif

if (s.length() == 0) {
if (s.empty()) {
if (nl) {
(*os) << newline << std::flush;
charsOnLine = 0;
Expand Down Expand Up @@ -632,7 +633,7 @@ namespace edm {
}

void ELoutput::changeFile(const std::string& filename) {
os.reset(new std::ofstream(filename.c_str(), std::ios /*_base*/ ::app), close_and_delete());
os.reset(new std::ofstream(filename.c_str(), std::ios /*_base*/ ::app), close_and_delete()); //NOLINT
emitToken("\n=======================================================", true);
emitToken("\nError Log changed to this file\n");
std::string const& ftime = formatTime(time(nullptr)); // Change log 7
Expand Down
2 changes: 1 addition & 1 deletion FWCore/Modules/src/BranchIDListsModifierProducer.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class BranchIDListsModifierProducer : public edm::global::EDProducer<> {
public:
BranchIDListsModifierProducer(edm::ParameterSet const& iPSet);

void produce(edm::StreamID, edm::Event&, edm::EventSetup const&) const;
void produce(edm::StreamID, edm::Event&, edm::EventSetup const&) const override;

static void fillDescriptions(edm::ConfigurationDescriptions& iDesc);

Expand Down
2 changes: 1 addition & 1 deletion FWCore/Modules/src/EmptySourceFromEventIDs.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ bool EmptySourceFromEventIDs::setRunAndEventInfo(edm::EventID& event,
return false;
}

event = std::move(events_.back());
event = events_.back();
events_.pop_back();
return true;
}
Expand Down
10 changes: 5 additions & 5 deletions FWCore/Services/plugins/SimpleMemoryCheck.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -411,19 +411,19 @@ namespace edm {
// query values
size_t allocated, active, resident, mapped, metadata;
size_t len = sizeof(size_t);
if (je_mallctl("stats.allocated", &allocated, &len, 0, 0) != 0) {
if (je_mallctl("stats.allocated", &allocated, &len, nullptr, 0) != 0) {
return info;
}
if (je_mallctl("stats.active", &active, &len, 0, 0) != 0) {
if (je_mallctl("stats.active", &active, &len, nullptr, 0) != 0) {
return info;
}
if (je_mallctl("stats.resident", &resident, &len, 0, 0) != 0) {
if (je_mallctl("stats.resident", &resident, &len, nullptr, 0) != 0) {
return info;
}
if (je_mallctl("stats.mapped", &mapped, &len, 0, 0) != 0) {
if (je_mallctl("stats.mapped", &mapped, &len, nullptr, 0) != 0) {
return info;
}
if (je_mallctl("stats.metadata", &metadata, &len, 0, 0) != 0) {
if (je_mallctl("stats.metadata", &metadata, &len, nullptr, 0) != 0) {
return info;
}
info.allocated = allocated / 1024.0 / 1024.0;
Expand Down
7 changes: 3 additions & 4 deletions IOPool/Streamer/src/StreamerInputSource.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,13 +334,12 @@ namespace edm::streamer {
std::optional<ProductProvenance> productProvenance{std::in_place, spitem.branchID(), *spitem.parents()};
if (spitem.prod() != nullptr) {
FDEBUG(10) << "addproduct next " << spitem.branchID() << std::endl;
eventPrincipal.putOnRead(branchDesc,
std::unique_ptr<WrapperBase>(const_cast<WrapperBase*>(spitem.prod())),
std::move(productProvenance));
eventPrincipal.putOnRead(
branchDesc, std::unique_ptr<WrapperBase>(const_cast<WrapperBase*>(spitem.prod())), productProvenance);
FDEBUG(10) << "addproduct done" << std::endl;
} else {
FDEBUG(10) << "addproduct empty next " << spitem.branchID() << std::endl;
eventPrincipal.putOnRead(branchDesc, std::unique_ptr<WrapperBase>(), std::move(productProvenance));
eventPrincipal.putOnRead(branchDesc, std::unique_ptr<WrapperBase>(), productProvenance);
FDEBUG(10) << "addproduct empty done" << std::endl;
}
} else {
Expand Down
2 changes: 1 addition & 1 deletion PerfTools/AllocMonitorPreload/src/memory_proxies.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ namespace {
}
};

static ThreadTracker& getTracker() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't like this code change.

Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this all inside an anonymous namespace anyway ?

Copy link
Contributor Author

@smuzaffar smuzaffar Dec 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Isn't this all inside an anonymous namespace anyway ?

yes @fwyzard. clang-tidy removed the static due to exact same reason (https://clang.llvm.org/extra/clang-tidy/checks/readability/static-definition-in-anonymous-namespace.html)

warning: 'getTracker' is a static definition in anonymous namespace; static is redundant here [readability-static-definition-in-anonymous-namespace]

Copy link
Contributor

Choose a reason for hiding this comment

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

I should have looked closer. I had thought the function was actually a member function of a class, not a free function.

ThreadTracker& getTracker() {
static ThreadTracker s_tracker;
return s_tracker;
}
Expand Down
6 changes: 3 additions & 3 deletions Utilities/XrdAdaptor/src/XrdRequestManager.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -771,12 +771,12 @@ std::future<IOSize> XrdAdaptor::RequestManager::handle(std::shared_ptr<std::vect
std::shared_ptr<XrdAdaptor::ClientRequest> c_ptr1, c_ptr2;
std::future<IOSize> future1, future2;
if (!req1->empty()) {
c_ptr1.reset(new XrdAdaptor::ClientRequest(*this, req1));
c_ptr1 = std::make_shared<XrdAdaptor::ClientRequest>(*this, req1);
activeSources[0]->handle(c_ptr1);
future1 = c_ptr1->get_future();
}
if (!req2->empty()) {
c_ptr2.reset(new XrdAdaptor::ClientRequest(*this, req2));
c_ptr2 = std::make_shared<XrdAdaptor::ClientRequest>(*this, req2);
activeSources[1]->handle(c_ptr2);
future2 = c_ptr2->get_future();
}
Expand Down Expand Up @@ -1106,7 +1106,7 @@ void XrdAdaptor::RequestManager::OpenHandler::HandleResponseWithHosts(XrdCl::XRo
std::string excludeString;
Source::determineHostExcludeString(*m_file, hostList.get(), excludeString);

source.reset(new Source(now, std::move(m_file), excludeString));
source = std::make_shared<Source>(now, std::move(m_file), excludeString);
m_promise.set_value(source);
} else {
releaseFile = std::move(m_file);
Expand Down