Add clang-tidy config and tidy-up generated code (#3254)

zeroc-ice · Dec 11, 2024 · fedd40a · fedd40a
1 parent af4ac49
commit fedd40a
Show file tree

Hide file tree

Showing 7 changed files with 100 additions and 11 deletions.
diff --git a/.clang-tidy b/.clang-tidy
@@ -0,0 +1,20 @@
+Checks:
+'-*,
+  clang-analyzer-*,
+  -clang-diagnostic-shadow-uncaptured-local,
+  cert-*,
+  modernize-*,
+  -modernize-avoid-c-arrays,
+  -modernize-use-trailing-return-type,
+  performance-*,
+  -performance-avoid-endl
+'
+# WarningsAsErrors: '*'
+HeaderFilterRegex: ''
+UseColor: true
+FormatStyle: 'file'
+ExtraArgs: ['-std=c++17']
+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.
+  performance-unnecessary-value-param.AllowedTypes: 'exception_ptr$;'
diff --git a/cpp/include/Ice/FactoryTable.h b/cpp/include/Ice/FactoryTable.h
@@ -78,7 +78,7 @@ namespace IceInternal
     class ICE_API CompactIdInit
     {
     public:
-        CompactIdInit(std::string_view, int) noexcept;
+        CompactIdInit(const char* typeId, int compactId) noexcept;
         ~CompactIdInit();
 
     private:

diff --git a/cpp/src/Ice/FactoryTable.cpp b/cpp/src/Ice/FactoryTable.cpp
@@ -190,7 +190,7 @@ IceInternal::FactoryTableInit::~FactoryTableInit()
     }
 }
 
-IceInternal::CompactIdInit::CompactIdInit(string_view typeId, int compactId) noexcept : _compactId(compactId)
+IceInternal::CompactIdInit::CompactIdInit(const char* typeId, int compactId) noexcept : _compactId(compactId)
 {
     assert(_compactId >= 0);
     factoryTable->addTypeId(_compactId, typeId);

diff --git a/cpp/src/slice2cpp/CPlusPlusUtil.cpp b/cpp/src/slice2cpp/CPlusPlusUtil.cpp
@@ -18,6 +18,34 @@ using namespace IceInternal;
 
 namespace
 {
+    bool isTriviallyCopyable(const StructPtr& st)
+    {
+        assert(st);
+        for (const auto& m : st->dataMembers())
+        {
+            if (BuiltinPtr builtin = dynamic_pointer_cast<Builtin>(m->type()))
+            {
+                if (builtin->isVariableLength())
+                {
+                    return false;
+                }
+                // Numeric and bool are fixed-length and trivially copyable.
+            }
+            else if (StructPtr nestedSt = dynamic_pointer_cast<Struct>(m->type()))
+            {
+                if (!isTriviallyCopyable(nestedSt))
+                {
+                    return false;
+                }
+            }
+            else if (!dynamic_pointer_cast<Enum>(m->type()))
+            {
+                return false;
+            }
+        }
+        return true;
+    }
+
     string stringTypeToString(const TypePtr&, const MetadataList& metadata, TypeContext typeCtx)
     {
         string strType = findMetadata(metadata, typeCtx);
@@ -423,6 +451,12 @@ Slice::isMovable(const TypePtr& type)
             }
         }
     }
+
+    if (StructPtr st = dynamic_pointer_cast<Struct>(type))
+    {
+        return !isTriviallyCopyable(st);
+    }
+
     return !dynamic_pointer_cast<Enum>(type);
 }
 

diff --git a/cpp/test/Ice/custom/CustomBuffer.h b/cpp/test/Ice/custom/CustomBuffer.h
@@ -17,7 +17,7 @@ namespace Test
     template<typename T> class CustomBuffer
     {
     public:
-        CustomBuffer() : _buf(0), _count(0) {}
+        CustomBuffer() : _buf(nullptr), _count(0) {}
 
         CustomBuffer(const CustomBuffer& o) : _buf(0), _count(o._count)
         {
@@ -31,14 +31,14 @@ namespace Test
             }
         }
 
-        ~CustomBuffer()
+        CustomBuffer(CustomBuffer&& o) : _buf(o._buf), _count(o._count)
         {
-            if (_buf != 0)
-            {
-                delete[] _buf;
-            }
+            o._buf = nullptr;
+            o._count = 0;
         }
 
+        ~CustomBuffer() { delete[] _buf; }
+
         CustomBuffer& operator=(const CustomBuffer& o)
         {
             _count = o._count;
@@ -53,6 +53,16 @@ namespace Test
             return *this;
         }
 
+        CustomBuffer& operator=(CustomBuffer&& o)
+        {
+            delete[] _buf;
+            _buf = o._buf;
+            _count = o._count;
+            o._buf = nullptr;
+            o._count = 0;
+            return *this;
+        }
+
         size_t count() const { return _count; }
 
         T* get() const { return _buf; }

diff --git a/cpp/test/Ice/custom/MyByteSeq.cpp b/cpp/test/Ice/custom/MyByteSeq.cpp
@@ -27,10 +27,16 @@ MyByteSeq::MyByteSeq(const MyByteSeq& seq)
     }
     else
     {
-        _data = 0;
+        _data = nullptr;
     }
 }
 
+MyByteSeq::MyByteSeq(MyByteSeq&& seq) noexcept : _size(seq._size), _data(seq._data)
+{
+    seq._size = 0;
+    seq._data = nullptr;
+}
+
 MyByteSeq::~MyByteSeq() { delete[] _data; }
 
 size_t
@@ -62,7 +68,7 @@ MyByteSeq::end() const
     return _data + _size;
 }
 
-void
+MyByteSeq&
 MyByteSeq::operator=(const MyByteSeq& rhs)
 {
     delete[] _data;
@@ -74,6 +80,21 @@ MyByteSeq::operator=(const MyByteSeq& rhs)
         _data = new std::byte[_size];
         memcpy(_data, rhs._data, _size);
     }
+    return *this;
+}
+
+MyByteSeq&
+MyByteSeq::operator=(MyByteSeq&& rhs)
+{
+    delete[] _data;
+    _data = nullptr;
+
+    _size = rhs._size;
+    _data = rhs._data;
+
+    rhs._size = 0;
+    rhs._data = nullptr;
+    return *this;
 }
 
 bool

diff --git a/cpp/test/Ice/custom/MyByteSeq.h b/cpp/test/Ice/custom/MyByteSeq.h
@@ -19,13 +19,17 @@ class MyByteSeq
     MyByteSeq();
     MyByteSeq(size_t);
     MyByteSeq(const MyByteSeq&);
+    MyByteSeq(MyByteSeq&&) noexcept;
     ~MyByteSeq();
 
     size_t size() const;
     void swap(MyByteSeq&);
     const_iterator begin() const;
     const_iterator end() const;
-    void operator=(const MyByteSeq&);
+
+    MyByteSeq& operator=(const MyByteSeq&);
+    MyByteSeq& operator=(MyByteSeq&&);
+
     bool operator==(const MyByteSeq&) const;
 
 private: