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.

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

Fix more C++ modernize lints #3353

Merged
merged 20 commits into from
Jan 14, 2025
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
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
1 change: 1 addition & 0 deletions .clang-tidy
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ Checks:
-modernize-avoid-c-arrays,
-modernize-use-trailing-return-type,
-modernize-concat-nested-namespaces,
-modernize-use-default-member-init,
Copy link
Member Author

Choose a reason for hiding this comment

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

Suppressed for now as -fix doesn't deal well with macro conditionals and produces incorrect code.

performance-*,
-performance-avoid-endl
'
Expand Down
2 changes: 1 addition & 1 deletion cpp/include/Ice/Communicator.h
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ namespace Ice
* @return A proxy to the main ("") facet of the Admin object, or nullopt if no Admin object is configured.
* @see #createAdmin
*/
std::optional<ObjectPrx> getAdmin() const; // NOLINT:modernize-use-nodiscard
std::optional<ObjectPrx> getAdmin() const; // NOLINT(modernize-use-nodiscard)
bernardnormier marked this conversation as resolved.
Show resolved Hide resolved

/**
* Add a new facet to the Admin object. Adding a servant with a facet that is already registered throws
Expand Down
2 changes: 2 additions & 0 deletions cpp/include/Ice/Config.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,11 @@
#endif

// The Ice version.
// NOLINTBEGIN(modernize-macro-to-enum)
#define ICE_STRING_VERSION "3.8.0-alpha.0" // "A.B.C", with A=major, B=minor, C=patch
#define ICE_INT_VERSION 30850 // AABBCC, with AA=major, BB=minor, CC=patch
#define ICE_SO_VERSION "38a0" // "ABC", with A=major, B=minor, C=patch
// NOLINTEND(modernize-macro-to-enum)

#if !defined(ICE_BUILDING_ICE) && defined(ICE_API_EXPORTS)
# define ICE_BUILDING_ICE
Expand Down
2 changes: 1 addition & 1 deletion cpp/include/Ice/IconvStringConverter.h
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ namespace IceInternal
assert(rs == 0);
char* inbuf = reinterpret_cast<char*>(const_cast<std::byte*>(sourceStart));
assert(sourceEnd > sourceStart);
size_t inbytesleft = static_cast<size_t>(sourceEnd - sourceStart);
auto inbytesleft = static_cast<size_t>(sourceEnd - sourceStart);

char* outbuf = nullptr;
size_t outbytesleft = 0;
Expand Down
4 changes: 2 additions & 2 deletions cpp/include/Ice/InputStream.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ namespace Ice
/// \cond INTERNAL
template<typename T> inline void patchValue(void* addr, const ValuePtr& v)
{
std::shared_ptr<T>* ptr = static_cast<std::shared_ptr<T>*>(addr);
auto* ptr = static_cast<std::shared_ptr<T>*>(addr);
*ptr = std::dynamic_pointer_cast<T>(v);
if (v && !(*ptr))
{
Expand Down Expand Up @@ -313,7 +313,7 @@ namespace Ice
{
std::uint8_t byte;
read(byte);
unsigned char val = static_cast<unsigned char>(byte);
auto val = static_cast<unsigned char>(byte);
if (val == 255)
{
std::int32_t v;
Expand Down
50 changes: 19 additions & 31 deletions cpp/include/Ice/MetricsAdminI.h
Original file line number Diff line number Diff line change
Expand Up @@ -125,12 +125,9 @@ namespace IceInternal
~EntryT()
{
assert(_object->total > 0);
for (typename std::map<std::string, std::pair<MetricsMapIPtr, SubMapMember>>::const_iterator p =
_subMaps.begin();
p != _subMaps.end();
++p)
for (const auto& subMap : _subMaps)
{
p->second.first->destroy(); // Break cyclic reference counts.
subMap.second.first->destroy(); // Break cyclic reference counts.
}
}

Expand All @@ -148,8 +145,7 @@ namespace IceInternal
MetricsMapIPtr m;
{
std::lock_guard lock(_map->_mutex);
typename std::map<std::string, std::pair<MetricsMapIPtr, SubMapMember>>::iterator p =
_subMaps.find(mapName);
auto p = _subMaps.find(mapName);
if (p == _subMaps.end())
{
std::pair<MetricsMapIPtr, SubMapMember> map = _map->createSubMap(mapName);
Expand Down Expand Up @@ -200,10 +196,7 @@ namespace IceInternal
[[nodiscard]] IceMX::MetricsPtr clone() const
{
TPtr metrics = std::dynamic_pointer_cast<T>(_object->ice_clone());
for (typename std::map<std::string, std::pair<MetricsMapIPtr, SubMapMember>>::const_iterator p =
_subMaps.begin();
p != _subMaps.end();
++p)
for (auto p = _subMaps.begin(); p != _subMaps.end(); ++p)
Copy link
Member Author

Choose a reason for hiding this comment

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

I don't know why -fix didn't convert this code in a loop.

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, in this case, I prefer to leave it alone as the syntax is hard to decipher.

{
metrics.get()->*p->second.second = p->second.first->getMetrics();
}
Expand Down Expand Up @@ -278,10 +271,9 @@ namespace IceInternal
IceMX::MetricsMap objects;

std::lock_guard lock(_mutex);
for (typename std::map<std::string, EntryTPtr>::const_iterator p = _objects.begin(); p != _objects.end();
++p)
for (const auto& object : _objects)
{
objects.push_back(p->second->clone());
objects.push_back(object.second->clone());
}
return objects;
}
Expand All @@ -291,10 +283,9 @@ namespace IceInternal
IceMX::MetricsFailuresSeq failures;

std::lock_guard lock(_mutex);
for (typename std::map<std::string, EntryTPtr>::const_iterator p = _objects.begin(); p != _objects.end();
++p)
for (const auto& object : _objects)
{
IceMX::MetricsFailures f = p->second->getFailures();
IceMX::MetricsFailures f = object.second->getFailures();
if (!f.failures.empty())
{
failures.push_back(f);
Expand All @@ -306,7 +297,7 @@ namespace IceInternal
IceMX::MetricsFailures getFailures(const std::string& id) override
{
std::lock_guard lock(_mutex);
typename std::map<std::string, EntryTPtr>::const_iterator p = _objects.find(id);
auto p = _objects.find(id);
if (p != _objects.end())
{
return p->second->getFailures();
Expand All @@ -316,8 +307,7 @@ namespace IceInternal

std::pair<MetricsMapIPtr, SubMapMember> createSubMap(const std::string& subMapName)
{
typename std::map<std::string, std::pair<SubMapMember, MetricsMapIPtr>>::const_iterator p =
_subMaps.find(subMapName);
auto p = _subMaps.find(subMapName);
if (p != _subMaps.end())
{
return std::pair<MetricsMapIPtr, SubMapMember>(
Expand All @@ -332,17 +322,17 @@ namespace IceInternal
//
// Check the accept and reject filters.
//
for (std::vector<RegExpPtr>::const_iterator p = _accept.begin(); p != _accept.end(); ++p)
for (const auto& filter : _accept)
{
if (!(*p)->match(helper, false))
if (!filter->match(helper, false))
{
return nullptr;
}
}

for (std::vector<RegExpPtr>::const_iterator p = _reject.begin(); p != _reject.end(); ++p)
for (const auto& filter : _reject)
{
if ((*p)->match(helper, true))
if (filter->match(helper, true))
{
return nullptr;
}
Expand All @@ -361,10 +351,8 @@ namespace IceInternal
else
{
std::ostringstream os;
std::vector<std::string>::const_iterator q = _groupBySeparators.begin();
for (std::vector<std::string>::const_iterator p = _groupByAttributes.begin();
p != _groupByAttributes.end();
++p)
auto q = _groupBySeparators.begin();
for (auto p = _groupByAttributes.begin(); p != _groupByAttributes.end(); ++p)
{
os << helper(*p);
if (q != _groupBySeparators.end())
Expand Down Expand Up @@ -395,7 +383,7 @@ namespace IceInternal
return previous;
}

typename std::map<std::string, EntryTPtr>::const_iterator p = _objects.find(key);
auto p = _objects.find(key);
if (p == _objects.end())
{
TPtr t = std::make_shared<T>();
Expand Down Expand Up @@ -443,7 +431,7 @@ namespace IceInternal
if (static_cast<int>(_detachedQueue.size()) == _retain)
{
// Remove entries which are no longer detached
typename std::list<EntryTPtr>::iterator p = _detachedQueue.begin();
auto p = _detachedQueue.begin();
while (p != _detachedQueue.end())
{
if (!(*p)->isDetached())
Expand Down Expand Up @@ -563,7 +551,7 @@ namespace IceInternal
std::shared_ptr<MetricsMapFactoryT<MetricsType>> factory;
{
std::lock_guard lock(_mutex);
std::map<std::string, MetricsMapFactoryPtr>::const_iterator p = _factories.find(map);
auto p = _factories.find(map);
if (p == _factories.end())
{
return;
Expand Down
49 changes: 23 additions & 26 deletions cpp/include/Ice/MetricsObserverI.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,17 +82,15 @@ namespace IceMX

~AttributeResolverT()
{
for (typename std::map<std::string, Resolver*>::iterator p = _attributes.begin();
p != _attributes.end();
++p)
for (const auto& attribute : _attributes)
{
delete p->second;
delete attribute.second;
}
}

std::string operator()(const Helper* helper, const std::string& attribute) const
{
typename std::map<std::string, Resolver*>::const_iterator p = _attributes.find(attribute);
auto p = _attributes.find(attribute);
if (p == _attributes.end())
{
if (attribute == "none")
Expand Down Expand Up @@ -344,25 +342,25 @@ namespace IceMX
void detach() override
{
std::chrono::microseconds lifetime = _previousDelay + _watch.stop();
for (typename EntrySeqType::const_iterator p = _objects.begin(); p != _objects.end(); ++p)
for (const auto& object : _objects)
{
(*p)->detach(lifetime.count());
object->detach(lifetime.count());
}
}

void failed(const std::string& exceptionName) override
{
for (typename EntrySeqType::const_iterator p = _objects.begin(); p != _objects.end(); ++p)
for (const auto& object : _objects)
{
(*p)->failed(exceptionName);
object->failed(exceptionName);
}
}

template<typename Function> void forEach(const Function& func)
{
for (typename EntrySeqType::const_iterator p = _objects.begin(); p != _objects.end(); ++p)
for (const auto& object : _objects)
{
(*p)->execute(func);
object->execute(func);
}
}

Expand All @@ -380,23 +378,22 @@ namespace IceMX
// Detach entries from previous observer which are no longer
// attached to this new observer.
//
for (typename EntrySeqType::const_iterator p = previous->_objects.begin(); p != previous->_objects.end();
++p)
for (const auto& previousObject : previous->_objects)
{
if (find(_objects.begin(), _objects.end(), *p) == _objects.end())
if (find(_objects.begin(), _objects.end(), previousObject) == _objects.end())
{
(*p)->detach(_previousDelay.count());
previousObject->detach(_previousDelay.count());
}
}
}

EntryPtrType getEntry(IceInternal::MetricsMapT<MetricsType>* map)
{
for (typename EntrySeqType::const_iterator p = _objects.begin(); p != _objects.end(); ++p)
for (const auto& object : _objects)
{
if ((*p)->getMap() == map)
if (object->getMap() == map)
{
return *p;
return object;
}
}
return nullptr;
Expand All @@ -407,10 +404,10 @@ namespace IceMX
getObserver(const std::string& mapName, const MetricsHelperT<ObserverMetricsType>& helper)
{
std::vector<typename IceInternal::MetricsMapT<ObserverMetricsType>::EntryTPtr> metricsObjects;
for (typename EntrySeqType::const_iterator p = _objects.begin(); p != _objects.end(); ++p)
for (const auto& object : _objects)
{
typename IceInternal::MetricsMapT<ObserverMetricsType>::EntryTPtr e =
(*p)->getMatching(mapName, helper);
object->getMatching(mapName, helper);
if (e)
{
metricsObjects.push_back(e);
Expand Down Expand Up @@ -465,9 +462,9 @@ namespace IceMX
}

typename ObserverImplType::EntrySeqType metricsObjects;
for (typename MetricsMapSeqType::const_iterator p = _maps.begin(); p != _maps.end(); ++p)
for (const auto& map : _maps)
{
typename ObserverImplType::EntryPtrType entry = (*p)->getMatching(helper);
typename ObserverImplType::EntryPtrType entry = map->getMatching(helper);
if (entry)
{
metricsObjects.push_back(entry);
Expand Down Expand Up @@ -501,9 +498,9 @@ namespace IceMX
}

typename ObserverImplType::EntrySeqType metricsObjects;
for (typename MetricsMapSeqType::const_iterator p = _maps.begin(); p != _maps.end(); ++p)
for (const auto& map : _maps)
{
typename ObserverImplType::EntryPtrType entry = (*p)->getMatching(helper, old->getEntry(p->get()));
typename ObserverImplType::EntryPtrType entry = map->getMatching(helper, old->getEntry(map.get()));
if (entry)
{
metricsObjects.push_back(entry);
Expand Down Expand Up @@ -541,9 +538,9 @@ namespace IceMX

std::vector<IceInternal::MetricsMapIPtr> maps = _metrics->getMaps(_name);
_maps.clear();
for (std::vector<IceInternal::MetricsMapIPtr>::const_iterator p = maps.begin(); p != maps.end(); ++p)
for (auto& map : maps)
bernardnormier marked this conversation as resolved.
Show resolved Hide resolved
{
_maps.push_back(std::dynamic_pointer_cast<IceInternal::MetricsMapT<MetricsType>>(*p));
_maps.push_back(std::dynamic_pointer_cast<IceInternal::MetricsMapT<MetricsType>>(map));
assert(_maps.back());
}
_enabled.exchange(_maps.empty() ? 0 : 1);
Expand Down
4 changes: 2 additions & 2 deletions cpp/include/Ice/OutputStream.h
Original file line number Diff line number Diff line change
Expand Up @@ -619,7 +619,7 @@ namespace Ice
*/
void write(std::string_view v, bool convert = true)
{
std::int32_t sz = static_cast<std::int32_t>(v.size());
auto sz = static_cast<std::int32_t>(v.size());
if (convert && sz > 0)
{
writeConverted(v.data(), static_cast<size_t>(sz));
Expand All @@ -645,7 +645,7 @@ namespace Ice
*/
void write(const char* vdata, size_t vsize, bool convert = true)
{
std::int32_t sz = static_cast<std::int32_t>(vsize);
auto sz = static_cast<std::int32_t>(vsize);
if (convert && sz > 0)
{
writeConverted(vdata, vsize);
Expand Down
Loading
Loading