Misc Sonar issues.

* declare functions "const"
* use pointer-to-const or reference-to-const
* use structured bindings
* extract assignment from expression
* mark constructors "explicit"
* pass large object parameters by reference-to-const
* use default implementation constructor
* explicitly capture required variables in lambdas

diff --git a/examples/cpp/operation-switch-host/s4u-operation-switch-host.cpp b/examples/cpp/operation-switch-host/s4u-operation-switch-host.cpp
index f8c183d..fe52596 100644 (file)
--- a/examples/cpp/operation-switch-host/s4u-operation-switch-host.cpp
+++ b/examples/cpp/operation-switch-host/s4u-operation-switch-host.cpp
@@ -28,9 +28,9 @@ int main(int argc, char* argv[])
   simgrid::plugins::Operation::init();
 
   // Retrieve hosts
-  auto tremblay = e.host_by_name("Tremblay");
-  auto jupiter  = e.host_by_name("Jupiter");
-  auto fafard   = e.host_by_name("Fafard");
+  auto* tremblay = e.host_by_name("Tremblay");
+  auto* jupiter  = e.host_by_name("Jupiter");
+  auto* fafard   = e.host_by_name("Fafard");
 
   // Create operations
   auto comm0 = simgrid::plugins::CommOp::init("comm0");
@@ -47,15 +47,15 @@ int main(int argc, char* argv[])
   exec2->add_successor(comm2);
 
   // Add a function to be called when operations end for log purpose
