Fix performance-unnecessary-value-param

Summary: As title. Reviewed By: wsanville Differential Revision: D50897105 fbshipit-source-id: 8097fb04a20bb274e4816545c4dcb52b76445f7c
facebook · Nov 2, 2023 · 663d075 · 663d075
1 parent 3f51958
commit 663d075
Show file tree

Hide file tree

Showing 6 changed files with 22 additions and 17 deletions.
diff --git a/libredex/Configurable.cpp b/libredex/Configurable.cpp
@@ -564,6 +564,6 @@ IMPLEMENT_REFLECTOR_EX(TypeMap, "dict")
 
 IMPLEMENT_TRAIT_REFLECTOR(bool)
 IMPLEMENT_TRAIT_REFLECTOR(int)
-IMPLEMENT_TRAIT_REFLECTOR(std::string)
+IMPLEMENT_TRAIT_REFLECTOR(const std::string&)
 
 #undef error_or_warn
diff --git a/libredex/DexHasher.cpp b/libredex/DexHasher.cpp
@@ -51,13 +51,13 @@ class Impl final {
   void hash(const IRCode* c);
   void hash(const cfg::ControlFlowGraph& cfg);
   void hash_code_init(
-      IRList::const_iterator begin,
-      IRList::const_iterator end,
+      const IRList::const_iterator& begin,
+      const IRList::const_iterator& end,
       std::unordered_map<const MethodItemEntry*, uint32_t>* mie_ids,
       std::unordered_map<DexPosition*, uint32_t>* pos_ids);
   void hash_code_flush(
-      IRList::const_iterator begin,
-      IRList::const_iterator end,
+      const IRList::const_iterator& begin,
+      const IRList::const_iterator& end,
       const std::unordered_map<const MethodItemEntry*, uint32_t>& mie_ids,
       const std::unordered_map<DexPosition*, uint32_t>& pos_ids);
   void hash(const IRInstruction* insn);
@@ -263,8 +263,8 @@ void Impl::hash(const cfg::ControlFlowGraph& cfg) {
 }
 
 void Impl::hash_code_init(
-    IRList::const_iterator begin,
-    IRList::const_iterator end,
+    const IRList::const_iterator& begin,
+    const IRList::const_iterator& end,
     std::unordered_map<const MethodItemEntry*, uint32_t>* mie_ids,
     std::unordered_map<DexPosition*, uint32_t>* pos_ids) {
   auto get_mie_id = [mie_ids](const MethodItemEntry* mie) {
@@ -347,8 +347,8 @@ void Impl::hash_code_init(
 }
 
 void Impl::hash_code_flush(
-    IRList::const_iterator begin,
-    IRList::const_iterator end,
+    const IRList::const_iterator& begin,
+    const IRList::const_iterator& end,
     const std::unordered_map<const MethodItemEntry*, uint32_t>& mie_ids,
     const std::unordered_map<DexPosition*, uint32_t>& pos_ids) {
   uint32_t mie_index = 0;

diff --git a/libredex/IRAssembler.cpp b/libredex/IRAssembler.cpp
@@ -534,7 +534,8 @@ std::unordered_map<std::string, MethodItemEntry*> get_catch_name_map(
 }
 
 // Can we merge this target into the same label as the previous target?
-bool can_merge(IRList::const_iterator prev, IRList::const_iterator it) {
+bool can_merge(const IRList::const_iterator& prev,
+               const IRList::const_iterator& it) {
   always_assert(it->type == MFLOW_TARGET);
   return prev->type == MFLOW_TARGET &&
          // can't merge if/goto targets with switch targets

diff --git a/libredex/IRList.h b/libredex/IRList.h
@@ -551,16 +551,16 @@ class IRList {
 
   // transfer all of `other` into `this` starting at `pos`
   // memory ownership is also transferred
-  void splice(IRList::const_iterator pos, IRList& other) {
+  void splice(const IRList::const_iterator& pos, IRList& other) {
     m_list.splice(pos, other.m_list);
   }
 
   // transfer `other[begin]` to `other[end]` into `this` starting at `pos`
   // memory ownership is also transferred
-  void splice_selection(IRList::const_iterator pos,
+  void splice_selection(const IRList::const_iterator& pos,
                         IRList& other,
-                        IRList::const_iterator begin,
-                        IRList::const_iterator end) {
+                        const IRList::const_iterator& begin,
+                        const IRList::const_iterator& end) {
     m_list.splice(pos, other.m_list, begin, end);
   }
 

diff --git a/service/escape-analysis/BlamingAnalysis.h b/service/escape-analysis/BlamingAnalysis.h
@@ -222,7 +222,9 @@ class BlameMap {
     const BlameStore::Value m_value;
   };
 
-  explicit BlameMap(BlameStore::Domain domain) : m_domain(std::move(domain)) {}
+  // TODO: Tidy complains about an unnecessary copy when using a value type.
+  //       This indicates that a move constructor may be missing for Domain.
+  explicit BlameMap(const BlameStore::Domain& domain) : m_domain(domain) {}
 
   size_t size() const { return m_domain.size(); }
 

diff --git a/service/escape-analysis/LocalPointersAnalysis.h b/service/escape-analysis/LocalPointersAnalysis.h
@@ -200,8 +200,10 @@ struct EscapeSummary {
 
   EscapeSummary() = default;
 
-  EscapeSummary(ParamSet ps, std::initializer_list<uint16_t> l)
-      : escaping_parameters(l), returned_parameters(std::move(ps)) {}
+  // TODO: Tidy complains about an unnecessary copy when using a value type.
+  //       This indicates that a move constructor may be missing for ParamSet.
+  EscapeSummary(const ParamSet& ps, std::initializer_list<uint16_t> l)
+      : escaping_parameters(l), returned_parameters(ps) {}
 
   static EscapeSummary from_s_expr(const sparta::s_expr&);