MSVC++ fix: ALLOW_UNUSED_LOCAL variables only used in static... [crashpad/crashpad : master]

23 views
Skip to first unread message

Mark Mentovai (Gerrit)

unread,
Nov 11, 2016, 1:07:15 PM11/11/16
to Scott Graham, crashp...@chromium.org

Mark Mentovai posted comments on MSVC++ fix: ALLOW_UNUSED_LOCAL variables only used in static_assert.

View Change

Patch Set 2: TBR (still a holiday)

    To view, visit this change. To unsubscribe, visit settings.

    Gerrit-MessageType: comment
    Gerrit-Change-Id: I00414b54c04b5b7e3aa564b0c6fd49d20a47b6ea
    Gerrit-PatchSet: 2
    Gerrit-Project: crashpad/crashpad
    Gerrit-Branch: master
    Gerrit-Owner: Mark Mentovai <ma...@chromium.org>Gerrit-Reviewer: Mark Mentovai <ma...@chromium.org>Gerrit-Reviewer: Scott Graham <sco...@chromium.org>

    Gerrit-HasComments: No

    Mark Mentovai (Gerrit)

    unread,
    Nov 11, 2016, 1:07:18 PM11/11/16
    to Scott Graham, crashp...@chromium.org

    Mark Mentovai merged MSVC++ fix: ALLOW_UNUSED_LOCAL variables only used in static_assert.

    View Change

    MSVC++ fix: ALLOW_UNUSED_LOCAL variables only used in static_assert
    
    After f83530bf9a0b and 72fbc56e58d3, while compiling
    arraysize_unsafe_test.cc:
    
    …\crashpad\util\misc\arraysize_unsafe_test.cc(58) : error C2220: warning treated as error - no 'object' file generated
    …\crashpad\util\misc\arraysize_unsafe_test.cc(58) : warning C4101: 's10' : unreferenced local variable
    …\crashpad\util\misc\arraysize_unsafe_test.cc(33) : warning C4101: 'i1' : unreferenced local variable
    …\crashpad\util\misc\arraysize_unsafe_test.cc(24) : warning C4101: 'c1' : unreferenced local variable
    …\crashpad\util\misc\arraysize_unsafe_test.cc(27) : warning C4101: 'c2' : unreferenced local variable
    …\crashpad\util\misc\arraysize_unsafe_test.cc(55) : warning C4101: 's1' : unreferenced local variable
    …\crashpad\util\misc\arraysize_unsafe_test.cc(39) : warning C4101: 'i4' : unreferenced local variable
    …\crashpad\util\misc\arraysize_unsafe_test.cc(45) : warning C4101: 'l9' : unreferenced local variable
    …\crashpad\util\misc\arraysize_unsafe_test.cc(30) : warning C4101: 'c4' : unreferenced local variable
    …\crashpad\util\misc\arraysize_unsafe_test.cc(42) : warning C4101: 'l8' : unreferenced local variable
    …\crashpad\util\misc\arraysize_unsafe_test.cc(36) : warning C4101: 'i2' : unreferenced local variable
    
    The line numbers are totally out of order!
    
    I think that my error was not actually ever running “gclient runhooks”,
    so I never tested this locally on Windows as I thought I had.
    
    https://build.chromium.org/p/client.crashpad/builders/crashpad_win_x64_dbg/builds/266/steps/compile%20with%20ninja/logs/stdio
    
    TBR=sco...@chromium.org (holiday)
    
    Change-Id: I00414b54c04b5b7e3aa564b0c6fd49d20a47b6ea
    Reviewed-on: https://chromium-review.googlesource.com/410129
    Reviewed-by: Mark Mentovai <ma...@chromium.org>
    ---
    M util/misc/arraysize_unsafe_test.cc
    1 file changed, 11 insertions(+), 0 deletions(-)
    
    
    Approvals:
      Mark Mentovai: Looks good to me
    
    
    diff --git a/util/misc/arraysize_unsafe_test.cc b/util/misc/arraysize_unsafe_test.cc
    index ed2e6ab..a6660ae 100644
    --- a/util/misc/arraysize_unsafe_test.cc
    +++ b/util/misc/arraysize_unsafe_test.cc
    @@ -14,6 +14,7 @@
     
     #include "util/misc/arraysize_unsafe.h"
     
    +#include "base/compiler_specific.h"
     #include "gtest/gtest.h"
     
     namespace crashpad {
    @@ -23,27 +24,35 @@
     TEST(ArraySizeUnsafe, ArraySizeUnsafe) {
       char c1[1];
       static_assert(ARRAYSIZE_UNSAFE(c1) == 1, "c1");
    +  ALLOW_UNUSED_LOCAL(c1);
     
       char c2[2];
       static_assert(ARRAYSIZE_UNSAFE(c2) == 2, "c2");
    +  ALLOW_UNUSED_LOCAL(c2);
     
       char c4[4];
       static_assert(ARRAYSIZE_UNSAFE(c4) == 4, "c4");
    +  ALLOW_UNUSED_LOCAL(c4);
     
       int i1[1];
       static_assert(ARRAYSIZE_UNSAFE(i1) == 1, "i1");
    +  ALLOW_UNUSED_LOCAL(i1);
     
       int i2[2];
       static_assert(ARRAYSIZE_UNSAFE(i2) == 2, "i2");
    +  ALLOW_UNUSED_LOCAL(i2);
     
       int i4[4];
       static_assert(ARRAYSIZE_UNSAFE(i4) == 4, "i4");
    +  ALLOW_UNUSED_LOCAL(i4);
     
       long l8[8];
       static_assert(ARRAYSIZE_UNSAFE(l8) == 8, "l8");
    +  ALLOW_UNUSED_LOCAL(l8);
     
       int l9[9];
       static_assert(ARRAYSIZE_UNSAFE(l9) == 9, "l9");
    +  ALLOW_UNUSED_LOCAL(l9);
     
       struct S {
         char c;
    @@ -54,9 +63,11 @@
     
       S s1[1];
       static_assert(ARRAYSIZE_UNSAFE(s1) == 1, "s1");
    +  ALLOW_UNUSED_LOCAL(s1);
     
       S s10[10];
       static_assert(ARRAYSIZE_UNSAFE(s10) == 10, "s10");
    +  ALLOW_UNUSED_LOCAL(s10);
     }
     
     }  // namespace
    

    To view, visit this change. To unsubscribe, visit settings.

    Gerrit-MessageType: merged
    Gerrit-Change-Id: I00414b54c04b5b7e3aa564b0c6fd49d20a47b6ea
    Gerrit-PatchSet: 3

    Scott Graham (Gerrit)

    unread,
    Nov 14, 2016, 12:09:08 PM11/14/16
    to Mark Mentovai, Scott Graham, crashp...@chromium.org

    Scott Graham posted comments on MSVC++ fix: ALLOW_UNUSED_LOCAL variables only used in static_assert.

    View Change

    Patch Set 3: Code-Review+1 That's an annoying warning. It seems like using it in a static_assert expression should be sufficient.

      To view, visit this change. To unsubscribe, visit settings.

      Gerrit-MessageType: comment


      Gerrit-Change-Id: I00414b54c04b5b7e3aa564b0c6fd49d20a47b6ea
      Gerrit-PatchSet: 3
      Gerrit-Project: crashpad/crashpad
      Gerrit-Branch: master
      Gerrit-Owner: Mark Mentovai <ma...@chromium.org>Gerrit-Reviewer: Mark Mentovai <ma...@chromium.org>Gerrit-Reviewer: Scott Graham <sco...@chromium.org>

      Gerrit-HasComments: No

      Mark Mentovai (Gerrit)

      unread,
      Nov 14, 2016, 12:11:11 PM11/14/16
      to Scott Graham, crashp...@chromium.org

      Mark Mentovai posted comments on MSVC++ fix: ALLOW_UNUSED_LOCAL variables only used in static_assert.

      View Change

      Patch Set 3: A bug report is probably in order on this one.
      Reply all
      Reply to author
      Forward
      0 new messages