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

[Tf] Update TF_FOR_ALL to C++11 iterator #3520

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from
Open

[Tf] Update TF_FOR_ALL to C++11 iterator #3520

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
c298886
update TF_FOR_ALL to C++11 iterator
Cewein Feb 5, 2025
18b48d3
change auto to TYPE name for clarity
Cewein Feb 5, 2025
51329be
fix value acces
Cewein Feb 12, 2025
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
6 changes: 3 additions & 3 deletions pxr/base/tf/errorMark.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,11 +109,11 @@ TfReportActiveErrorMarks()
localStacks = TfErrorMark_GetActiveMarkStacks();
}

TF_FOR_ALL(i, localStacks) {
for(const auto& i: localStacks) {
Copy link
Member

Choose a reason for hiding this comment

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

Throughout the diff, let's match the style of the surrounding code, with a space between for and the paren.

In general the convention with auto is to only use it when the type is visible in the context. Like if std::vector<float> was declared on a previous line, auto would obvious from context. In the case of the _ActiveMarkStacksMap we have to go digging to discover that the iteration will be on a series of vector<uintptr_t> so it would help readability if the line read

for (const vector::<uintptr_t>& i: localStacks) {
Since the goal of this PR is to improve readability by expanding the TF_FOR_ALL macro, I think it's worthwhile to think about each replacement.

Copy link
Author

@Cewein Cewein Feb 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

this make sense, I was not sure on how to update with auto or the actual type since sometime the auto as sometime it might translate to lengthy types.

printf("== TfErrorMark @ %p created from ===========================\n",
i->first);
i.first);
std::stringstream ss;
ArchPrintStackFrames(ss, i->second);
ArchPrintStackFrames(ss, i.second);
printf("%s\n", ss.str().c_str());
}
}
Expand Down
64 changes: 32 additions & 32 deletions pxr/base/tf/mallocTag.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,8 @@ Tf_MallocTagStringMatchTable::SetMatchList(const std::string& matchList)
{
_matchStrings.clear();
std::vector<std::string> items = TfStringTokenize(matchList, ",\t\n");
TF_FOR_ALL(i, items) {
_matchStrings.push_back(_MatchString(TfStringTrim(*i, " ")));
for(std::string i: items) {
Copy link
Member

Choose a reason for hiding this comment

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

could be a reference here, std::string&. could reserve _matchStrings to matchList.size even though the existing code didn't do it. emplace_back would allow construction in place. Another optimization that didn't originally exist, but since we're touching the loop :)

_matchStrings.push_back(_MatchString(TfStringTrim(i, " ")));
}
}

Expand Down Expand Up @@ -681,12 +681,12 @@ Tf_MallocGlobalData::_SetTraceNames(const std::string& matchList)
_traceMatchTable.SetMatchList(matchList);

// Update trace flag on every existing call site.
TF_FOR_ALL(i, _callSiteTable) {
if (_traceMatchTable.Match(i->second->_name.get())) {
i->second->_flags |= Tf_MallocCallSite::_TraceFlag;
for(const auto& i: _callSiteTable) {
if (_traceMatchTable.Match(i.second->_name.get())) {
i.second->_flags |= Tf_MallocCallSite::_TraceFlag;
}
else {
i->second->_flags &= ~Tf_MallocCallSite::_TraceFlag;
i.second->_flags &= ~Tf_MallocCallSite::_TraceFlag;
}
}
}
Expand Down Expand Up @@ -757,12 +757,12 @@ Tf_MallocGlobalData::_SetDebugNames(const std::string& matchList)
_debugMatchTable.SetMatchList(matchList);

// Update debug flag on every existing call site.
TF_FOR_ALL(i, _callSiteTable) {
if (_debugMatchTable.Match(i->second->_name.get())) {
i->second->_flags |= Tf_MallocCallSite::_DebugFlag;
for(const auto& i: _callSiteTable) {
if (_debugMatchTable.Match(i.second->_name.get())) {
i.second->_flags |= Tf_MallocCallSite::_DebugFlag;
}
else {
i->second->_flags &= ~Tf_MallocCallSite::_DebugFlag;
i.second->_flags &= ~Tf_MallocCallSite::_DebugFlag;
}
}
}
Expand Down Expand Up @@ -824,10 +824,10 @@ Tf_MallocGlobalData::_BuildUniqueMallocStacks(TfMallocTag::CallTree* tree)
vector<uintptr_t>, _MallocStackData, _HashMallocStack> _Map;
_Map map;

TF_FOR_ALL(it, _callStackTable) {
for(const auto& it: _callStackTable) {
// Since _callStackTable does not change at this point it is
// ok to store the address of the malloc stack in the data.
const TfMallocTag::CallStackInfo &stackInfo = it->second;
const TfMallocTag::CallStackInfo &stackInfo = it.second;
_MallocStackData data = { &stackInfo.stack, 0, 0 };

pair<_Map::iterator, bool> insertResult = map.insert(
Expand All @@ -841,8 +841,8 @@ Tf_MallocGlobalData::_BuildUniqueMallocStacks(TfMallocTag::CallTree* tree)
// Sort the malloc stack data by allocation size.
std::vector<const _MallocStackData *> sortedStackData;
sortedStackData.reserve(map.size());
TF_FOR_ALL(it, map) {
sortedStackData.push_back(&it->second);
for(const auto& it: map) {
sortedStackData.push_back(&it.second);
}

std::sort(
Expand Down Expand Up @@ -958,8 +958,8 @@ TfMallocTag::GetCapturedMallocStacks()
traces.swap(_mallocGlobalData->_callStackTable);
}

TF_FOR_ALL(i, traces) {
result.push_back(i->second.stack);
for(const auto& i: traces) {
result.push_back(i.second.stack);
}

return result;
Expand Down Expand Up @@ -1088,8 +1088,8 @@ _GetCallSites(TfMallocTag::CallTree::PathNode* node,
Tf_GetOrCreateCallSite(table, node->siteName.c_str());
site->_totalBytes += node->nBytesDirect;

TF_FOR_ALL(pi, node->children) {
_GetCallSites(&(*pi), table);
for(auto& pi: node->children) {
_GetCallSites(&pi, table);
}
}

Expand Down Expand Up @@ -1118,13 +1118,13 @@ TfMallocTag::GetCallTree(CallTree* tree, bool skipRepeated)

// Copy the callsites into the calltree
tree->callSites.reserve(callSiteTable.size());
TF_FOR_ALL(csi, callSiteTable) {
for(const auto& csi: callSiteTable) {
CallTree::CallSite cs = {
csi->second->_name.get(),
static_cast<size_t>(csi->second->_totalBytes)
csi.second->_name.get(),
static_cast<size_t>(csi.second->_totalBytes)
};
tree->callSites.push_back(cs);
delete csi->second;
delete csi.second;
}

gd->_BuildUniqueMallocStacks(tree);
Expand Down Expand Up @@ -1228,11 +1228,11 @@ _GetAsCommaSeparatedString(size_t number)
string str = TfStringPrintf("%ld", number);
size_t n = str.size();

TF_FOR_ALL(it, str) {
for(char it: str) {
if (n < str.size() && n%3 == 0) {
result.push_back(',');
}
result.push_back(*it);
result.push_back(it);
n--;
}
return result;
Expand Down Expand Up @@ -1345,8 +1345,8 @@ _PrintMallocCallSites(

// Use a map to sort by allocation size.
map<size_t, const string *> map;
TF_FOR_ALL(csi, callSites) {
map.insert(make_pair(csi->nBytes, &csi->name));
for(const auto& csi: callSites) {
map.insert(make_pair(csi.nBytes, &csi.name));
}

// XXX:cleanup We should pass in maxNameWidth.
Expand Down Expand Up @@ -1403,8 +1403,8 @@ _GetNumAllocationInSubTree(
const TfMallocTag::CallTree::PathNode &node)
{
int64_t nAllocations = node.nAllocations;
TF_FOR_ALL(it, node.children) {
nAllocations += _GetNumAllocationInSubTree(*it);
for(const auto& it: node.children) {
nAllocations += _GetNumAllocationInSubTree(it);
}
return nAllocations;
}
Expand Down Expand Up @@ -1451,15 +1451,15 @@ _ReportMallocNode(
// (i.e. that sorting is a view into the unaltered source data).
std::vector<const TfMallocTag::CallTree::PathNode *> sortedChildren;
sortedChildren.reserve(node.children.size());
TF_FOR_ALL(it, node.children) {
sortedChildren.push_back(&(*it));
for(const auto& it: node.children) {
sortedChildren.push_back(&it);
}

std::sort(
sortedChildren.begin(), sortedChildren.end(), _MallocPathNodeLessThan);

TF_FOR_ALL(it, sortedChildren) {
_ReportMallocNode(out, **it, level+1);
for(const auto& it: sortedChildren) {
_ReportMallocNode(out, *it, level+1);
}
}

Expand Down
8 changes: 4 additions & 4 deletions pxr/base/tf/notice.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,8 @@ TfNotice::Revoke(Key& key)
void
TfNotice::Revoke(Keys* keys)
{
TF_FOR_ALL(i, *keys) {
Revoke(*i);
for(auto& i: *keys) {
Revoke(i);
}
keys->clear();
}
Expand All @@ -158,8 +158,8 @@ TfNotice::RevokeAndWait(Key& key)
void
TfNotice::RevokeAndWait(Keys* keys)
{
TF_FOR_ALL(i, *keys) {
RevokeAndWait(*i);
for(auto& i: *keys) {
RevokeAndWait(i);
}
keys->clear();
}
Expand Down
30 changes: 15 additions & 15 deletions pxr/base/tf/noticeRegistry.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,17 +116,17 @@ _BeginSend(const TfNotice &notice,
const std::type_info &senderType,
const std::vector<TfNotice::WeakProbePtr> &probes)
{
TF_FOR_ALL(i, probes)
if (*i)
(*i)->BeginSend(notice, sender, senderType);
for(const auto& i: probes)
if (i)
i->BeginSend(notice, sender, senderType);
}

void
Tf_NoticeRegistry::_EndSend(const std::vector<TfNotice::WeakProbePtr> &probes)
{
TF_FOR_ALL(i, probes)
if (*i)
(*i)->EndSend();
for(const auto& i: probes)
if (i)
i->EndSend();
}

void
Expand All @@ -138,19 +138,19 @@ _BeginDelivery(const TfNotice &notice,
const std::type_info &listenerType,
const std::vector<TfNotice::WeakProbePtr> &probes)
{
TF_FOR_ALL(i, probes)
if (*i)
(*i)->BeginDelivery(notice, sender,
for(const TfNotice::WeakProbePtr& i: probes)
if (i)
i->BeginDelivery(notice, sender,
senderType, listener, listenerType);
}

void
Tf_NoticeRegistry::
_EndDelivery(const std::vector<TfNotice::WeakProbePtr> &probes)
{
TF_FOR_ALL(i, probes)
if (*i)
(*i)->EndDelivery();
for(const TfNotice::WeakProbePtr& i: probes)
if (i)
i->EndDelivery();
}

TfNotice::Key
Expand Down Expand Up @@ -229,9 +229,9 @@ Tf_NoticeRegistry::_Send(const TfNotice &n, const TfType & noticeType,
// Copy off a list of the probes.
_Lock lock(_probeMutex);
probeList.reserve(_probes.size());
TF_FOR_ALL(i, _probes) {
if (*i) {
probeList.push_back(*i);
for(const auto& i: _probes) {
if (i) {
probeList.push_back(i);
}
}
doProbing = !probeList.empty();
Expand Down
4 changes: 2 additions & 2 deletions pxr/base/tf/pyContainerConversions.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ struct TfPySequenceToPython
static PyObject* convert(ContainerType const &c)
{
pxr_boost::python::list result;
TF_FOR_ALL(i, c) {
result.append(*i);
for(const auto& i: c) {
result.append(i);
}
return pxr_boost::python::incref(result.ptr());
}
Expand Down
4 changes: 2 additions & 2 deletions pxr/base/tf/pyEnum.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ Tf_PyEnumRegistry::Tf_PyEnumRegistry()
Tf_PyEnumRegistry::~Tf_PyEnumRegistry()
{
// release our references on all the objects we own.
TF_FOR_ALL(i, _objectsToEnums)
decref(i->first);
for(const auto& i: _objectsToEnums)
decref(i.first);
}
// CODE_COVERAGE_ON

Expand Down
4 changes: 2 additions & 2 deletions pxr/base/tf/pyError.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,8 @@ TfPyConvertPythonExceptionToTfErrors()
extract<vector<TfError> > extractor(args);
if (extractor.check()) {
vector<TfError> errs = extractor();
TF_FOR_ALL(e, errs)
TfDiagnosticMgr::GetInstance().AppendError(*e);
for(const TfError& e: errs)
TfDiagnosticMgr::GetInstance().AppendError(e);
}
} else {
TF_ERROR(exc, TF_PYTHON_EXCEPTION, "Tf Python Exception");
Expand Down
18 changes: 9 additions & 9 deletions pxr/base/tf/refPtrTracker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,9 +152,9 @@ void
TfRefPtrTracker::ReportAllWatchedCounts(std::ostream& stream) const
{
stream << "TfRefPtrTracker watched counts:" << std::endl;
TF_FOR_ALL(i, _watched) {
stream << " " << i->first << ": " << i->second
<< " (type " << _GetDemangled(i->first) << ")"
for(const auto& i: _watched) {
stream << " " << i.first << ": " << i.second
<< " (type " << _GetDemangled(i.first) << ")"
<< std::endl;
}
}
Expand All @@ -164,9 +164,9 @@ TfRefPtrTracker::ReportAllTraces(std::ostream& stream) const
{
stream << "TfRefPtrTracker traces:" << std::endl;
_Lock lock(_mutex);
TF_FOR_ALL(i, _traces) {
const Trace& trace = i->second;
stream << " Owner: " << i->first
for(const auto& i: _traces) {
const Trace& trace = i.second;
stream << " Owner: " << i.first
<< " " << _type[trace.type] << " " << trace.obj << ":"
<< std::endl;
stream << "=============================================================="
Expand Down Expand Up @@ -195,10 +195,10 @@ TfRefPtrTracker::ReportTracesForWatched(
<< " (type " << _GetDemangled(watched) << ")" << std::endl;

// Loop over all traces and report the ones that are watching watched.
TF_FOR_ALL(i, _traces) {
const Trace& trace = i->second;
for(const auto& i: _traces) {
const Trace& trace = i.second;
if (trace.obj == watched) {
stream << " Owner: " << i->first
stream << " Owner: " << i.first
<< " " << _type[trace.type] << ":"
<< std::endl;
stream << "=============================================================="
Expand Down
Loading