Enable google-explicit-constructor/hicpp-explicit-conversions

Summary: Enable clang-tidy checks for explicit single-parameter constructors. Reviewed By: int3 Differential Revision: D20230350 fbshipit-source-id: ccabfa02b6b440c166df39db6d7740b6efc196d6
kami-lang · Mar 9, 2020 · 9a5ed63 · 9a5ed63
1 parent 250b7ad
commit 9a5ed63
Show file tree

Hide file tree

Showing 69 changed files with 219 additions and 184 deletions.
diff --git a/.clang-tidy b/.clang-tidy
@@ -1,4 +1,4 @@
-Checks: '-*,modernize-make-shared,performance-*,-performance-inefficient-string-concatenation,readability-const-return-type,readability-container-size-empty,misc-definitions-in-headers,modernize-use-override'
+Checks: '-*,modernize-make-shared,performance-*,-performance-inefficient-string-concatenation,readability-const-return-type,readability-container-size-empty,misc-definitions-in-headers,modernize-use-override,google-explicit-constructor,hicpp-explicit-conversions'
 WarningsAsErrors: '*'
 CheckOptions:
   - key: modernize-use-override.IgnoreDestructors

diff --git a/libredex/ApkManager.h b/libredex/ApkManager.h
@@ -13,7 +13,7 @@
 
 class ApkManager {
  public:
-  ApkManager(std::string&& apk_dir) : m_apk_dir(apk_dir) {}
+  explicit ApkManager(std::string&& apk_dir) : m_apk_dir(apk_dir) {}
 
   virtual ~ApkManager() {
     for (auto& fd : m_files) {

diff --git a/libredex/CFGMutation.h b/libredex/CFGMutation.h
@@ -24,7 +24,7 @@ namespace cfg {
 class CFGMutation {
  public:
   /// Create a new mutation to apply to \p cfg.
-  CFGMutation(ControlFlowGraph& cfg);
+  explicit CFGMutation(ControlFlowGraph& cfg);
 
   /// CFGMutation is not copyable
   CFGMutation(const CFGMutation&) = delete;

diff --git a/libredex/CallGraph.cpp b/libredex/CallGraph.cpp
@@ -18,7 +18,7 @@ using namespace call_graph;
 
 class SingleCalleeStrategy final : public BuildStrategy {
  public:
-  SingleCalleeStrategy(const Scope& scope) : m_scope(scope) {
+  explicit SingleCalleeStrategy(const Scope& scope) : m_scope(scope) {
     auto non_virtual_vec = mog::get_non_true_virtuals(scope);
     m_non_virtual.insert(non_virtual_vec.begin(), non_virtual_vec.end());
   }

diff --git a/libredex/CallGraph.h b/libredex/CallGraph.h
@@ -112,7 +112,7 @@ class Edge {
 
 class Graph final {
  public:
-  Graph(const BuildStrategy&);
+  explicit Graph(const BuildStrategy&);
 
   NodeId entry() const { return m_entry; }
   NodeId exit() const { return m_exit; }

diff --git a/libredex/ConcurrentContainers.h b/libredex/ConcurrentContainers.h
@@ -380,10 +380,13 @@ class ConcurrentContainerIterator final {
   using base_iterator = std::conditional_t<std::is_const<Container>::value,
                                            typename Container::const_iterator,
                                            typename Container::iterator>;
+  using const_base_iterator = typename Container::const_iterator;
   using difference_type = std::ptrdiff_t;
   using value_type = typename base_iterator::value_type;
   using pointer = typename base_iterator::pointer;
+  using const_pointer = typename Container::const_iterator::pointer;
   using reference = typename base_iterator::reference;
+  using const_reference = typename Container::const_iterator::reference;
   using iterator_category = std::forward_iterator_tag;
 
   explicit ConcurrentContainerIterator(Container* slots)
@@ -432,12 +435,12 @@ class ConcurrentContainerIterator final {
     return m_position.operator->();
   }
 
-  const reference operator*() const {
+  const_reference operator*() const {
     always_assert(m_position != m_slots[n_slots - 1].end());
     return *m_position;
   }
 
-  const pointer operator->() const {
+  const_pointer operator->() const {
     always_assert(m_position != m_slots[n_slots - 1].end());
     return m_position.operator->();
   }

diff --git a/libredex/ConfigFiles.h b/libredex/ConfigFiles.h
@@ -29,7 +29,7 @@ using MethodMap = std::map<MethodTuple, DexClass*>;
  * ConfigFiles should be a readonly structure
  */
 struct ConfigFiles {
-  ConfigFiles(const Json::Value& config);
+  explicit ConfigFiles(const Json::Value& config);
   ConfigFiles(const Json::Value& config, const std::string& outdir);
 
   const std::vector<std::string>& get_coldstart_classes() {

diff --git a/libredex/Creators.h b/libredex/Creators.h
@@ -384,7 +384,7 @@ struct MethodBlock {
  */
 struct MethodCreator {
  public:
-  MethodCreator(DexMethod* meth);
+  explicit MethodCreator(DexMethod* meth);
   MethodCreator(DexMethodRef* ref,
                 DexAccessFlags access,
                 DexAnnotationSet* anno = nullptr,

diff --git a/libredex/DexAnnotation.h b/libredex/DexAnnotation.h
@@ -60,7 +60,7 @@ class DexEncodedValue : public Gatherable {
   DexEncodedValueTypes m_evtype;
   uint64_t m_value;
 
-  DexEncodedValue(DexEncodedValueTypes type, uint64_t value = 0)
+  explicit DexEncodedValue(DexEncodedValueTypes type, uint64_t value = 0)
       : Gatherable() {
     m_evtype = type;
     m_value = value;
@@ -110,7 +110,8 @@ class DexEncodedValueString : public DexEncodedValue {
   DexString* m_string;
 
  public:
-  DexEncodedValueString(DexString* string) : DexEncodedValue(DEVT_STRING) {
+  explicit DexEncodedValueString(DexString* string)
+      : DexEncodedValue(DEVT_STRING) {
     m_string = string;
   }
 
@@ -138,7 +139,7 @@ class DexEncodedValueType : public DexEncodedValue {
   DexType* m_type;
 
  public:
-  DexEncodedValueType(DexType* type) : DexEncodedValue(DEVT_TYPE) {
+  explicit DexEncodedValueType(DexType* type) : DexEncodedValue(DEVT_TYPE) {
     m_type = type;
   }
 
@@ -196,7 +197,8 @@ class DexEncodedValueMethod : public DexEncodedValue {
   DexMethodRef* m_method;
 
  public:
-  DexEncodedValueMethod(DexMethodRef* method) : DexEncodedValue(DEVT_METHOD) {
+  explicit DexEncodedValueMethod(DexMethodRef* method)
+      : DexEncodedValue(DEVT_METHOD) {
     m_method = method;
   }
 
@@ -303,8 +305,8 @@ class DexEncodedValueArray : public DexEncodedValue {
    * Static values are encoded without a DEVT_ARRAY header byte
    * so we differentiate that here.
    */
-  DexEncodedValueArray(std::deque<DexEncodedValue*>* evalues,
-                       bool static_val = false)
+  explicit DexEncodedValueArray(std::deque<DexEncodedValue*>* evalues,
+                                bool static_val = false)
       : DexEncodedValue(DEVT_ARRAY), m_evalues(evalues) {
     m_static_val = static_val;
   }

diff --git a/libredex/DexClass.h b/libredex/DexClass.h
@@ -184,7 +184,7 @@ class DexType {
   DexString* m_name;
 
   // See UNIQUENESS above for the rationale for the private constructor pattern.
-  DexType(DexString* dstring) { m_name = dstring; }
+  explicit DexType(DexString* dstring) { m_name = dstring; }
 
  public:
   // DexType retrieval/creation
@@ -474,7 +474,7 @@ class DexTypeList {
   std::deque<DexType*> m_list;
 
   // See UNIQUENESS above for the rationale for the private constructor pattern.
-  DexTypeList(std::deque<DexType*>&& p) { m_list = std::move(p); }
+  explicit DexTypeList(std::deque<DexType*>&& p) { m_list = std::move(p); }
 
  public:
   std::deque<DexType*>::iterator begin() { return m_list.begin(); }
@@ -1118,7 +1118,7 @@ class DexClass {
   std::vector<DexMethod*> m_dmethods;
   std::vector<DexMethod*> m_vmethods;
 
-  DexClass(const std::string& location) : m_location(location){};
+  explicit DexClass(const std::string& location) : m_location(location){};
   void load_class_annotations(DexIdx* idx, uint32_t anno_off);
   void load_class_data_item(DexIdx* idx,
                             uint32_t cdi_off,

diff --git a/libredex/DexDebugInstruction.h b/libredex/DexDebugInstruction.h
@@ -30,7 +30,7 @@ class DexDebugInstruction : public Gatherable {
   DexDebugItemOpcode m_opcode;
 
  public:
-  DexDebugInstruction(DexDebugItemOpcode op, uint32_t v = DEX_NO_INDEX)
+  explicit DexDebugInstruction(DexDebugItemOpcode op, uint32_t v = DEX_NO_INDEX)
       : Gatherable() {
     m_opcode = op;
     m_uvalue = v;
@@ -77,7 +77,8 @@ class DexDebugOpcodeSetFile : public DexDebugInstruction {
   DexString* m_str;
 
  public:
-  DexDebugOpcodeSetFile(DexString* str) : DexDebugInstruction(DBG_SET_FILE) {
+  explicit DexDebugOpcodeSetFile(DexString* str)
+      : DexDebugInstruction(DBG_SET_FILE) {
     m_str = str;
   }
 

diff --git a/libredex/DexHasher.h b/libredex/DexHasher.h
@@ -37,7 +37,7 @@ struct DexHash {
 
 class DexScopeHasher final {
  public:
-  DexScopeHasher(const Scope& scope) : m_scope(scope) {}
+  explicit DexScopeHasher(const Scope& scope) : m_scope(scope) {}
   DexHash run();
 
  private:
@@ -46,7 +46,7 @@ class DexScopeHasher final {
 
 class DexClassHasher final {
  public:
-  DexClassHasher(DexClass* cls) : m_cls(cls) {}
+  explicit DexClassHasher(DexClass* cls) : m_cls(cls) {}
   DexHash run();
 
  private:

diff --git a/libredex/DexInstruction.h b/libredex/DexInstruction.h
@@ -65,7 +65,7 @@ class DexInstruction : public Gatherable {
   }
 
  public:
-  DexInstruction(DexOpcode op)
+  explicit DexInstruction(DexOpcode op)
       : Gatherable(), m_opcode(op), m_count(count_from_opcode()) {}
 
   DexInstruction(DexOpcode opcode, uint16_t arg) : DexInstruction(opcode) {
@@ -313,7 +313,7 @@ class DexOpcodeData : public DexInstruction {
     memcpy(m_data, opcodes, count * sizeof(uint16_t));
   }
 
-  DexOpcodeData(const std::vector<uint16_t>& opcodes)
+  explicit DexOpcodeData(const std::vector<uint16_t>& opcodes)
       : DexInstruction(&opcodes[0], 0),
         m_data_count(opcodes.size() - 1),
         m_data(new uint16_t[opcodes.size() - 1]) {

diff --git a/libredex/DexOutput.h b/libredex/DexOutput.h
@@ -240,8 +240,8 @@ class GatheredTypes {
   void build_method_map();
 
  public:
-  GatheredTypes(DexClasses* classes,
-                PostLowering const* post_lowering = nullptr);
+  explicit GatheredTypes(DexClasses* classes,
+                         PostLowering const* post_lowering = nullptr);
 
   DexOutputIdx* get_dodx(const uint8_t* base);
   template <class T = decltype(compare_dexstrings)>

diff --git a/libredex/DexPosition.h b/libredex/DexPosition.h
@@ -24,7 +24,7 @@ struct DexPosition final {
   // when a function gets inlined for the first time, all its DexPositions will
   // have the DexPosition of the callsite as their parent.
   DexPosition* parent{nullptr};
-  DexPosition(uint32_t line);
+  explicit DexPosition(uint32_t line);
   DexPosition(DexString* method, DexString* file, uint32_t line);
 
   void bind(DexString* method_, DexString* file_);
@@ -58,7 +58,7 @@ class RealPositionMapper : public PositionMapper {
   void write_map_v2();
 
  public:
-  RealPositionMapper(const std::string& filename_v2)
+  explicit RealPositionMapper(const std::string& filename_v2)
       : m_filename_v2(filename_v2) {}
   DexString* get_source_file(const DexClass*) override;
   uint32_t position_to_line(DexPosition*) override;

diff --git a/libredex/DexStore.h b/libredex/DexStore.h
@@ -43,8 +43,8 @@ class DexStore {
   bool m_generated = false;
 
  public:
-  DexStore(const DexMetadata& metadata) : m_metadata(metadata){};
-  DexStore(const std::string& name);
+  explicit DexStore(const DexMetadata& metadata) : m_metadata(metadata){};
+  explicit DexStore(const std::string& name);
 
   std::string get_name() const;
   const std::string& get_dex_magic() const { return dex_magic; }
@@ -74,12 +74,12 @@ class DexStoreClassesIterator
   classes_iterator m_current_classes;
 
  public:
-  DexStoreClassesIterator(std::vector<DexStore>& stores)
+  explicit DexStoreClassesIterator(std::vector<DexStore>& stores)
       : m_stores(stores),
         m_current_store(stores.begin()),
         m_current_classes(m_current_store->get_dexen().begin()) {}
 
-  DexStoreClassesIterator(const std::vector<DexStore>& stores)
+  explicit DexStoreClassesIterator(const std::vector<DexStore>& stores)
       : m_stores(const_cast<std::vector<DexStore>&>(stores)),
         m_current_store(m_stores.begin()),
         m_current_classes(m_current_store->get_dexen().begin()) {}

diff --git a/libredex/DexTypeEnvironment.h b/libredex/DexTypeEnvironment.h
@@ -160,8 +160,7 @@ class DexTypeEnvironment final
   // insert a redundant '= default'.
   DexTypeEnvironment() = default;
 
-  explicit DexTypeEnvironment(
-      std::initializer_list<std::pair<reg_t, DexTypeDomain>> l)
+  DexTypeEnvironment(std::initializer_list<std::pair<reg_t, DexTypeDomain>> l)
       : ReducedProductAbstractDomain(
             std::make_tuple(RegTypeEnvironment(l), FieldTypeEnvironment())) {}
 

diff --git a/libredex/Dominators.h b/libredex/Dominators.h
@@ -23,7 +23,7 @@ class SimpleFastDominators {
    *
    *    K. D. Cooper et.al. A Simple, Fast Dominance Algorithm.
    */
-  SimpleFastDominators(const typename GraphInterface::Graph& graph) {
+  explicit SimpleFastDominators(const typename GraphInterface::Graph& graph) {
     // Sort nodes in postorder and create a map of each node to its postorder
     // number.
     m_postordering = graph::postorder_sort<GraphInterface>(graph);

diff --git a/libredex/IRList.h b/libredex/IRList.h
@@ -38,7 +38,8 @@ struct TryEntry {
 struct CatchEntry {
   DexType* catch_type;
   MethodItemEntry* next; // always null for catchall
-  CatchEntry(DexType* catch_type) : catch_type(catch_type), next(nullptr) {}
+  explicit CatchEntry(DexType* catch_type)
+      : catch_type(catch_type), next(nullptr) {}
 
   bool operator==(const CatchEntry& other) const;
 };
@@ -75,7 +76,7 @@ struct BranchTarget {
   int32_t case_key;
 
   BranchTarget() = default;
-  BranchTarget(MethodItemEntry* src) : src(src), type(BRANCH_SIMPLE) {}
+  explicit BranchTarget(MethodItemEntry* src) : src(src), type(BRANCH_SIMPLE) {}
 
   BranchTarget(MethodItemEntry* src, int32_t case_key)
       : src(src), type(BRANCH_MULTI), case_key(case_key) {}
@@ -142,15 +143,15 @@ struct MethodItemEntry {
   }
   MethodItemEntry(TryEntryType try_type, MethodItemEntry* catch_start)
       : type(MFLOW_TRY), tentry(new TryEntry(try_type, catch_start)) {}
-  MethodItemEntry(DexType* catch_type)
+  explicit MethodItemEntry(DexType* catch_type)
       : type(MFLOW_CATCH), centry(new CatchEntry(catch_type)) {}
-  MethodItemEntry(BranchTarget* bt) {
+  explicit MethodItemEntry(BranchTarget* bt) {
     this->type = MFLOW_TARGET;
     this->target = bt;
   }
-  MethodItemEntry(std::unique_ptr<DexDebugInstruction> dbgop)
+  explicit MethodItemEntry(std::unique_ptr<DexDebugInstruction> dbgop)
       : type(MFLOW_DEBUG), dbgop(std::move(dbgop)) {}
-  MethodItemEntry(std::unique_ptr<DexPosition> pos)
+  explicit MethodItemEntry(std::unique_ptr<DexPosition> pos)
       : type(MFLOW_POSITION), pos(std::move(pos)) {}
 
   bool operator==(const MethodItemEntry&) const;

diff --git a/libredex/InstructionAnalyzer.h b/libredex/InstructionAnalyzer.h
@@ -164,7 +164,7 @@ class InstructionAnalyzerCombiner final {
         "InstructionAnalyzerBase!");
   }
 
-  InstructionAnalyzerCombiner(typename Analyzers::State... states)
+  explicit InstructionAnalyzerCombiner(typename Analyzers::State... states)
       : m_states(std::make_tuple(states...)) {}
 
   // If all sub-analyzers have a default-constructible state, then this

diff --git a/libredex/KeepReason.h b/libredex/KeepReason.h
@@ -44,7 +44,7 @@ struct Reason {
     const DexMethod* method;
   };
 
-  Reason(KeepReasonType type) : type(type) {
+  explicit Reason(KeepReasonType type) : type(type) {
     always_assert(type != KEEP_RULE && type != REFLECTION);
   }
 

diff --git a/libredex/MethodDevirtualizer.h b/libredex/MethodDevirtualizer.h
@@ -15,17 +15,6 @@ struct DevirtualizerConfigs {
   bool dmethods_not_using_this = true;
   bool dmethods_using_this = false;
   bool ignore_keep = false;
-
-  DevirtualizerConfigs(bool vmethods_not_using_this = true,
-                       bool vmethods_using_this = false,
-                       bool dmethods_not_using_this = true,
-                       bool dmethods_using_this = false,
-                       bool ignore_keep = false)
-      : vmethods_not_using_this(vmethods_not_using_this),
-        vmethods_using_this(vmethods_using_this),
-        dmethods_not_using_this(dmethods_not_using_this),
-        dmethods_using_this(dmethods_using_this),
-        ignore_keep(ignore_keep) {}
 };
 
 struct DevirtualizerMetrics {

diff --git a/libredex/OptData.h b/libredex/OptData.h
@@ -90,7 +90,7 @@ class MethodOptData {
   friend class OptDataMapper;
 
  public:
-  MethodOptData(const DexMethod* method);
+  explicit MethodOptData(const DexMethod* method);
   std::shared_ptr<InsnOptData> get_insn_opt_data(const IRInstruction* insn);
 
  private:
@@ -111,7 +111,7 @@ class ClassOptData {
   friend class OptDataMapper;
 
  public:
-  ClassOptData(const DexClass* cls);
+  explicit ClassOptData(const DexClass* cls);
   std::shared_ptr<MethodOptData> get_meth_opt_data(const DexMethod* method);
 
  private:

diff --git a/libredex/Pass.h b/libredex/Pass.h
@@ -24,7 +24,7 @@ class PassManager;
 
 class Pass : public Configurable {
  public:
-  Pass(const std::string& name) : m_name(name) {
+  explicit Pass(const std::string& name) : m_name(name) {
     PassRegistry::get().register_pass(this);
   }