Remove destructor

Summary:
The destructor used to apply scope-wide changes, a quite heavy-weight operation, possibly exception throwing.
This could be surprising, and also made the Linter really unhappy.
This formerly implicit behavior is now made explicit via a `flush` function.

This is a behavior-preserving change.

Reviewed By: thezhangwei

Differential Revision: D57064529

fbshipit-source-id: 7013853d80994c5be062e5ee834ed106f4a4e03a

libredex/DexUtil.cpp

@@ -279,6 +279,12 @@ void VisibilityChanges::insert(const VisibilityChanges& other) {
   methods.insert(other.methods.begin(), other.methods.end());
 }
 
+void VisibilityChanges::clear() {
+  classes.clear();
+  fields.clear();
+  methods.clear();
+}
+
 void VisibilityChanges::apply() const {
   for (auto cls : classes) {
     set_public(cls);

libredex/DexUtil.h

@@ -39,6 +39,7 @@ struct VisibilityChanges {
   void insert(const VisibilityChanges& other);
   void apply() const;
   bool empty() const;
+  void clear();
 };
 
 /**

opt/builder_pattern/BuilderTransform.cpp

@@ -57,8 +57,6 @@ std::unordered_set<IRInstruction*> BuilderTransform::try_inline_calls(
   return not_inlined_insns;
 }
 
-BuilderTransform::~BuilderTransform() {}
-
 /**
  * For all the methods of the given type, try inlining all super calls and
  * constructors of the super type. If any of them fails, return false.
@@ -292,6 +290,7 @@ void BuilderTransform::cleanup() {
     DexMethod::erase_method(copy);
     DexMethod::delete_method_DO_NOT_USE(copy);
   }
+  m_inliner->flush();
 }
 
 } // namespace builder_pattern

opt/builder_pattern/BuilderTransform.h

@@ -24,7 +24,7 @@ class BuilderTransform {
                        init_classes_with_side_effects,
                    const inliner::InlinerConfig& inliner_config,
                    DexStoresVector& stores);
-  ~BuilderTransform();
+  void flush();
 
   bool inline_super_calls_and_ctors(const DexType* type);
 

opt/instrument/BlockInstrument.cpp

@@ -2062,6 +2062,7 @@ void BlockInstrumentHelper::do_basic_block_tracing(
         analysis_cls, field->get_name()->str(),
         static_cast<int>(ProfileTypeFlags::BasicBlockHitCount));
   }
+  inliner.flush();
 
   write_metadata(cfg, options.metadata_file_name, instrumented_methods,
                  options.instrumentation_strategy);

opt/object-escape-analysis/ObjectEscapeAnalysis.cpp

@@ -2085,6 +2085,8 @@ void ObjectEscapeAnalysisPass::run_pass(DexStoresVector& stores,
          &inlinable_methods_kept, &callees_cache, &method_summary_cache,
          &conf.get_method_profiles());
 
+  inliner.flush();
+
   TRACE(OEA, 1, "[object escape analysis] total savings: %zu",
         (size_t)stats.total_savings);
   TRACE(

opt/remove-builders/RemoveBuilders.cpp

@@ -404,6 +404,8 @@ void RemoveBuildersPass::run_pass(DexStoresVector& stores,
   TRACE(BUILDERS, 1, "Methods removed: %zu", b_counter.methods_removed);
   TRACE(BUILDERS, 1, "Fields removed: %zu", b_counter.fields_removed);
   TRACE(BUILDERS, 1, "Methods cleared: %zu", b_counter.methods_cleared);
+
+  b_transform.flush();
 }
 
 static RemoveBuildersPass s_pass;

opt/remove-builders/RemoveBuildersHelper.h

@@ -115,6 +115,8 @@ class BuilderTransform {
       const std::function<std::unordered_set<DexMethod*>(IRCode*, DexType*)>&
           get_methods_to_inline);
 
+  void flush() { m_inliner->flush(); }
+
  private:
   std::unique_ptr<MultiMethodInliner> m_inliner;
   inliner::InlinerConfig m_inliner_config;

opt/virtual_merging/VirtualMerging.cpp

@@ -124,6 +124,9 @@ VirtualMerging::VirtualMerging(DexStoresVector& stores,
       /* const std::unordered_set<DexMethodRef*>& configured_pure_methods */ {},
       min_sdk_api));
 }
+
+void VirtualMerging::flush() { m_inliner->flush(); }
+
 VirtualMerging::~VirtualMerging() {}
 
 // Part 1: Identify which virtual methods get invoked via invoke-super --- we'll
@@ -1513,6 +1516,7 @@ void VirtualMergingPass::run_pass(DexStoresVector& stores,
                     m_perf_config);
   vm.run(conf.get_method_profiles(), m_strategy, m_insertion_strategy);
   auto stats = vm.get_stats();
+  vm.flush();
 
   mgr.incr_metric(METRIC_DEDUPPED_VIRTUAL_METHODS, dedupped);
   mgr.incr_metric(METRIC_INVOKE_SUPER_METHODS, stats.invoke_super_methods);

opt/virtual_merging/VirtualMerging.h

@@ -156,6 +156,8 @@ class VirtualMerging {
                std::vector<std::pair<const DexMethod*, const DexMethod*>>,
                virtualscopes_comparator>;
 
+  void flush();
+
  private:
   MergablePairsByVirtualScope compute_mergeable_pairs_by_virtual_scopes(
       const method_profiles::MethodProfiles&,

service/method-inliner/Inliner.cpp

@@ -348,8 +348,7 @@ void MultiMethodInliner::inline_methods() {
   info.critical_path_length =
       m_scheduler.run(methods_to_schedule.begin(), methods_to_schedule.end());
 
-  delayed_visibility_changes_apply();
-  delayed_invoke_direct_to_static();
+  flush();
   info.waited_seconds = m_scheduler.get_thread_pool().get_waited_seconds();
 }
 
@@ -2177,8 +2176,12 @@ bool MultiMethodInliner::cross_store_reference(const DexMethod* caller,
 }
 
 void MultiMethodInliner::delayed_visibility_changes_apply() {
+  if (!m_delayed_visibility_changes) {
+    return;
+  }
   visibility_changes_apply_and_record_make_static(
       *m_delayed_visibility_changes);
+  m_delayed_visibility_changes->clear();
 }
 
 void MultiMethodInliner::visibility_changes_apply_and_record_make_static(

service/method-inliner/Inliner.h

@@ -251,10 +251,17 @@ class MultiMethodInliner {
       bool local_only = false,
       InlinerCostConfig inliner_cost_config = DEFAULT_COST_CONFIG);
 
-  ~MultiMethodInliner() { delayed_invoke_direct_to_static(); }
+  /*
+   * Applies certain delayed scope-wide changes, including in particular
+   * visibility and staticizing changes.
+   */
+  void flush() {
+    delayed_visibility_changes_apply();
+    delayed_invoke_direct_to_static();
+  }
 
   /**
-   * attempt inlining for all candidates.
+   * Attempt inlining for all candidates, and flushes scope-wide changes.
    */
   void inline_methods();
 
@@ -532,9 +539,6 @@ class MultiMethodInliner {
   /**
    * Staticize required methods (stored in `m_delayed_make_static`) and update
    * opcodes accordingly.
-   *
-   * NOTE: It only needs to be called once after inlining. Since it is called
-   *       from the destructor, there is no need to manually call it.
    */
   void delayed_invoke_direct_to_static();