[issue1129] Enable more performance-related clang-tidy checks and fix…

… code accordingly.

misc/style/run-clang-tidy.py

@@ -14,9 +14,11 @@
 import utils
 
 
+LIGHTWEIGHT_TYPES = ["StateID"]
 IGNORES = [
     "'cplex.h' file not found [clang-diagnostic-error]",
     "'soplex.h' file not found [clang-diagnostic-error]",
+    "local copy 'copied_key' of the variable 'key' is never modified; consider avoiding the copy [performance-unnecessary-copy-initialization]",
 ]
 
 
@@ -47,15 +49,29 @@ def check_search_code_with_clang_tidy():
     # categories instead of deleting them to see which additional checks
     # we could activate.
     checks = [
-        # Enable with CheckTriviallyCopyableMove=0 when we require
-        # clang-tidy >= 6.0 (see issue856).
-        # "misc-move-const-arg",
-        "misc-move-constructor-init",
-        "misc-use-after-move",
+        "bugprone-use-after-move",
 
+        # "performance-avoid-endl",
+        # "performance-enum-size",
+        "performance-faster-string-find",
         "performance-for-range-copy",
-        "performance-implicit-cast-in-loop",
+        "performance-implicit-conversion-in-loop",
+        "performance-inefficient-algorithm",
+        # "performance-inefficient-string-concatenation",
         "performance-inefficient-vector-operation",
+        # Enable with CheckTriviallyCopyableMove=0 when we require
+        # clang-tidy >= 6.0 (see issue856).
+        # "performance-move-const-arg",
+        #"performance-move-constructor-init",
+        "performance-no-automatic-move",
+        # "performance-no-int-to-ptr",
+        # "performance-noexcept-destructor",
+        # "performance-noexcept-move-constructor",
+        # "performance-noexcept-swap",
+        "performance-trivially-destructible",
+        "performance-type-promotion-in-math-fn",
+        "performance-unnecessary-copy-initialization",
+        "performance-unnecessary-value-param",
 
         "readability-avoid-const-params-in-decls",
         # "readability-braces-around-statements",
@@ -83,12 +99,17 @@ def check_search_code_with_clang_tidy():
         "readability-static-definition-in-anonymous-namespace",
         "readability-uniqueptr-delete-release",
         ]
+    config = f"""{{CheckOptions: [\
+        {{key: performance-unnecessary-value-param.AllowedTypes, value: "{';'.join(LIGHTWEIGHT_TYPES)}"}},\
+    ]}}""".replace("    ", "")
     cmd = [
         "run-clang-tidy-12",
         "-quiet",
         "-p", build_dir,
         "-clang-tidy-binary=clang-tidy-12",
-        "-checks=-*," + ",".join(checks)]
+        "-checks=-*," + ",".join(checks),
+        f"-config={config}",
+    ]
     print("Running clang-tidy: " + " ".join(pipes.quote(x) for x in cmd))
     print()
     # Don't check returncode here because clang-tidy exits with 1 if it finds any issues.

src/search/cartesian_abstractions/cost_saturation.cc

