Fix more MSVC warnings, courgette/ edition.

courgette/encoded_program.cc

Index: courgette/encoded_program.cc
diff --git a/courgette/encoded_program.cc b/courgette/encoded_program.cc
index b78e7fef93cc9092c81e38c0d1d9af94dc7fac15..b120246d4bc73db694da53797465f9ab8c14b21d 100644
--- a/courgette/encoded_program.cc
+++ b/courgette/encoded_program.cc
@@ -46,8 +46,6 @@ CheckBool WriteVector(const V& items, SinkStream* buffer) {
   size_t count = items.size();
   bool ok = buffer->WriteSizeVarint32(count);
   for (size_t i = 0; ok && i < count;  ++i) {
-    COMPILE_ASSERT(sizeof(items[0]) <= sizeof(uint32),  // NOLINT
-                   T_must_fit_in_uint32);

[Peter Kasting, 2014/09/30 00:08:01]

This assertion isn't correct.  It's legal to pass in types larger than uint32 as
long as they hold values that can be expressed in 32 bits -- something which
WriteSizeVarint32() LOG_ASSERT()s already.

     ok = buffer->WriteSizeVarint32(items[i]);
   }
   return ok;
@@ -196,7 +194,7 @@ CheckBool EncodedProgram::AddOrigin(RVA origin) {
   return ops_.push_back(ORIGIN) && origins_.push_back(origin);
 }
 
-CheckBool EncodedProgram::AddCopy(uint32 count, const void* bytes) {
+CheckBool EncodedProgram::AddCopy(size_t count, const void* bytes) {
   const uint8* source = static_cast<const uint8*>(bytes);
 
   bool ok = true;
@@ -214,7 +212,7 @@ CheckBool EncodedProgram::AddCopy(uint32 count, const void* bytes) {
     }
     if (ok && ops_.back() == COPY) {
       copy_counts_.back() += count;
-      for (uint32 i = 0; ok && i < count; ++i) {
+      for (size_t i = 0; ok && i < count; ++i) {
         ok = copy_bytes_.push_back(source[i]);
       }
       return ok;
@@ -226,7 +224,7 @@ CheckBool EncodedProgram::AddCopy(uint32 count, const void* bytes) {
       ok = ops_.push_back(COPY1) && copy_bytes_.push_back(source[0]);
     } else {
       ok = ops_.push_back(COPY) && copy_counts_.push_back(count);
-      for (uint32 i = 0; ok && i < count; ++i) {
+      for (size_t i = 0; ok && i < count; ++i) {
         ok = copy_bytes_.push_back(source[i]);
       }
     }
@@ -427,7 +425,7 @@ CheckBool EncodedProgram::EvaluateRel32ARM(OP op,
                                             &decompressed_op)) {
         return false;
       }
-      uint16 op16 = decompressed_op;
+      uint16 op16 = static_cast<uint16>(decompressed_op);
       if (!output->Write(&op16, 2))
         return false;
       current_rva += 2;
@@ -447,7 +445,7 @@ CheckBool EncodedProgram::EvaluateRel32ARM(OP op,
                                             &decompressed_op)) {
         return false;
       }
-      uint16 op16 = decompressed_op;
+      uint16 op16 = static_cast<uint16>(decompressed_op);
       if (!output->Write(&op16, 2))
         return false;
       current_rva += 2;
@@ -556,11 +554,11 @@ CheckBool EncodedProgram::AssembleTo(SinkStream* final_buffer) {
       }
 
       case COPY: {
-        uint32 count;
+        size_t count;
         if (!VectorAt(copy_counts_, ix_copy_counts, &count))
           return false;
         ++ix_copy_counts;
-        for (uint32 i = 0;  i < count;  ++i) {
+        for (size_t i = 0;  i < count;  ++i) {
           uint8 b;
           if (!VectorAt(copy_bytes_, ix_copy_bytes, &b))
             return false;
@@ -568,7 +566,7 @@ CheckBool EncodedProgram::AssembleTo(SinkStream* final_buffer) {
           if (!output->Write(&b, 1))
             return false;
         }
-        current_rva += count;
+        current_rva += static_cast<RVA>(count);

[Peter Kasting, 2014/09/30 00:08:01]

It's somewhat inconsistent for me to complain earlier about it not being obvious
that casting a size_t to a uint32 at the callsite of EmitBytesInstruction() is
safe, but then allow that same cast to happen here.  I imagine changing RVA from
a uint32 to a size_t is out of the question, so another possibility besides
doing this is to use RVA (instead of size_t) for the type in the places I
modified in this CL.  Or perhaps the code here should do some sort of
checked_cast or something.

[dgarrett, 2014/10/01 20:49:37]

I suspect that count should remain a uint32 here. That IS what's being read out
of the encoded format.

[Peter Kasting, 2014/10/01 22:12:05]

? |copy_counts_| is now a SizeTVector, so |count| really is a size_t.  This goes
hand-in-hand with changing AddCopy() to take a size_t, etc.

As I said previously, it would also be possible to undo all the size_t changes
(they're all connected) and use RVA everywhere, in which case the cast here
would basically move to DisassemblerElf32::ParseSimpleRegion().  We could then
decide to upgrade that cast to something like a checked_cast.

(Or we could just upgrade the cast here.)

         break;
       }