From ad72f0d7875448e7cdb0b01ba87caf8b719c5fb0 Mon Sep 17 00:00:00 2001 From: Bernard Normier Date: Thu, 12 Dec 2024 10:48:24 -0500 Subject: [PATCH] Fix lints in generated C++ code --- .clang-tidy | 8 +++-- cpp/include/Ice/Exception.h | 10 +++---- cpp/include/Ice/Object.h | 4 +-- cpp/include/Ice/UserException.h | 2 +- cpp/include/Ice/Value.h | 6 ++-- cpp/src/slice2cpp/Gen.cpp | 46 ++++++++++++++++++++-------- cpp/test/Ice/custom/CustomBuffer.h | 48 +++++++++++++++++------------- cpp/test/Ice/custom/MyByteSeq.cpp | 4 +-- cpp/test/Ice/custom/MyByteSeq.h | 16 +++++----- 9 files changed, 86 insertions(+), 58 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index ffafc46bd63..a43e86204f0 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -6,14 +6,16 @@ Checks: modernize-*, -modernize-avoid-c-arrays, -modernize-use-trailing-return-type, + -modernize-concat-nested-namespaces, performance-*, -performance-avoid-endl ' -# WarningsAsErrors: '*' -HeaderFilterRegex: '' +WarningsAsErrors: '*' +HeaderFilterRegex: '.*' +ExcludeHeaderFilterRegex: 'include/.*' UseColor: true FormatStyle: 'file' -ExtraArgs: ['-std=c++17'] +ExtraArgs: ['-std=c++20'] CheckOptions: modernize-use-nullptr.NullMacros: 'NULL' # std::exception_ptr is a cheap to copy, pointer-like type; we pass it by value all the time. diff --git a/cpp/include/Ice/Exception.h b/cpp/include/Ice/Exception.h index dc83de24e15..d0a773194a9 100644 --- a/cpp/include/Ice/Exception.h +++ b/cpp/include/Ice/Exception.h @@ -54,14 +54,14 @@ namespace Ice * Returns the error message of this exception. * @return The error message. */ - const char* what() const noexcept override; + [[nodiscard]] const char* what() const noexcept override; /** * Returns the type ID of this exception. This corresponds to the Slice type ID for Slice-defined exceptions, * and to a similar fully scoped name for other exceptions. For example "::Ice::CommunicatorDestroyedException". * @return The type ID of this exception */ - virtual const char* ice_id() const noexcept = 0; + [[nodiscard]] virtual const char* ice_id() const noexcept = 0; /** * Outputs a description of this exception to a stream. This function is called by operator<<(std::ostream&, @@ -76,19 +76,19 @@ namespace Ice * Returns the name of the file where this exception was constructed. * @return The file name. */ - const char* ice_file() const noexcept; + [[nodiscard]] const char* ice_file() const noexcept; /** * Returns the line number where this exception was constructed. * @return The line number. */ - int ice_line() const noexcept; + [[nodiscard]] int ice_line() const noexcept; /** * Returns the stack trace at the point this exception was constructed. * @return The stack trace as a string, or an empty string if stack trace collection is not enabled. */ - std::string ice_stackTrace() const; + [[nodiscard]] std::string ice_stackTrace() const; /** * Enables the collection of stack traces for exceptions. On Windows, calling this function more than once is diff --git a/cpp/include/Ice/Object.h b/cpp/include/Ice/Object.h index 90a25118561..464e4e2f638 100644 --- a/cpp/include/Ice/Object.h +++ b/cpp/include/Ice/Object.h @@ -77,7 +77,7 @@ namespace Ice * @param current The Current object for the invocation. * @return The Slice type IDs of the interfaces supported by this object, in alphabetical order. */ - virtual std::vector ice_ids(const Current& current) const; + [[nodiscard]] virtual std::vector ice_ids(const Current& current) const; /// \cond INTERNAL void _iceD_ice_ids(IncomingRequest&, std::function); /// \endcond @@ -87,7 +87,7 @@ namespace Ice * @param current The Current object for the invocation. * @return The Slice type ID of the most-derived interface. */ - virtual std::string ice_id(const Current& current) const; + [[nodiscard]] virtual std::string ice_id(const Current& current) const; /// \cond INTERNAL void _iceD_ice_id(IncomingRequest&, std::function); /// \endcond diff --git a/cpp/include/Ice/UserException.h b/cpp/include/Ice/UserException.h index 5651952fb1e..992ce1452ea 100644 --- a/cpp/include/Ice/UserException.h +++ b/cpp/include/Ice/UserException.h @@ -36,7 +36,7 @@ namespace Ice virtual void _write(OutputStream*) const; virtual void _read(InputStream*); - virtual bool _usesClasses() const; + [[nodiscard]] virtual bool _usesClasses() const; /// \endcond protected: diff --git a/cpp/include/Ice/Value.h b/cpp/include/Ice/Value.h index f1db99e4279..c387984a4c4 100644 --- a/cpp/include/Ice/Value.h +++ b/cpp/include/Ice/Value.h @@ -60,14 +60,14 @@ namespace Ice * Creates a shallow polymorphic copy of this instance. * @return The cloned value. */ - ValuePtr ice_clone() const { return _iceCloneImpl(); } + [[nodiscard]] ValuePtr ice_clone() const { return _iceCloneImpl(); } /** * Obtains the sliced data associated with this instance. * @return The sliced data if the value has a preserved-slice base class and has been sliced during * unmarshaling of the value, nil otherwise. */ - SlicedDataPtr ice_getSlicedData() const; + [[nodiscard]] SlicedDataPtr ice_getSlicedData() const; /// \cond STREAM virtual void _iceWrite(Ice::OutputStream*) const; @@ -85,7 +85,7 @@ namespace Ice static std::shared_ptr clone(const T& other) { return std::make_shared(other); } }; - virtual ValuePtr _iceCloneImpl() const; + [[nodiscard]] virtual ValuePtr _iceCloneImpl() const; /// \endcond /// \cond STREAM diff --git a/cpp/src/slice2cpp/Gen.cpp b/cpp/src/slice2cpp/Gen.cpp index 6a889298aa2..bb6721f2da2 100644 --- a/cpp/src/slice2cpp/Gen.cpp +++ b/cpp/src/slice2cpp/Gen.cpp @@ -1111,9 +1111,14 @@ Slice::Gen::ForwardDeclVisitor::visitEnum(const EnumPtr& p) H << "class "; } H << getDeprecatedAttribute(p) << fixKwd(p->name()); - if (!unscoped && p->maxValue() <= 0xFF) + if (!unscoped) { - H << " : ::std::uint8_t"; + H << " : ::std::" << (p->maxValue() <= numeric_limits::max() ? "uint8_t" : "int32_t"); + + if (p->maxValue() > numeric_limits::max() && p->maxValue() <= numeric_limits::max()) + { + H << " // NOLINT:performance-enum-size"; + } } H << sb; @@ -1221,7 +1226,12 @@ Slice::Gen::ForwardDeclVisitor::visitConst(const ConstPtr& p) << typeToString(p->type(), false, scope, p->typeMetadata(), _useWstring) << " " << fixKwd(p->name()) << " " << getDeprecatedAttribute(p) << "= "; writeConstantValue(H, p->type(), p->valueType(), p->value(), _useWstring, p->typeMetadata(), scope); - H << ';' << sp; + H << ';'; + if (!isConstexprType(p->type())) + { + H << " // NOLINT:cert-err58-cpp"; + } + H << sp; } Slice::Gen::DefaultFactoryVisitor::DefaultFactoryVisitor(Output& c) : C(c), _factoryTableInitDone(false) {} @@ -1543,6 +1553,10 @@ Slice::Gen::ProxyVisitor::visitOperation(const OperationPtr& p) } H << nl << deprecatedAttribute << retS << ' ' << fixKwd(name) << spar << paramsDecl << contextDecl << epar << " const;"; + if (ret) + { + H << " // NOLINT:modernize-use-nodiscard"; + } C << sp; C << nl << retSImpl << nl << scoped << fixKwd(name) << spar << paramsImplDecl << "const ::Ice::Context& context" @@ -2068,7 +2082,7 @@ Slice::Gen::DataDefVisitor::visitExceptionStart(const ExceptionPtr& p) C << nl << "return \"" << p->scoped() << "\";"; C << eb; - H << sp << nl << _dllMemberExport << "const char* ice_id() const noexcept override;"; + H << sp << nl << _dllMemberExport << "[[nodiscard]] const char* ice_id() const noexcept override;"; C << sp << nl << "const char*" << nl << scoped.substr(2) << "::ice_id() const noexcept"; C << sb; C << nl << "return ice_staticId();"; @@ -2084,7 +2098,7 @@ Slice::Gen::DataDefVisitor::visitExceptionStart(const ExceptionPtr& p) { H << sp; H << nl << "/// \\cond STREAM"; - H << nl << _dllMemberExport << "bool _usesClasses() const override;"; + H << nl << _dllMemberExport << "[[nodiscard]] bool _usesClasses() const override;"; H << nl << "/// \\endcond"; C << sp; @@ -2218,7 +2232,7 @@ Slice::Gen::DataDefVisitor::visitClassDefStart(const ClassDefPtr& p) C << nl << "return \"" << p->scoped() << "\";"; C << eb; - H << sp << nl << _dllMemberExport << "const char* ice_id() const noexcept override;"; + H << sp << nl << _dllMemberExport << "[[nodiscard]] const char* ice_id() const noexcept override;"; C << sp << nl << "const char*" << nl << scoped.substr(2) << "::ice_id() const noexcept"; C << sb; C << nl << "return ice_staticId();"; @@ -2235,7 +2249,7 @@ Slice::Gen::DataDefVisitor::visitClassDefStart(const ClassDefPtr& p) H << sp; H << nl << "/// Creates a shallow polymorphic copy of this instance."; H << nl << "/// @return The cloned value."; - H << nl << p->name() << "Ptr ice_clone() const { return ::std::static_pointer_cast<" << name + H << nl << "[[nodiscard]] " << p->name() << "Ptr ice_clone() const { return ::std::static_pointer_cast<" << name << ">(_iceCloneImpl()); }"; return true; @@ -2302,7 +2316,7 @@ Slice::Gen::DataDefVisitor::visitClassDefEnd(const ClassDefPtr& p) } H << nl << name << "(const " << name << "&) = default;"; - H << sp << nl << _dllMemberExport << "::Ice::ValuePtr _iceCloneImpl() const override;"; + H << sp << nl << _dllMemberExport << "[[nodiscard]] ::Ice::ValuePtr _iceCloneImpl() const override;"; C << sp; C << nl << "::Ice::ValuePtr" << nl << scoped.substr(2) << "::_iceCloneImpl() const"; C << sb; @@ -2571,13 +2585,13 @@ Slice::Gen::InterfaceVisitor::visitInterfaceDefStart(const InterfaceDefPtr& p) H << nl << "/// Obtains a list of the Slice type IDs representing the interfaces supported by this object."; H << nl << "/// @param current The Current object for the invocation."; H << nl << "/// @return A list of fully-scoped type IDs."; - H << nl << "::std::vector<::std::string> ice_ids(const " << getUnqualified("::Ice::Current&", scope) + H << nl << "[[nodiscard]] ::std::vector<::std::string> ice_ids(const " << getUnqualified("::Ice::Current&", scope) << " current) const override;"; H << sp; H << nl << "/// Obtains a Slice type ID representing the most-derived interface supported by this object."; H << nl << "/// @param current The Current object for the invocation."; H << nl << "/// @return A fully-scoped type ID."; - H << nl << "::std::string ice_id(const " << getUnqualified("::Ice::Current&", scope) << " current) const override;"; + H << nl << "[[nodiscard]] ::std::string ice_id(const " << getUnqualified("::Ice::Current&", scope) << " current) const override;"; H << sp; H << nl << "/// Obtains the Slice type ID corresponding to this interface."; H << nl << "/// @return A fully-scoped type ID."; @@ -2743,12 +2757,20 @@ Slice::Gen::InterfaceVisitor::visitOperation(const OperationPtr& p) DocCommentPtr comment = p->parseDocComment(cppLinkFormatter); + string isConst = p->hasMetadata("cpp:const") ? " const" : ""; + string noDiscard = ""; + if (ret) { string typeS = inputTypeToString(ret, p->returnIsOptional(), interfaceScope, p->getMetadata(), _useWstring); responseParams.push_back(typeS + " " + returnValueParam); responseParamsDecl.push_back(typeS + " ret"); responseParamsImplDecl.push_back(typeS + " ret"); + + if (!amd && !isConst.empty()) + { + noDiscard = "[[nodiscard]] "; + } } string retS; @@ -2880,8 +2902,6 @@ Slice::Gen::InterfaceVisitor::visitOperation(const OperationPtr& p) C << eb; } - string isConst = p->hasMetadata("cpp:const") ? " const" : ""; - string opName = amd ? (name + "Async") : fixKwd(name); H << sp; @@ -2905,7 +2925,7 @@ Slice::Gen::InterfaceVisitor::visitOperation(const OperationPtr& p) postParams.push_back("@param " + currentParam + " The Current object for the invocation."); writeOpDocSummary(H, p, comment, pt, true, GenerateDeprecated::No, StringList(), postParams, returns); } - H << nl << "virtual " << retS << ' ' << opName << spar << params << epar << isConst << " = 0;"; + H << nl << noDiscard << "virtual " << retS << ' ' << opName << spar << params << epar << isConst << " = 0;"; H << nl << "/// \\cond INTERNAL"; H << nl << "void _iceD_" << name << "(::Ice::IncomingRequest&, ::std::function)" << isConst << ';'; diff --git a/cpp/test/Ice/custom/CustomBuffer.h b/cpp/test/Ice/custom/CustomBuffer.h index 65528c09b72..882c723f8d9 100644 --- a/cpp/test/Ice/custom/CustomBuffer.h +++ b/cpp/test/Ice/custom/CustomBuffer.h @@ -17,9 +17,9 @@ namespace Test template class CustomBuffer { public: - CustomBuffer() : _buf(nullptr), _count(0) {} + CustomBuffer() = default; - CustomBuffer(const CustomBuffer& o) : _buf(0), _count(o._count) + CustomBuffer(const CustomBuffer& o) : _count(o._count) { if (_count > 0) { @@ -31,7 +31,7 @@ namespace Test } } - CustomBuffer(CustomBuffer&& o) : _buf(o._buf), _count(o._count) + CustomBuffer(CustomBuffer&& o) noexcept : _buf(o._buf), _count(o._count) { o._buf = nullptr; o._count = 0; @@ -41,31 +41,37 @@ namespace Test CustomBuffer& operator=(const CustomBuffer& o) { - _count = o._count; - if (_count > 0) + if (this != &o) { - _buf = new T[_count]; - for (size_t i = 0; i < _count; ++i) + _count = o._count; + if (_count > 0) { - _buf[i] = o._buf[i]; + _buf = new T[_count]; + for (size_t i = 0; i < _count; ++i) + { + _buf[i] = o._buf[i]; + } } } return *this; } - CustomBuffer& operator=(CustomBuffer&& o) + CustomBuffer& operator=(CustomBuffer&& o) noexcept { - delete[] _buf; - _buf = o._buf; - _count = o._count; - o._buf = nullptr; - o._count = 0; + if (this != &o) + { + delete[] _buf; + _buf = o._buf; + _count = o._count; + o._buf = nullptr; + o._count = 0; + } return *this; } - size_t count() const { return _count; } + [[nodiscard]] size_t count() const { return _count; } - T* get() const { return _buf; } + [[nodiscard]] T* get() const { return _buf; } void set(T* buf, size_t count) { @@ -79,13 +85,13 @@ namespace Test _count = count; for (size_t i = 0; i < count; ++i) { - _buf[i] = static_cast(rand()); + _buf[i] = static_cast(rand()); // NOLINT } } private: - T* _buf; - size_t _count; + T* _buf{nullptr}; + size_t _count{0}; }; template bool operator!=(const CustomBuffer& lhs, const CustomBuffer& rhs) @@ -152,7 +158,7 @@ namespace Ice { std::pair a; stream->read(a); - size_t count = static_cast(a.second - a.first); + auto count = static_cast(a.second - a.first); if (count > 0) { auto b = new T[count]; @@ -164,7 +170,7 @@ namespace Ice } else { - v.set(0, 0); + v.set(nullptr, 0); } } }; diff --git a/cpp/test/Ice/custom/MyByteSeq.cpp b/cpp/test/Ice/custom/MyByteSeq.cpp index b0cc4f798ee..af00756cfdd 100644 --- a/cpp/test/Ice/custom/MyByteSeq.cpp +++ b/cpp/test/Ice/custom/MyByteSeq.cpp @@ -46,7 +46,7 @@ MyByteSeq::size() const } void -MyByteSeq::swap(MyByteSeq& seq) +MyByteSeq::swap(MyByteSeq& seq) noexcept { size_t tmpSize = seq._size; std::byte* tmpData = seq._data; @@ -84,7 +84,7 @@ MyByteSeq::operator=(const MyByteSeq& rhs) } MyByteSeq& -MyByteSeq::operator=(MyByteSeq&& rhs) +MyByteSeq::operator=(MyByteSeq&& rhs) noexcept { delete[] _data; _data = nullptr; diff --git a/cpp/test/Ice/custom/MyByteSeq.h b/cpp/test/Ice/custom/MyByteSeq.h index 7362f3e72d4..6031554c7e1 100644 --- a/cpp/test/Ice/custom/MyByteSeq.h +++ b/cpp/test/Ice/custom/MyByteSeq.h @@ -11,10 +11,10 @@ class MyByteSeq { public: - typedef std::byte* iterator; - typedef std::byte* const_iterator; + using iterator = std::byte*; + using const_iterator = std::byte *; - typedef std::byte value_type; + using value_type = std::byte; MyByteSeq(); MyByteSeq(size_t); @@ -22,13 +22,13 @@ class MyByteSeq MyByteSeq(MyByteSeq&&) noexcept; ~MyByteSeq(); - size_t size() const; - void swap(MyByteSeq&); - const_iterator begin() const; - const_iterator end() const; + [[nodiscard]] size_t size() const; + void swap(MyByteSeq&) noexcept ; + [[nodiscard]] const_iterator begin() const; + [[nodiscard]] const_iterator end() const; MyByteSeq& operator=(const MyByteSeq&); - MyByteSeq& operator=(MyByteSeq&&); + MyByteSeq& operator=(MyByteSeq&&) noexcept ; bool operator==(const MyByteSeq&) const;