Fix lints in generated C++ code (#3262)

.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.

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

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<std::string> ice_ids(const Current& current) const;
+        [[nodiscard]] virtual std::vector<std::string> ice_ids(const Current& current) const;
         /// \cond INTERNAL
         void _iceD_ice_ids(IncomingRequest&, std::function<void(OutgoingResponse)>);
         /// \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<void(OutgoingResponse)>);
         /// \endcond

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:

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<T> clone(const T& other) { return std::make_shared<CloneEnabler>(other); }
         };
 
-        virtual ValuePtr _iceCloneImpl() const;
+        [[nodiscard]] virtual ValuePtr _iceCloneImpl() const;
         /// \endcond
 
         /// \cond STREAM

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<uint8_t>::max() ? "uint8_t" : "int32_t");
+
+        if (p->maxValue() > numeric_limits<uint8_t>::max() && p->maxValue() <= numeric_limits<int16_t>::max())
+        {
+            H << " // NOLINT:performance-enum-size";
+        }
     }
     H << sb;
 
@@ -1221,7 +1226,13 @@ 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())) // i.e. string or wstring
+    {
+        // The string/wstring constructor can throw, which produces a clang-tidy lint for const or static objects.
+        H << " // NOLINT:cert-err58-cpp";
+    }
+    H << sp;
 }
 
 Slice::Gen::DefaultFactoryVisitor::DefaultFactoryVisitor(Output& c) : C(c), _factoryTableInitDone(false) {}
@@ -1544,6 +1555,12 @@ Slice::Gen::ProxyVisitor::visitOperation(const OperationPtr& p)
     H << nl << deprecatedAttribute << retS << ' ' << fixKwd(name) << spar << paramsDecl << contextDecl << epar
       << " const;";
 
+    // We don't want to add [[nodiscard]] to proxy member functions.
+    if (ret && p->outParameters().empty())
+    {
+        H << " // NOLINT:modernize-use-nodiscard";
+    }
+
     C << sp;
     C << nl << retSImpl << nl << scoped << fixKwd(name) << spar << paramsImplDecl << "const ::Ice::Context& context"
       << epar << " const";
@@ -2068,7 +2085,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 +2101,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 +2235,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 +2252,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 +2319,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 +2588,14 @@ 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 +2761,23 @@ 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");
+
+        // clang-tidy produces a lint for a const member function that returns a value, is not [[nodiscard]] and
+        // has no non-const reference parameters (= Slice out parameters).
+        // See https://clang.llvm.org/extra/clang-tidy/checks/modernize/use-nodiscard.html
+        if (!amd && !isConst.empty() && p->outParameters().empty())
+        {
+            noDiscard = "[[nodiscard]] ";
+        }
     }
 
     string retS;
@@ -2880,8 +2909,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 +2932,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<void(::Ice::OutgoingResponse)>)"
       << isConst << ';';

cpp/test/Ice/custom/CustomBuffer.h

@@ -17,9 +17,9 @@ namespace Test
     template<typename T> 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<T>(rand());
+                _buf[i] = static_cast<T>(rand()); // NOLINT
             }
         }
 
     private:
-        T* _buf;
-        size_t _count;
+        T* _buf{nullptr};
+        size_t _count{0};
     };
 
     template<typename T> bool operator!=(const CustomBuffer<T>& lhs, const CustomBuffer<T>& rhs)
@@ -152,7 +158,7 @@ namespace Ice
         {
             std::pair<const T*, const T*> a;
             stream->read(a);
-            size_t count = static_cast<size_t>(a.second - a.first);
+            auto count = static_cast<size_t>(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);
             }
         }
     };

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;