-  simgrid::plugins::Operation::on_end_cb([](simgrid::plugins::Operation* op) {
+  simgrid::plugins::Operation::on_end_cb([](const simgrid::plugins::Operation* op) {
     XBT_INFO("Operation %s finished (%d)", op->get_name().c_str(), op->get_count());
   });
 
   // Add a function to be called before each executions of comm0
   // This function modifies the graph of operations by adding or removing
   // successors to comm0
-  int count = 0;
-  comm0->on_this_start([&](simgrid::plugins::Operation* op) {
+  comm0->on_this_start([comm0, exec1, exec2, jupiter, fafard](const simgrid::plugins::Operation*) {
+    static int count = 0;
     if (count % 2 == 0) {
       comm0->set_destination(jupiter);
       comm0->add_successor(exec1);

diff --git a/include/simgrid/plugins/operation.hpp b/include/simgrid/plugins/operation.hpp
index 4661613..6725466 100644 (file)
--- a/include/simgrid/plugins/operation.hpp
+++ b/include/simgrid/plugins/operation.hpp
@@ -24,12 +24,9 @@ using CommOpPtr =  boost::intrusive_ptr<CommOp>;
 XBT_PUBLIC void intrusive_ptr_release(CommOp* c);
 XBT_PUBLIC void intrusive_ptr_add_ref(CommOp* c);
 
-class ExtendedAttributeActivity {
-public:
+struct ExtendedAttributeActivity {
   static simgrid::xbt::Extension<simgrid::s4u::Activity, ExtendedAttributeActivity> EXTENSION_ID;
   Operation* operation_;
-
-  ExtendedAttributeActivity(){};
 };
 
 class Operation {
@@ -53,7 +50,7 @@ protected:
   s4u::ActivityPtr current_activity_;
   std::function<void(Operation*)> end_func_;
   std::function<void(Operation*)> start_func_;
-  Operation(const std::string& name);
+  explicit Operation(const std::string& name);
   virtual ~Operation()   = default;
   virtual void execute() = 0;
 
@@ -63,16 +60,16 @@ protected:
 
 public:
   static void init();
-  const std::string& get_name() { return name_; }
-  const char* get_cname() { return name_.c_str(); }
+  const std::string& get_name() const { return name_; }
+  const char* get_cname() const { return name_.c_str(); }
   void enqueue_execs(int n);
   void set_amount(double amount);
   double get_amount() const { return amount_; }
   void add_successor(OperationPtr op);
   void remove_successor(OperationPtr op);
-  void on_this_start(std::function<void(Operation*)> func);
-  void on_this_end(std::function<void(Operation*)> func);
-  int get_count();
+  void on_this_start(const std::function<void(Operation*)>& func);
+  void on_this_end(const std::function<void(Operation*)>& func);
+  int get_count() const;
 
   /** Add a callback fired before an operation activity start.
    * Triggered after the on_this_start function**/
@@ -98,7 +95,7 @@ class ExecOp : public Operation {
 private:
   s4u::Host* host_;
 
-  ExecOp(const std::string& name);
+  explicit ExecOp(const std::string& name);
   void execute() override;
 
 public:
@@ -117,7 +114,7 @@ private:
   s4u::Host* source_;
   s4u::Host* destination_;
 
-  CommOp(const std::string& name);
+  explicit CommOp(const std::string& name);
   void execute() override;
 
 public:

diff --git a/include/xbt/config.hpp b/include/xbt/config.hpp
index 4cf17bf..372ec9b 100644 (file)
--- a/include/xbt/config.hpp
+++ b/include/xbt/config.hpp
@@ -92,10 +92,10 @@ XBT_PUBLIC void alias(const char* realname, std::initializer_list<const char*> a
  */
 template <class T>
 XBT_PUBLIC void declare_flag(const std::string& name, const std::string& description, T value,
-                             std::function<void(const T&)> callback = std::function<void(const T&)>());
+                             std::function<void(const T&)> callback = nullptr);
 template <class T>
 void declare_flag(const std::string& name, std::initializer_list<const char*> aliases, const std::string& description,
-                  T value, std::function<void(const T&)> callback = std::function<void(const T&)>())
+                  T value, std::function<void(const T&)> callback = nullptr)
 {
   declare_flag(name, description, std::move(value), std::move(callback));
   alias(name.c_str(), aliases);
@@ -265,7 +265,7 @@ public:
        const std::map<std::string, std::string, std::less<>>& valid_values)
       : value_(value), name_(name)
   {
-    simgrid::config::bind_flag(value_, name, desc, valid_values, [](std::string) {});
+    simgrid::config::bind_flag(value_, name, desc, valid_values, [](const std::string&) {});
   }
 
   /* As earlier, a constructor accepting a map of valid values -> their description,

diff --git a/src/mc/api/State.cpp b/src/mc/api/State.cpp
index 0366a31..34a41d6 100644 (file)
--- a/src/mc/api/State.cpp
+++ b/src/mc/api/State.cpp
@@ -94,7 +94,7 @@ std::size_t State::count_todo_multiples() const
 std::deque<Transition*>& State::get_recipe()
 {
   if (recipe_.empty()) {
-    for (auto* s = this; s != nullptr; s = s->get_parent_state().get())
+    for (const auto* s = this; s != nullptr; s = s->get_parent_state().get())
       if (s->get_transition_in() != nullptr)
         recipe_.push_front(s->get_transition_in().get());
   }

diff --git a/src/plugins/operation.cpp b/src/plugins/operation.cpp
index 6021786..ce34fe6 100644 (file)
--- a/src/plugins/operation.cpp
+++ b/src/plugins/operation.cpp
@@ -119,7 +119,7 @@ void Operation::init()
     return;
   Operation::inited_                      = true;
   ExtendedAttributeActivity::EXTENSION_ID = simgrid::s4u::Activity::extension_create<ExtendedAttributeActivity>();
-  simgrid::s4u::Activity::on_completion_cb([&](simgrid::s4u::Activity const& activity) {
+  simgrid::s4u::Activity::on_completion_cb([](simgrid::s4u::Activity const& activity) {
     activity.extension<ExtendedAttributeActivity>()->operation_->complete();
   });
 }
@@ -175,9 +175,9 @@ void Operation::remove_successor(OperationPtr successor)
  *  @brief Set a function to be called before each execution.
  *  @note The function is called before the underlying Activity starts.
  */
-void Operation::on_this_start(std::function<void(Operation*)> func)
+void Operation::on_this_start(const std::function<void(Operation*)>& func)
 {
-  simgrid::kernel::actor::simcall_answered([this, func] { start_func_ = func; });
+  simgrid::kernel::actor::simcall_answered([this, &func] { start_func_ = func; });
 }
 
 /** @ingroup plugin_operation
@@ -185,15 +185,15 @@ void Operation::on_this_start(std::function<void(Operation*)> func)
  *  @brief Set a function to be called after each execution.
  *  @note The function is called after the underlying Activity ends, but before sending tokens to successors.
  */
-void Operation::on_this_end(std::function<void(Operation*)> func)
+void Operation::on_this_end(const std::function<void(Operation*)>& func)
 {
-  simgrid::kernel::actor::simcall_answered([this, func] { end_func_ = func; });
+  simgrid::kernel::actor::simcall_answered([this, &func] { end_func_ = func; });
 }
 
 /** @ingroup plugin_operation
  *  @brief Return the number of completed executions.
  */
-int Operation::get_count()
+int Operation::get_count() const
 {
   return count_;
 }
@@ -208,8 +208,7 @@ ExecOp::ExecOp(const std::string& name) : Operation(name) {}
  */
 ExecOpPtr ExecOp::init(const std::string& name)
 {
-  auto op = ExecOpPtr(new ExecOp(name));
-  return op;
+  return ExecOpPtr(new ExecOp(name));
 }
 
 /** @ingroup plugin_operation
@@ -217,7 +216,7 @@ ExecOpPtr ExecOp::init(const std::string& name)
  */
 ExecOpPtr ExecOp::init(const std::string& name, double flops, s4u::Host* host)
 {
-  return ExecOpPtr(new ExecOp(name))->set_flops(flops)->set_host(host);
+  return init(name)->set_flops(flops)->set_host(host);
 }
 
 /**
@@ -273,8 +272,7 @@ CommOp::CommOp(const std::string& name) : Operation(name) {}
  */
 CommOpPtr CommOp::init(const std::string& name)
 {
-  auto op = CommOpPtr(new CommOp(name));
-  return op;
+  return CommOpPtr(new CommOp(name));
 }
 
 /** @ingroup plugin_operation
@@ -283,11 +281,7 @@ CommOpPtr CommOp::init(const std::string& name)
 CommOpPtr CommOp::init(const std::string& name, double bytes, s4u::Host* source,
                        s4u::Host* destination)
 {
-  auto op = CommOpPtr(new CommOp(name));
-  op->set_bytes(bytes);
-  op->set_source(source);
-  op->set_destination(destination);
-  return op;
+  return init(name)->set_bytes(bytes)->set_source(source)->set_destination(destination);
 }
 
 /**

diff --git a/src/xbt/utils/iter/subsets_tests.cpp b/src/xbt/utils/iter/subsets_tests.cpp
index 41bdf41..59c95ae 100644 (file)
--- a/src/xbt/utils/iter/subsets_tests.cpp
+++ b/src/xbt/utils/iter/subsets_tests.cpp
@@ -65,8 +65,8 @@ TEST_CASE("simgrid::xbt::powerset_iterator: Iteration General Properties")
       }
     }
 
-    for (const auto& iter : element_counts) {
-      REQUIRE(iter.second == expected_count);
+    for (const auto& [_, count] : element_counts) {
+      REQUIRE(count == expected_count);
     }
   }
 }
@@ -187,4 +187,4 @@ TEST_CASE("simgrid::xbt::variable_for_loop: Edge Cases")
               (outer_loop1.size() * outer_loop2.size() * outer_loop3.size() * outer_loop4.size() * outer_loop5.size()));
     }
   }
-}
\ No newline at end of file
+}

diff --git a/src/xbt/utils/iter/variable_for_loop.hpp b/src/xbt/utils/iter/variable_for_loop.hpp
index bd5d29a..cbcb0a2 100644 (file)
--- a/src/xbt/utils/iter/variable_for_loop.hpp
+++ b/src/xbt/utils/iter/variable_for_loop.hpp
@@ -85,11 +85,11 @@ template <typename IterableType> void variable_for_loop<IterableType>::increment
 
   for (auto j = k; j != std::numeric_limits<size_t>::max(); j--) {
     // Attempt to move to the next element of the `j`th collection
-    const auto& new_position = ++current_subset[j];
+    ++current_subset[j];
 
     // If the `j`th element has reached its own end, reset it
     // back to the beginning and keep moving forward
-    if (new_position == underlying_collections[j].get().cend()) {
+    if (current_subset[j] == underlying_collections[j].get().cend()) {
       current_subset[j] = underlying_collections[j].get().cbegin();
     } else {
       // Otherwise we've found the largest element which needed to