Add clang tidy and CI (#216)

.clang-tidy

@@ -0,0 +1,17 @@
+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'
+CheckOptions:
+  - { key: modernize-use-nullptr.NullMacros, value: 'NULL' }

.github/workflows/clang_tidy.yml

@@ -0,0 +1,65 @@
+name: Clang Tidy
+
+on:
+  workflow_dispatch:
+  push:
+    branches: ["main"]
+  pull_request:
+    # The branches below must be a subset of the branches above
+    branches: ["main"]
+
+jobs:
+  clang-tidy:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Install clang-tidy
+        run: |
+          # This LLVM script will add the relevant LLVM PPA: https://apt.llvm.org/
+          wget https://apt.llvm.org/llvm.sh -O /tmp/llvm.sh
+          chmod +x /tmp/llvm.sh
+          sudo /tmp/llvm.sh 18
+          sudo apt-get install -y clang-tidy-18
+          rm /tmp/llvm.sh
+          clang-tidy-18 --version
+
+      - name: Install bear
+        run: |
+          sudo apt-get update
+          sudo apt-get install -y bear
+          bear --version
+
+      - name: Checkout Ice
+        uses: actions/checkout@v4
+        with:
+          repository: zeroc-ice/ice
+          ref: main
+          path: ice
+
+      - name: Setup Ice Build Dependencies
+        uses: ./ice/.github/actions/setup-dependencies
+
+      - name: Build Ice
+        uses: ./ice/.github/actions/build
+        timeout-minutes: 90
+        with:
+          working_directory: ice/cpp
+          build_flags: srcs
+
+      - name: Set ICE_HOME
+        run: |
+          echo "ICE_HOME=${{ github.workspace }}/ice" >> $GITHUB_ENV
+
+      - name: Checkout repository
+        uses: actions/checkout@v4
+        with:
+          path: ice-demos
+
+      - name: Build C++ Demos
+        timeout-minutes: 30
+        working-directory: ice-demos/cpp
+        run: bear -- make
+
+      - name: Run Clang Tidy
+        working-directory: ice-demos/cpp
+        run: |
+          find . -name "*.h" -o -name "*.cpp" | xargs run-clang-tidy-18 -j$(nproc) -quiet

cpp/Chat/client/ChatUtils.cpp

@@ -4,12 +4,13 @@
 
 #include <Chat.h>
 #include <ChatUtils.h>
+#include <array>
 
 using namespace std;
 
-using HtmlEntity = pair<const string, const string>;
+using HtmlEntity = pair<string_view, string_view>;
 
-static const HtmlEntity htmlEntities[] = {
+constexpr std::array<HtmlEntity, 5> htmlEntities = {
     HtmlEntity("&quot;", "\""),
     HtmlEntity("&#39;", "'"),
     HtmlEntity("&lt;", "<"),

cpp/Chat/client/Client.cpp

@@ -63,7 +63,7 @@ class ChatRoomCallbackI final : public Chat::ChatRoomCallback
 };
 
 Chat::ChatSessionPrx
-createSession(Glacier2::RouterPrx router)
+createSession(const Glacier2::RouterPrx& router)
 {
     while (true)
     {
@@ -87,7 +87,7 @@ createSession(Glacier2::RouterPrx router)
                 throw runtime_error("Glaicer2::createSession return null. Is the SessionManager configured?");
             }
             router->ice_getCachedConnection()->setCloseCallback(
-                [](Ice::ConnectionPtr)
+                [](const Ice::ConnectionPtr&)
                 {
                     const lock_guard<mutex> lock(coutMutex);
                     cout << "The Glacier2 session has been destroyed." << endl;
@@ -110,7 +110,7 @@ createSession(Glacier2::RouterPrx router)
 }
 
 void
-run(shared_ptr<Ice::Communicator> communicator)
+run(const shared_ptr<Ice::Communicator>& communicator)
 {
     optional<Glacier2::RouterPrx> router{communicator->getDefaultRouter()};
     if (!router)

cpp/Chat/client/PollingClient.cpp

@@ -105,6 +105,7 @@ class GetUpdatesTask
         }
     }
 
+    [[nodiscard]]
     bool isDone() const
     {
         const lock_guard<mutex> lock(const_cast<mutex&>(_mutex));

cpp/Chat/server/ChatSessionI.cpp

@@ -29,7 +29,7 @@ class SessionCallbackAdapter : public ChatRoomCallbackAdapter, public enable_sha
         auto self = shared_from_this();
         try
         {
-            _callback->initAsync(users, nullptr, [self](exception_ptr) { self->failed(); });
+            _callback->initAsync(users, nullptr, [self](const exception_ptr&) { self->failed(); });
         }
         catch (const Ice::CommunicatorDestroyedException&)
         {
@@ -42,7 +42,7 @@ class SessionCallbackAdapter : public ChatRoomCallbackAdapter, public enable_sha
         auto self = shared_from_this();
         try
         {
-            _callback->joinAsync(e->timestamp, e->name, nullptr, [self](exception_ptr) { self->failed(); });
+            _callback->joinAsync(e->timestamp, e->name, nullptr, [self](const exception_ptr&) { self->failed(); });
         }
         catch (const Ice::CommunicatorDestroyedException&)
         {
@@ -55,7 +55,7 @@ class SessionCallbackAdapter : public ChatRoomCallbackAdapter, public enable_sha
         auto self = shared_from_this();
         try
         {
-            _callback->leaveAsync(e->timestamp, e->name, nullptr, [self](exception_ptr) { self->failed(); });
+            _callback->leaveAsync(e->timestamp, e->name, nullptr, [self](const exception_ptr&) { self->failed(); });
         }
         catch (const Ice::CommunicatorDestroyedException&)
         {
@@ -68,7 +68,12 @@ class SessionCallbackAdapter : public ChatRoomCallbackAdapter, public enable_sha
         auto self = shared_from_this();
         try
         {
-            _callback->sendAsync(e->timestamp, e->name, e->message, nullptr, [self](exception_ptr) { self->failed(); });
+            _callback->sendAsync(
+                e->timestamp,
+                e->name,
+                e->message,
+                nullptr,
+                [self](const exception_ptr&) { self->failed(); });
         }
         catch (const Ice::CommunicatorDestroyedException&)
         {

cpp/Chat/server/ChatUtils.cpp

@@ -12,9 +12,8 @@ using namespace std;
 
 static const unsigned int maxNameSize = 12;
 static const unsigned int minNameSize = 3;
-static const string nameRange = "abcdefghijklmnopqrstuvwxyz1234567890ABCDEFGHIJKLMNOPQRSTUVWXYZ";
-static const string isEmptyTokens = "\t\r\n\f\v ";
-
+constexpr string_view nameRange = "abcdefghijklmnopqrstuvwxyz1234567890ABCDEFGHIJKLMNOPQRSTUVWXYZ";
+constexpr string_view isEmptyTokens = "\t\r\n\f\v ";
 static const unsigned int maxMessageSize = 1024;
 
 string

cpp/Chat/server/PollingChatSessionI.cpp

@@ -7,28 +7,28 @@
 
 using namespace std;
 
-class PollCallbackAdapter : public ChatRoomCallbackAdapter
+class PollCallbackAdapter final : public ChatRoomCallbackAdapter
 {
 public:
-    virtual void init(Ice::StringSeq users) override
+    void init(Ice::StringSeq users) final
     {
         const lock_guard<mutex> sync(_mutex);
         _users = std::move(users);
     }
 
-    virtual void send(const shared_ptr<PollingChat::MessageEvent>& e) override
+    void send(const shared_ptr<PollingChat::MessageEvent>& e) final
     {
         const lock_guard<mutex> sync(_mutex);
         _updates.push_back(e);
     }
 
-    virtual void join(const shared_ptr<PollingChat::UserJoinedEvent>& e) override
+    void join(const shared_ptr<PollingChat::UserJoinedEvent>& e) final
     {
         const lock_guard<mutex> sync(_mutex);
         _updates.push_back(e);
     }
 
-    virtual void leave(const shared_ptr<PollingChat::UserLeftEvent>& e) override
+    void leave(const shared_ptr<PollingChat::UserLeftEvent>& e) final
     {
         const lock_guard<mutex> sync(_mutex);
         _updates.push_back(e);
@@ -129,7 +129,7 @@ PollingChatSessionI::send(string message, const Ice::Current&)
         }
         throw Chat::InvalidMessageException(ex.what());
     }
-    return _chatRoom->send(_name, std::move(msg));
+    return _chatRoom->send(_name, msg);
 }
 
 void

cpp/Glacier2/callback/Client.cpp

@@ -104,7 +104,7 @@ run(const shared_ptr<Ice::Communicator>& communicator)
 
     const Ice::ConnectionPtr connection = router->ice_getCachedConnection();
     assert(connection);
-    connection->setCloseCallback([](Ice::ConnectionPtr)
+    connection->setCloseCallback([](const Ice::ConnectionPtr&)
                                  { cout << "The Glacier2 session has been destroyed." << endl; });
 
     //

cpp/Glacier2/simpleChat/ChatSessionI.cpp

@@ -46,14 +46,14 @@ void
 ChatRoom::enter(ChatSessionPrx session, ChatCallbackPrx callback, const Ice::Current& current)
 {
     const lock_guard<mutex> sync(_mutex);
-    _callbacks.push_back(callback);
+    _callbacks.emplace_back(std::move(callback));
 
     auto p = _connectionMap.find(current.con);
     if (p == _connectionMap.end())
     {
         cout << "enter: create new entry in connection map" << endl;
 
-        _connectionMap[current.con].push_back(session);
+        _connectionMap[current.con].emplace_back(std::move(session));
 
         current.con->setCloseCallback(
             [this](const shared_ptr<Ice::Connection>& con)
@@ -68,7 +68,7 @@ ChatRoom::enter(ChatSessionPrx session, ChatCallbackPrx callback, const Ice::Cur
     else
     {
         cout << "enter: add session to existing connection map entry" << endl;
-        p->second.push_back(session);
+        p->second.emplace_back(std::move(session));
     }
 }
 

cpp/Glacier2/simpleChat/Client.cpp

@@ -13,10 +13,10 @@ using namespace Demo;
 // mutex to prevent intertwined cout output
 mutex coutMutex;
 
-class ChatCallbackI : public ChatCallback
+class ChatCallbackI final : public ChatCallback
 {
 public:
-    virtual void message(string data, const Ice::Current&) override
+    void message(string data, const Ice::Current&) final
     {
         const lock_guard<mutex> lock(coutMutex);
         cout << data << endl;
@@ -102,7 +102,7 @@ run(const shared_ptr<Ice::Communicator>& communicator)
     const Ice::ConnectionPtr connection = router->ice_getCachedConnection();
     assert(connection);
     connection->setCloseCallback(
-        [](Ice::ConnectionPtr)
+        [](const Ice::ConnectionPtr&)
         {
             const lock_guard<mutex> lock(coutMutex);
             cout << "The Glacier2 session has been destroyed." << endl;

cpp/Glacier2/simpleChat/Server.cpp

@@ -10,10 +10,10 @@
 using namespace std;
 using namespace Demo;
 
-class DummyPermissionsVerifierI : public Glacier2::PermissionsVerifier
+class DummyPermissionsVerifierI final : public Glacier2::PermissionsVerifier
 {
 public:
-    virtual bool checkPermissions(string userId, string password, string&, const Ice::Current&) const override
+    bool checkPermissions(string userId, string password, string&, const Ice::Current&) const final
     {
         cout << "verified user `" << userId << "' with password `" << password << "'" << endl;
         return true;

cpp/Ice/async/Client.cpp

@@ -75,7 +75,7 @@ run(const shared_ptr<Ice::Communicator>& communicator)
                 hello->sayHelloAsync(
                     5000,
                     nullptr,
-                    [](exception_ptr e)
+                    [](const exception_ptr& e)
                     {
                         try
                         {

cpp/Ice/asyncInvocation/Client.cpp

@@ -125,7 +125,7 @@ run(const shared_ptr<Ice::Communicator>& communicator, const string& appName)
                 assert(false);
                 p.set_value(make_tuple(answer, remainder));
             },
-            [&p](exception_ptr ex) { p.set_exception(ex); });
+            [&p](const exception_ptr& ex) { p.set_exception(ex); });
         try
         {
             auto result = p.get_future().get();

cpp/Ice/bidir/CallbackI.cpp

@@ -14,7 +14,7 @@ CallbackSenderI::addClient(optional<CallbackReceiverPrx> client, const Ice::Curr
 {
     const lock_guard<mutex> lock(_mutex);
     cout << "adding client `" << Ice::identityToString(client->ice_getIdentity()) << "'" << endl;
-    _clients.push_back(client->ice_fixed(current.con));
+    _clients.emplace_back(client->ice_fixed(current.con));
 }
 
 void
@@ -74,14 +74,14 @@ CallbackSenderI::invokeCallback()
             for (const auto& p : _clients)
             {
                 auto self = shared_from_this();
-                p->callbackAsync(num, nullptr, [self, p](exception_ptr eptr) { self->removeClient(p, eptr); });
+                p->callbackAsync(num, nullptr, [self, p](const exception_ptr& eptr) { self->removeClient(p, eptr); });
             }
         }
     }
 }
 
 void
-CallbackSenderI::removeClient(const optional<CallbackReceiverPrx>& client, exception_ptr eptr)
+CallbackSenderI::removeClient(const optional<CallbackReceiverPrx>& client, const exception_ptr& eptr)
 {
     const lock_guard<mutex> lock(_mutex);
     auto p = find(_clients.begin(), _clients.end(), client);

cpp/Ice/bidir/CallbackI.h

@@ -17,7 +17,7 @@ class CallbackSenderI final : public Demo::CallbackSender, public std::enable_sh
 
 private:
     void invokeCallback();
-    void removeClient(const std::optional<Demo::CallbackReceiverPrx>&, std::exception_ptr);
+    void removeClient(const std::optional<Demo::CallbackReceiverPrx>&, const std::exception_ptr&);
 
     bool _destroy = false;
     std::vector<std::optional<Demo::CallbackReceiverPrx>> _clients;

cpp/Ice/interleaved/ThroughputI.cpp

@@ -8,7 +8,7 @@
 Demo::Throughput::EchoByteSeqMarshaledResult
 ThroughputI::echoByteSeq(std::pair<const std::byte*, const std::byte*> seq, const Ice::Current& current)
 {
-    return EchoByteSeqMarshaledResult(seq, current);
+    return {seq, current};
 }
 
 void

cpp/Ice/invoke/Client.cpp

@@ -194,13 +194,13 @@ run(const shared_ptr<Ice::Communicator>& communicator)
                 Ice::OutputStream out(communicator);
                 out.startEncapsulation();
                 Demo::StructureSeq arr;
-                arr.push_back(Demo::Structure());
+                arr.emplace_back();
                 arr.back().name = "red";
                 arr.back().value = Color::red;
-                arr.push_back(Demo::Structure());
+                arr.emplace_back();
                 arr.back().name = "green";
                 arr.back().value = Color::green;
-                arr.push_back(Demo::Structure());
+                arr.emplace_back();
                 arr.back().name = "blue";
                 arr.back().value = Color::blue;
                 out.write(arr);

cpp/Ice/locator/Locator.cpp

@@ -95,7 +95,11 @@ namespace
             response(_registry->getAdapter(id));
         }
 
-        optional<Ice::LocatorRegistryPrx> getRegistry(const Ice::Current&) const final { return _registryPrx; }
+        [[nodiscard]]
+        optional<Ice::LocatorRegistryPrx> getRegistry(const Ice::Current&) const final
+        {
+            return _registryPrx;
+        }
 
     private:
         const shared_ptr<LocatorRegistryI> _registry;