[Courgette] Refactor: Add and use Instruction*Receptor classes; call ParseFile() in 2 passes.

courgette/assembly_program.cc

Index: courgette/assembly_program.cc
diff --git a/courgette/assembly_program.cc b/courgette/assembly_program.cc
index d36556fc67ad4e90496a551ad5f7b32f51a6bedf..259141d2f1ba014b4863f5fd36a003850afda932 100644
--- a/courgette/assembly_program.cc
+++ b/courgette/assembly_program.cc
@@ -107,6 +107,120 @@ class InstructionWithLabelARM : public InstructionWithLabel {
 
 }  // namespace
 
+/******** InstructionReceptor ********/
+
+InstructionReceptor::InstructionReceptor() {}
+
+InstructionReceptor::~InstructionReceptor() {}
+
+/******** InstructionCountReceptor ********/
+
+InstructionCountReceptor::InstructionCountReceptor() : size_(0) {}
+
+InstructionCountReceptor::~InstructionCountReceptor() {}
+
+// TODO(huangs): Populate these with size_ += ...

[grt (UTC plus 2), 2016/11/04 09:38:53]

TODO when?

[huangs, 2016/11/04 18:20:39]

The follow-up CL, which focuses on AssemblyProgram.  Added 2016/11.

+CheckBool InstructionCountReceptor::EmitPeRelocs() {
+  return true;
+}
+
+CheckBool InstructionCountReceptor::EmitElfRelocation() {
+  return true;
+}
+
+CheckBool InstructionCountReceptor::EmitElfARMRelocation() {
+  return true;
+}
+
+CheckBool InstructionCountReceptor::EmitOrigin(RVA rva) {
+  return true;
+}
+
+CheckBool InstructionCountReceptor::EmitSingleByte(uint8_t byte) {
+  return true;
+}
+
+CheckBool InstructionCountReceptor::EmitMultipleBytes(const uint8_t* value,
+                                                      size_t len) {
+  return true;
+}
+
+CheckBool InstructionCountReceptor::EmitRel32(Label* label) {
+  return true;
+}
+
+CheckBool InstructionCountReceptor::EmitRel32ARM(uint16_t op,
+                                                 Label* label,
+                                                 const uint8_t* arm_op,
+                                                 uint16_t op_size) {
+  return true;
+}
+
+CheckBool InstructionCountReceptor::EmitAbs32(Label* label) {
+  return true;
+}
+
+CheckBool InstructionCountReceptor::EmitAbs64(Label* label) {
+  return true;
+}
+
+/******** InstructionStoreReceptor ********/
+
+// TODO(huangs): Populate these.

[grt (UTC plus 2), 2016/11/04 09:38:53]

when?

[huangs, 2016/11/04 18:20:39]

Same.

+InstructionStoreReceptor::InstructionStoreReceptor(size_t init_size,
+                                                   AssemblyProgram* program)
+    : program_(program) {
+  CHECK(program_ != nullptr);

[grt (UTC plus 2), 2016/11/04 09:38:53]

CHECK_NE(program_, nullptr);

[huangs, 2016/11/04 18:20:39]

Should be consistent with later uses (DCHECK). Comparing choices:

CHECK(pointer);  // Must not be null.
CHECK(!pointer);  // Must be null.

vs.

CHECK_NE(pointer, nullptr);  // Must not be null.
CHECK_EQ(pointer, nullptr);  // Must be null.

Changing to the first one for now because I think it's easier to read. But let
me know if you prefer the other and I'll change throughout.

[grt (UTC plus 2), 2016/11/07 09:36:17]

I like the brief approach as well.

[huangs, 2016/11/07 19:46:52]

Acknowledged.

+}
+
+InstructionStoreReceptor::~InstructionStoreReceptor() {}
+
+CheckBool InstructionStoreReceptor::EmitPeRelocs() {
+  return program_->EmitPeRelocs();
+}
+
+CheckBool InstructionStoreReceptor::EmitElfRelocation() {
+  return program_->EmitElfRelocation();
+}
+
+CheckBool InstructionStoreReceptor::EmitElfARMRelocation() {
+  return program_->EmitElfARMRelocation();
+}
+
+CheckBool InstructionStoreReceptor::EmitOrigin(RVA rva) {
+  return program_->EmitOrigin(rva);
+}
+
+CheckBool InstructionStoreReceptor::EmitSingleByte(uint8_t byte) {
+  return program_->EmitSingleByte(byte);
+}
+
+CheckBool InstructionStoreReceptor::EmitMultipleBytes(const uint8_t* value,
+                                                      size_t len) {
+  return program_->EmitMultipleBytes(value, len);
+}
+
+CheckBool InstructionStoreReceptor::EmitRel32(Label* label) {
+  return program_->EmitRel32(label);
+}
+
+CheckBool InstructionStoreReceptor::EmitRel32ARM(uint16_t op,
+                                                 Label* label,
+                                                 const uint8_t* arm_op,
+                                                 uint16_t op_size) {
+  return program_->EmitRel32ARM(op, label, arm_op, op_size);
+}
+
+CheckBool InstructionStoreReceptor::EmitAbs32(Label* label) {
+  return program_->EmitAbs32(label);
+}
+
+CheckBool InstructionStoreReceptor::EmitAbs64(Label* label) {
+  return program_->EmitAbs64(label);
+}
+
+/******** AssemblyProgram ********/
+
 AssemblyProgram::AssemblyProgram(ExecutableType kind)
   : kind_(kind), image_base_(0) {
 }