@@ -190,7 +190,7 @@ bool CostSaturation::state_is_dead_end(const State &state) const {
 void CostSaturation::build_abstractions(
     const vector<shared_ptr<AbstractTask>> &subtasks,
     const utils::CountdownTimer &timer,
-    function<bool()> should_abort) {
+    const function<bool()> &should_abort) {
     int rem_subtasks = subtasks.size();
     for (shared_ptr<AbstractTask> subtask : subtasks) {
         subtask = get_remaining_costs_task(subtask);

src/search/cartesian_abstractions/cost_saturation.h

@@ -49,7 +49,7 @@ class CostSaturation {
     void build_abstractions(
         const std::vector<std::shared_ptr<AbstractTask>> &subtasks,
         const utils::CountdownTimer &timer,
-        std::function<bool()> should_abort);
+        const std::function<bool()> &should_abort);
     void print_statistics(utils::Duration init_time) const;
 
 public:

src/search/heuristics/lm_cut_landmarks.cc

@@ -274,8 +274,8 @@ void LandmarkCutLandmarks::validate_h_max() const {
 }
 
 bool LandmarkCutLandmarks::compute_landmarks(
-    const State &state, CostCallback cost_callback,
-    LandmarkCallback landmark_callback) {
+    const State &state, const CostCallback &cost_callback,
+    const LandmarkCallback &landmark_callback) {
     for (RelaxedOperator &op : relaxed_operators) {
         op.cost = op.base_cost;
     }

src/search/heuristics/lm_cut_landmarks.h

@@ -103,8 +103,8 @@ class LandmarkCutLandmarks {
 
       Returns true iff state is detected as a dead end.
     */
-    bool compute_landmarks(const State &state, CostCallback cost_callback,
-                           LandmarkCallback landmark_callback);
+    bool compute_landmarks(const State &state, const CostCallback &cost_callback,
+                           const LandmarkCallback &landmark_callback);
 };
 
 inline void RelaxedOperator::update_h_max_supporter() {

src/search/lp/lp_solver.cc

@@ -107,7 +107,7 @@ const string &LinearProgram::get_objective_name() const {
     return objective_name;
 }
 
-void LinearProgram::set_objective_name(string name) {
+void LinearProgram::set_objective_name(const string &name) {
     objective_name = name;
 }
 

src/search/lp/lp_solver.h

@@ -91,7 +91,7 @@ class LinearProgram {
     const named_vector::NamedVector<LPConstraint> &get_constraints() const;
     double get_infinity() const;
     LPObjectiveSense get_sense() const;
-    void set_objective_name(std::string name);
+    void set_objective_name(const std::string &name);
     const std::string &get_objective_name() const;
 };
 

src/search/merge_and_shrink/merge_and_shrink_algorithm.cc

@@ -33,7 +33,7 @@ using plugins::Bounds;
 using utils::ExitCode;
 
 namespace merge_and_shrink {
-static void log_progress(const utils::Timer &timer, string msg, utils::LogProxy &log) {
+static void log_progress(const utils::Timer &timer, const string &msg, utils::LogProxy &log) {
     log << "M&S algorithm timer: " << timer << " (" << msg << ")" << endl;
 }
 

src/search/parser/abstract_syntax_tree.cc

@@ -403,7 +403,7 @@ const plugins::Type &ListNode::get_type(DecorateContext &context) const {
     }
 }
 
-LiteralNode::LiteralNode(Token value)
+LiteralNode::LiteralNode(const Token &value)
     : value(value) {
 }
 

src/search/parser/abstract_syntax_tree.h

@@ -89,7 +89,7 @@ class ListNode : public ASTNode {
 class LiteralNode : public ASTNode {
     Token value;
 public:
-    explicit LiteralNode(Token value);
+    explicit LiteralNode(const Token &value);
     DecoratedASTNodePtr decorate(DecorateContext &context) const override;
     void dump(std::string indent) const override;
     const plugins::Type &get_type(DecorateContext &context) const override;

src/search/parser/decorated_abstract_syntax_tree.cc

@@ -84,7 +84,7 @@ const DecoratedASTNode &FunctionArgument::get_value() const {
     return *value;
 }
 
-void FunctionArgument::dump(string indent) const {
+void FunctionArgument::dump(const string &indent) const {
     cout << indent << key << " = " << endl;
     value->dump("| " + indent);
 }
@@ -128,7 +128,7 @@ void DecoratedLetNode::dump(string indent) const {
 }
 
 DecoratedFunctionCallNode::DecoratedFunctionCallNode(
-    shared_ptr<const plugins::Feature> feature, vector<FunctionArgument> &&arguments,
+    const shared_ptr<const plugins::Feature> &feature, vector<FunctionArgument> &&arguments,
     const string &unparsed_config)
     : feature(feature), arguments(move(arguments)), unparsed_config(unparsed_config) {
 }

src/search/parser/decorated_abstract_syntax_tree.h

@@ -67,7 +67,7 @@ class FunctionArgument {
 
     std::string get_key() const;
     const DecoratedASTNode &get_value() const;
-    void dump(std::string indent) const;
+    void dump(const std::string &indent) const;
 
     // TODO: This is here only for the iterated search. Once we switch to builders, we won't need it any more.
     bool is_lazily_constructed() const;
@@ -99,7 +99,7 @@ class DecoratedFunctionCallNode : public DecoratedASTNode {
     std::string unparsed_config;
 public:
     DecoratedFunctionCallNode(
-        std::shared_ptr<const plugins::Feature> feature,
+        const std::shared_ptr<const plugins::Feature> &feature,
         std::vector<FunctionArgument> &&arguments,
         const std::string &unparsed_config);
 

src/search/task_utils/sampling.cc

@@ -19,7 +19,7 @@ static State sample_state_with_random_walk(
     int init_h,
     double average_operator_cost,
     utils::RandomNumberGenerator &rng,
-    function<bool(State)> is_dead_end) {
+    const function<bool(State)> &is_dead_end) {
     assert(init_h != numeric_limits<int>::max());
     int n;
     if (init_h == 0) {

src/search/task_utils/variable_order_finder.cc

@@ -17,7 +17,7 @@ using utils::ExitCode;
 namespace variable_order_finder {
 VariableOrderFinder::VariableOrderFinder(const TaskProxy &task_proxy,
                                          VariableOrderType variable_order_type,
-                                         shared_ptr<utils::RandomNumberGenerator> rng)
+                                         const shared_ptr<utils::RandomNumberGenerator> &rng)
     : task_proxy(task_proxy),
       variable_order_type(variable_order_type) {
     int var_count = task_proxy.get_variables().size();

src/search/task_utils/variable_order_finder.h

@@ -42,7 +42,7 @@ class VariableOrderFinder {
     VariableOrderFinder(
         const TaskProxy &task_proxy,
         VariableOrderType variable_order_type,
-        std::shared_ptr<utils::RandomNumberGenerator> rng = nullptr);
+        const std::shared_ptr<utils::RandomNumberGenerator> &rng = nullptr);
     ~VariableOrderFinder() = default;
     bool done() const;
     int next();