Clean up DexInstuctionIterator.

Follow-up to
    https://android-review.googlesource.com/493297 .

Test: m test-art-host-gtest
Test: testrunner.py --host
Bug: 63756964
Change-Id: I454a748858e54f7ddfc54f631d7cd97d63557aff

3 files changed, 46 insertions, 45 deletions

diff --git a/runtime/dex_instruction_iterator.h b/runtime/dex_instruction_iterator.h
index 6585875df4..280746e9dc 100644
--- a/runtime/dex_instruction_iterator.h
+++ b/runtime/dex_instruction_iterator.h
@@ -26,8 +26,8 @@ namespace art {
 
 class DexInstructionIterator : public std::iterator<std::forward_iterator_tag, Instruction> {
  public:
-  using Type = std::iterator<std::forward_iterator_tag, Instruction>::value_type;
-  using difference_type = typename std::iterator<std::forward_iterator_tag, Type>::difference_type;
+  using value_type = std::iterator<std::forward_iterator_tag, Instruction>::value_type;
+  using difference_type = std::iterator<std::forward_iterator_tag, value_type>::difference_type;
 
   DexInstructionIterator() = default;
   DexInstructionIterator(const DexInstructionIterator&) = default;
@@ -35,12 +35,8 @@ class DexInstructionIterator : public std::iterator<std::forward_iterator_tag, I
   DexInstructionIterator& operator=(const DexInstructionIterator&) = default;
   DexInstructionIterator& operator=(DexInstructionIterator&&) = default;
 
-  explicit DexInstructionIterator(const Type* inst) : inst_(inst) {}
-  explicit DexInstructionIterator(const uint16_t* inst) : inst_(Type::At(inst)) {}
-
-  ALWAYS_INLINE bool operator==(const DexInstructionIterator& other) const {
-    return inst_ == other.inst_ || inst_ == nullptr || other.inst_ == nullptr;
-  }
+  explicit DexInstructionIterator(const value_type* inst) : inst_(inst) {}
+  explicit DexInstructionIterator(const uint16_t* inst) : inst_(value_type::At(inst)) {}
 
   // Value after modification.
   DexInstructionIterator& operator++() {
@@ -55,11 +51,11 @@ class DexInstructionIterator : public std::iterator<std::forward_iterator_tag, I
     return temp;
   }
 
-  const Type& operator*() const {
+  const value_type& operator*() const {
     return *inst_;
   }
 
-  const Type* operator->() const {
+  const value_type* operator->() const {
     return &**this;
   }
 
@@ -69,14 +65,19 @@ class DexInstructionIterator : public std::iterator<std::forward_iterator_tag, I
         reinterpret_cast<const uint16_t*>(code_item_begin.inst_);
   }
 
-  const Type* Inst() const {
+  const value_type* Inst() const {
     return inst_;
   }
 
  private:
-  const Type* inst_ = nullptr;
+  const value_type* inst_ = nullptr;
 };
 
+static ALWAYS_INLINE inline bool operator==(const DexInstructionIterator& lhs,
+                                            const DexInstructionIterator& rhs) {
+  return lhs.Inst() == rhs.Inst();
+}
+
 static inline bool operator!=(const DexInstructionIterator& lhs,
                               const DexInstructionIterator& rhs) {
   return !(lhs == rhs);
diff --git a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
index eef277398c..813a264ed9 100644
--- a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
+++ b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
@@ -1119,8 +1119,8 @@ extern "C" const void* artQuickResolutionTrampoline(
       const DexFile::CodeItem* code;
       code = caller->GetCodeItem();
       CHECK_LT(dex_pc, code->insns_size_in_code_units_);
-      const Instruction* instr = &code->InstructionAt(dex_pc);
-      Instruction::Code instr_code = instr->Opcode();
+      const Instruction& instr = code->InstructionAt(dex_pc);
+      Instruction::Code instr_code = instr.Opcode();
       bool is_range;
       switch (instr_code) {
         case Instruction::INVOKE_DIRECT:
@@ -1164,10 +1164,10 @@ extern "C" const void* artQuickResolutionTrampoline(
           is_range = true;
           break;
         default:
-          LOG(FATAL) << "Unexpected call into trampoline: " << instr->DumpString(nullptr);
+          LOG(FATAL) << "Unexpected call into trampoline: " << instr.DumpString(nullptr);
           UNREACHABLE();
       }
-      called_method.index = (is_range) ? instr->VRegB_3rc() : instr->VRegB_35c();
+      called_method.index = (is_range) ? instr.VRegB_3rc() : instr.VRegB_35c();
       // Check that the invoke matches what we expected, note that this path only happens for debug
       // builds.
       if (found_stack_map) {
@@ -2484,16 +2484,16 @@ extern "C" TwoWordReturn artInvokeInterfaceTrampoline(ArtMethod* interface_metho
     uint32_t dex_pc = QuickArgumentVisitor::GetCallingDexPc(sp);
     const DexFile::CodeItem* code_item = caller_method->GetCodeItem();
     DCHECK_LT(dex_pc, code_item->insns_size_in_code_units_);
-    const Instruction* instr = &code_item->InstructionAt(dex_pc);
-    Instruction::Code instr_code = instr->Opcode();
+    const Instruction& instr = code_item->InstructionAt(dex_pc);
+    Instruction::Code instr_code = instr.Opcode();
     DCHECK(instr_code == Instruction::INVOKE_INTERFACE ||
            instr_code == Instruction::INVOKE_INTERFACE_RANGE)
-        << "Unexpected call into interface trampoline: " << instr->DumpString(nullptr);
+        << "Unexpected call into interface trampoline: " << instr.DumpString(nullptr);
     if (instr_code == Instruction::INVOKE_INTERFACE) {
-      dex_method_idx = instr->VRegB_35c();
+      dex_method_idx = instr.VRegB_35c();
     } else {
       DCHECK_EQ(instr_code, Instruction::INVOKE_INTERFACE_RANGE);
-      dex_method_idx = instr->VRegB_3rc();
+      dex_method_idx = instr.VRegB_3rc();
     }
 
     const DexFile& dex_file = caller_method->GetDeclaringClass()->GetDexFile();
@@ -2600,11 +2600,11 @@ extern "C" uintptr_t artInvokePolymorphic(
   ArtMethod* caller_method = QuickArgumentVisitor::GetCallingMethod(sp);
   uint32_t dex_pc = QuickArgumentVisitor::GetCallingDexPc(sp);
   const DexFile::CodeItem* code = caller_method->GetCodeItem();
-  const Instruction* inst = &code->InstructionAt(dex_pc);
-  DCHECK(inst->Opcode() == Instruction::INVOKE_POLYMORPHIC ||
-         inst->Opcode() == Instruction::INVOKE_POLYMORPHIC_RANGE);
+  const Instruction& inst = code->InstructionAt(dex_pc);
+  DCHECK(inst.Opcode() == Instruction::INVOKE_POLYMORPHIC ||
+         inst.Opcode() == Instruction::INVOKE_POLYMORPHIC_RANGE);
   const DexFile* dex_file = caller_method->GetDexFile();
-  const uint32_t proto_idx = inst->VRegH();
+  const uint32_t proto_idx = inst.VRegH();
   const char* shorty = dex_file->GetShorty(proto_idx);
   const size_t shorty_length = strlen(shorty);
   static const bool kMethodIsStatic = false;  // invoke() and invokeExact() are not static.
@@ -2621,7 +2621,7 @@ extern "C" uintptr_t artInvokePolymorphic(
   // Resolve method - it's either MethodHandle.invoke() or MethodHandle.invokeExact().
   ClassLinker* linker = Runtime::Current()->GetClassLinker();
   ArtMethod* resolved_method = linker->ResolveMethod<ClassLinker::ResolveMode::kCheckICCEAndIAE>(
-      self, inst->VRegB(), caller_method, kVirtual);
+      self, inst.VRegB(), caller_method, kVirtual);
   DCHECK((resolved_method ==
           jni::DecodeArtMethod(WellKnownClasses::java_lang_invoke_MethodHandle_invokeExact)) ||
          (resolved_method ==
@@ -2642,15 +2642,15 @@ extern "C" uintptr_t artInvokePolymorphic(
     return static_cast<uintptr_t>('V');
   }
 
-  DCHECK_EQ(ArtMethod::NumArgRegisters(shorty) + 1u, (uint32_t)inst->VRegA());
+  DCHECK_EQ(ArtMethod::NumArgRegisters(shorty) + 1u, (uint32_t)inst.VRegA());
   DCHECK_EQ(resolved_method->IsStatic(), kMethodIsStatic);
 
   // Fix references before constructing the shadow frame.
   gc_visitor.FixupReferences();
 
   // Construct shadow frame placing arguments consecutively from |first_arg|.
-  const bool is_range = (inst->Opcode() == Instruction::INVOKE_POLYMORPHIC_RANGE);
-  const size_t num_vregs = is_range ? inst->VRegA_4rcc() : inst->VRegA_45cc();
+  const bool is_range = (inst.Opcode() == Instruction::INVOKE_POLYMORPHIC_RANGE);
+  const size_t num_vregs = is_range ? inst.VRegA_4rcc() : inst.VRegA_45cc();
   const size_t first_arg = 0;
   ShadowFrameAllocaUniquePtr shadow_frame_unique_ptr =
       CREATE_SHADOW_FRAME(num_vregs, /* link */ nullptr, resolved_method, dex_pc);
diff --git a/runtime/verifier/method_verifier.cc b/runtime/verifier/method_verifier.cc
index b789ac6b8d..cfdf20d3b9 100644
--- a/runtime/verifier/method_verifier.cc
+++ b/runtime/verifier/method_verifier.cc
@@ -1955,7 +1955,7 @@ bool MethodVerifier::CodeFlowVerifyMethod() {
     if (dead_start >= 0) {
       LogVerifyInfo()
           << "dead code " << reinterpret_cast<void*>(dead_start)
-          << "-" << reinterpret_cast<void*>(instructions.end().GetDexPC(instructions.begin()));
+          << "-" << reinterpret_cast<void*>(instructions.end().GetDexPC(instructions.begin()) - 1);
     }
     // To dump the state of the verify after a method, do something like:
     // if (dex_file_->PrettyMethod(dex_method_idx_) ==
@@ -2637,7 +2637,7 @@ bool MethodVerifier::CodeFlowVerifyInstruction(uint32_t* start_guess) {
         break;
       }
 
-      const Instruction* instance_of_inst = &code_item_->InstructionAt(instance_of_idx);
+      const Instruction& instance_of_inst = code_item_->InstructionAt(instance_of_idx);
 
       /* Check for peep-hole pattern of:
        *    ...;
@@ -2652,9 +2652,9 @@ bool MethodVerifier::CodeFlowVerifyInstruction(uint32_t* start_guess) {
        *  - when vX == vY.
        */
       if (!CurrentInsnFlags()->IsBranchTarget() &&
-          (Instruction::INSTANCE_OF == instance_of_inst->Opcode()) &&
-          (inst->VRegA_21t() == instance_of_inst->VRegA_22c()) &&
-          (instance_of_inst->VRegA_22c() != instance_of_inst->VRegB_22c())) {
+          (Instruction::INSTANCE_OF == instance_of_inst.Opcode()) &&
+          (inst->VRegA_21t() == instance_of_inst.VRegA_22c()) &&
+          (instance_of_inst.VRegA_22c() != instance_of_inst.VRegB_22c())) {
         // Check the type of the instance-of is different than that of registers type, as if they
         // are the same there is no work to be done here. Check that the conversion is not to or
         // from an unresolved type as type information is imprecise. If the instance-of is to an
@@ -2665,9 +2665,9 @@ bool MethodVerifier::CodeFlowVerifyInstruction(uint32_t* start_guess) {
         // type is assignable to the original then allow optimization. This check is performed to
         // ensure that subsequent merges don't lose type information - such as becoming an
         // interface from a class that would lose information relevant to field checks.
-        const RegType& orig_type = work_line_->GetRegisterType(this, instance_of_inst->VRegB_22c());
+        const RegType& orig_type = work_line_->GetRegisterType(this, instance_of_inst.VRegB_22c());
         const RegType& cast_type = ResolveClass<CheckAccess::kYes>(
-            dex::TypeIndex(instance_of_inst->VRegC_22c()));
+            dex::TypeIndex(instance_of_inst.VRegC_22c()));
 
         if (!orig_type.Equals(cast_type) &&
             !cast_type.IsUnresolvedTypes() && !orig_type.IsUnresolvedTypes() &&
@@ -2684,7 +2684,7 @@ bool MethodVerifier::CodeFlowVerifyInstruction(uint32_t* start_guess) {
           }
           update_line->CopyFromLine(work_line_.get());
           update_line->SetRegisterType<LockOp::kKeep>(this,
-                                                      instance_of_inst->VRegB_22c(),
+                                                      instance_of_inst.VRegB_22c(),
                                                       cast_type);
           if (!GetInstructionFlags(instance_of_idx).IsBranchTarget() && 0 != instance_of_idx) {
             // See if instance-of was preceded by a move-object operation, common due to the small
@@ -2699,26 +2699,26 @@ bool MethodVerifier::CodeFlowVerifyInstruction(uint32_t* start_guess) {
                             work_insn_idx_)) {
               break;
             }
-            const Instruction* move_inst = &code_item_->InstructionAt(move_idx);
-            switch (move_inst->Opcode()) {
+            const Instruction& move_inst = code_item_->InstructionAt(move_idx);
+            switch (move_inst.Opcode()) {
               case Instruction::MOVE_OBJECT:
-                if (move_inst->VRegA_12x() == instance_of_inst->VRegB_22c()) {
+                if (move_inst.VRegA_12x() == instance_of_inst.VRegB_22c()) {
                   update_line->SetRegisterType<LockOp::kKeep>(this,
-                                                              move_inst->VRegB_12x(),
+                                                              move_inst.VRegB_12x(),
                                                               cast_type);
                 }
                 break;
               case Instruction::MOVE_OBJECT_FROM16:
-                if (move_inst->VRegA_22x() == instance_of_inst->VRegB_22c()) {
+                if (move_inst.VRegA_22x() == instance_of_inst.VRegB_22c()) {
                   update_line->SetRegisterType<LockOp::kKeep>(this,
-                                                              move_inst->VRegB_22x(),
+                                                              move_inst.VRegB_22x(),
                                                               cast_type);
                 }
                 break;
               case Instruction::MOVE_OBJECT_16:
-                if (move_inst->VRegA_32x() == instance_of_inst->VRegB_22c()) {
+                if (move_inst.VRegA_32x() == instance_of_inst.VRegB_22c()) {
                   update_line->SetRegisterType<LockOp::kKeep>(this,
-                                                              move_inst->VRegB_32x(),
+                                                              move_inst.VRegB_32x(),
                                                               cast_type);
                 }
                 break;