@@ -123,28 +237,28 @@ AssemblyProgram::~AssemblyProgram() {
   }
 }
 
-CheckBool AssemblyProgram::EmitPeRelocsInstruction() {
+CheckBool AssemblyProgram::EmitPeRelocs() {
   return Emit(ScopedInstruction(UncheckedNew<PeRelocsInstruction>()));
 }
 
-CheckBool AssemblyProgram::EmitElfRelocationInstruction() {
+CheckBool AssemblyProgram::EmitElfRelocation() {
   return Emit(ScopedInstruction(UncheckedNew<ElfRelocsInstruction>()));
 }
 
-CheckBool AssemblyProgram::EmitElfARMRelocationInstruction() {
+CheckBool AssemblyProgram::EmitElfARMRelocation() {
   return Emit(ScopedInstruction(UncheckedNew<ElfARMRelocsInstruction>()));
 }
 
-CheckBool AssemblyProgram::EmitOriginInstruction(RVA rva) {
+CheckBool AssemblyProgram::EmitOrigin(RVA rva) {
   return Emit(ScopedInstruction(UncheckedNew<OriginInstruction>(rva)));
 }
 
-CheckBool AssemblyProgram::EmitByteInstruction(uint8_t byte) {
+CheckBool AssemblyProgram::EmitSingleByte(uint8_t byte) {
   return EmitShared(GetByteInstruction(byte));
 }
 
-CheckBool AssemblyProgram::EmitBytesInstruction(const uint8_t* values,
-                                                size_t len) {
+CheckBool AssemblyProgram::EmitMultipleBytes(const uint8_t* values,
+                                             size_t len) {
   return Emit(ScopedInstruction(UncheckedNew<BytesInstruction>(values, len)));
 }
 
@@ -228,6 +342,26 @@ void AssemblyProgram::HandleInstructionLabels(
   }
 }
 
+CheckBool AssemblyProgram::CreateInstructionCountReceptor(

[grt (UTC plus 2), 2016/11/04 09:38:53]

why CheckBool rather than returning a pointer since this can never fail?

[huangs, 2016/11/04 18:20:39]

For symmetry with CreateInstructionStoreReceptor(), which may fail.

[grt (UTC plus 2), 2016/11/07 09:36:17]

Ah. May fail in the future in a different CL. I'm not sure that splitting this
into two CLs makes sense since this CL raises questions that are answered in the
other. I'll try to review them independently anyway.

[huangs, 2016/11/07 19:46:52]

Okay, thanks!

+    InstructionCountReceptor** ret) {
+  DCHECK(ret);
+  DCHECK(!count_receptor_);
+  count_receptor_.reset(new InstructionCountReceptor);
+  *ret = count_receptor_.get();
+  return true;
+}
+
+CheckBool AssemblyProgram::CreateInstructionStoreReceptor(
+    InstructionStoreReceptor** ret) {
+  DCHECK(ret);
+  DCHECK(count_receptor_);
+  DCHECK(!store_receptor_);
+  store_receptor_.reset(
+      new InstructionStoreReceptor(count_receptor_->size(), this));
+  *ret = store_receptor_.get();
+  return true;
+}
+
 CheckBool AssemblyProgram::Emit(ScopedInstruction instruction) {
   if (!instruction || !instructions_.push_back(instruction.get()))
     return false;