win: Fixes for minidump_string_writer_test.cc

minidump/minidump_string_writer_test.cc

Index: minidump/minidump_string_writer_test.cc
diff --git a/minidump/minidump_string_writer_test.cc b/minidump/minidump_string_writer_test.cc
index 8c04bcd36dbc4da2699ccaf44cdb41ef2ba8d579..bf47274879608fe33df34d76a16a0b5a08940bc6 100644
--- a/minidump/minidump_string_writer_test.cc
+++ b/minidump/minidump_string_writer_test.cc
@@ -51,6 +51,12 @@ TEST(MinidumpStringWriter, MinidumpUTF16StringWriter) {
               MinidumpStringAtRVAAsString(file_writer.string(), 0));
   }
 
+#if defined(OS_WIN)

[Mark Mentovai, 2015/02/05 14:37:42]

COMPILER_MSVC, not OS_WIN.

Make sure the rest of your MSVC #pragma warnings are COMPILER_MSVC too. I think
they are.

+#pragma warning(push)
+// Can't generate default constructor, assignment operator, can never be

[Mark Mentovai, 2015/02/05 14:37:42]

This is OK, but I’m surprised to see this kind of warning being generated here.
This pattern for test data shows up in Chromium too, like on line 173 of this
file, and I don’t recall seeing #pragma warning for it ever before. Is the
compiler getting thrown off by the char16 array or is it the const that’s doing
it?

[scottmg, 2015/02/05 17:09:39]

It's the const. Removing that seems better?

+// instantiated.
+#pragma warning(disable: 4510 4512 4610)
+#endif  // OS_WIN
   const struct {
     size_t input_length;
     const char* input_string;
@@ -69,6 +75,9 @@ TEST(MinidumpStringWriter, MinidumpUTF16StringWriter) {
       {8, "o\303\263\303\266\305\221o", 5, {'o', 0x00f3, 0x00f6, 0x151, 'o'}},
       {4, "\360\220\204\202", 2, {0xd800, 0xdd02}},  // 𐄂 (non-BMP)
   };
+#if defined(OS_WIN)
+#pragma warning(pop)
+#endif  // OS_WIN
 
   for (size_t index = 0; index < arraysize(kTestData); ++index) {
     SCOPED_TRACE(base::StringPrintf(
@@ -89,8 +98,9 @@ TEST(MinidumpStringWriter, MinidumpUTF16StringWriter) {
 
     const size_t expected_utf16_units_with_nul =
         kTestData[index].output_length + 1;
+    MINIDUMP_STRING tmp = {0}; (void)tmp;

[Mark Mentovai, 2015/02/05 14:37:42]

ALLOW_UNUSED_LOCAL here and on line 145.

[scottmg, 2015/02/05 17:09:39]

Done.

     const size_t expected_utf16_bytes =
-        expected_utf16_units_with_nul * sizeof(MINIDUMP_STRING::Buffer[0]);
+        expected_utf16_units_with_nul * sizeof(tmp.Buffer[0]);
     ASSERT_EQ(sizeof(MINIDUMP_STRING) + expected_utf16_bytes,
               file_writer.string().size());
 
@@ -132,8 +142,9 @@ TEST(MinidumpStringWriter, ConvertInvalidUTF8ToUTF16) {
     const MINIDUMP_STRING* minidump_string =
         MinidumpStringAtRVA(file_writer.string(), 0);
     EXPECT_TRUE(minidump_string);
+    MINIDUMP_STRING tmp = {0}; (void)tmp;
     EXPECT_EQ(file_writer.string().size() - sizeof(MINIDUMP_STRING) -
-                  sizeof(MINIDUMP_STRING::Buffer[0]),
+                  sizeof(tmp.Buffer[0]),
               minidump_string->Length);
     base::string16 output_string =
         MinidumpStringAtRVAAsString(file_writer.string(), 0);