Clang for win should have [-Werror,-Wsign-compare] |
|||||
Issue descriptionhttps://chromium-review.googlesource.com/c/chromium/src/+/1149763 unsigned foo; int bar; DCHECK_GE(foo, bar); Expected: ../../base/logging.h:782:26: error: comparison of integers of different signs: 'const unsigned int' and 'const int' [-Werror,-Wsign-compare] DEFINE_CHECK_OP_IMPL(GE, >=) Actual: No warning. On mac and linux builder, it warns but no warning on win. It force me double check on bot. It is frustrating.
,
Jul 27
Let me check which library adds the flag to compile of third_party/blink.
,
Jul 27
Come from https://cs.chromium.org/chromium/src/build/config/compiler/BUILD.gn?l=1304&rcl=7fbb6c196de27b6a14579479f30a53791e7634e8 let me try to remove.
,
Jul 27
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/119f93e7c9cf11f2ce4bd5cba318105ebb7db954 commit 119f93e7c9cf11f2ce4bd5cba318105ebb7db954 Author: Takuto Ikuta <tikuta@chromium.org> Date: Fri Jul 27 17:25:50 2018 Fix to remove /wd4018 (-Wno-sign-compare) for /device/vr This is a part of effort to remove /wd4018 warning suppression. Master CL is https://chromium-review.googlesource.com/c/chromium/src/+/1152755 This CL was uploaded by git cl split. R=bajones@chromium.org Bug: 868248 , 588506 Cq-Include-Trybots: luci.chromium.try:win_optional_gpu_tests_rel Change-Id: I980cd92d9e69cd3914e2066f159a786959823ba2 Reviewed-on: https://chromium-review.googlesource.com/1152839 Reviewed-by: Brandon Jones <bajones@chromium.org> Commit-Queue: Takuto Ikuta <tikuta@chromium.org> Cr-Commit-Position: refs/heads/master@{#578691} [modify] https://crrev.com/119f93e7c9cf11f2ce4bd5cba318105ebb7db954/device/vr/openvr/openvr_render_loop.cc
,
Jul 27
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b6ae83fd01b733dcdfe3d2844512fd8a40393209 commit b6ae83fd01b733dcdfe3d2844512fd8a40393209 Author: Takuto Ikuta <tikuta@chromium.org> Date: Fri Jul 27 23:07:15 2018 Fix to remove /wd4018 (-Wno-sign-compare) for /chrome_elf This is a part of effort to remove /wd4018 warning suppression. Master CL is https://chromium-review.googlesource.com/c/chromium/src/+/1152755 This CL was uploaded by git cl split. R=pennymac@chromium.org Bug: 868248 , 588506 Change-Id: Iba8b4516550b89b5acf534bc1fa11d959154550e Reviewed-on: https://chromium-review.googlesource.com/1152841 Reviewed-by: Penny MacNeil <pennymac@chromium.org> Commit-Queue: Takuto Ikuta <tikuta@chromium.org> Cr-Commit-Position: refs/heads/master@{#578846} [modify] https://crrev.com/b6ae83fd01b733dcdfe3d2844512fd8a40393209/chrome_elf/third_party_dlls/main_unittest.cc
,
Jul 27
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/beea8ba5631bdc2a9d8d6587a13b80b390e4658c commit beea8ba5631bdc2a9d8d6587a13b80b390e4658c Author: Takuto Ikuta <tikuta@chromium.org> Date: Fri Jul 27 23:08:26 2018 Fix to remove /wd4018 (-Wno-sign-compare) for /services/service_manager/sandbox/win This is a part of effort to remove /wd4018 warning suppression. Master CL is https://chromium-review.googlesource.com/c/chromium/src/+/1152755 This CL was uploaded by git cl split. R=forshaw@chromium.org Bug: 868248 , 588506 Change-Id: I6d0a6763d4afbe3fd51f9841b19adbe70209c68d Reviewed-on: https://chromium-review.googlesource.com/1152842 Reviewed-by: Penny MacNeil <pennymac@chromium.org> Commit-Queue: Takuto Ikuta <tikuta@chromium.org> Cr-Commit-Position: refs/heads/master@{#578848} [modify] https://crrev.com/beea8ba5631bdc2a9d8d6587a13b80b390e4658c/services/service_manager/sandbox/win/sandbox_win.cc
,
Jul 28
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/311d9e7b7fe4ec95cb964ca957269210c63c3360 commit 311d9e7b7fe4ec95cb964ca957269210c63c3360 Author: Takuto Ikuta <tikuta@chromium.org> Date: Sat Jul 28 02:32:21 2018 Fix to remove /wd4018 (-Wno-sign-compare) for /device/bluetooth This is a part of effort to remove /wd4018 warning suppression. Master CL is https://chromium-review.googlesource.com/c/chromium/src/+/1152755 This CL was uploaded by git cl split. R=ortuno@chromium.org Bug: 868248 , 588506 Change-Id: I6f381e608322431356681f57a315271dca409d83 Reviewed-on: https://chromium-review.googlesource.com/1152843 Reviewed-by: Reilly Grant <reillyg@chromium.org> Commit-Queue: Takuto Ikuta <tikuta@chromium.org> Cr-Commit-Position: refs/heads/master@{#578899} [modify] https://crrev.com/311d9e7b7fe4ec95cb964ca957269210c63c3360/device/bluetooth/bluetooth_uuid_unittest.cc
,
Jul 30
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/07ffa1bda3f0449943fd45cb84e80854425bdcf4 commit 07ffa1bda3f0449943fd45cb84e80854425bdcf4 Author: Takuto Ikuta <tikuta@chromium.org> Date: Mon Jul 30 22:17:20 2018 Fix to remove /wd4018 (-Wno-sign-compare) for /ui/views This is a part of effort to remove /wd4018 warning suppression. Master CL is https://chromium-review.googlesource.com/c/chromium/src/+/1152755 This CL was uploaded by git cl split. R=sky@chromium.org Bug: 868248 , 588506 Change-Id: Ibeb235f73e433aa9bc34dbdb7513582cf11427c8 Reviewed-on: https://chromium-review.googlesource.com/1152844 Reviewed-by: Scott Violet <sky@chromium.org> Commit-Queue: Takuto Ikuta <tikuta@chromium.org> Cr-Commit-Position: refs/heads/master@{#579196} [modify] https://crrev.com/07ffa1bda3f0449943fd45cb84e80854425bdcf4/ui/views/win/pen_event_processor.cc [modify] https://crrev.com/07ffa1bda3f0449943fd45cb84e80854425bdcf4/ui/views/win/pen_event_processor.h
,
Jul 31
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1f75f99ff2f94234223b089297b66ecd791906ee commit 1f75f99ff2f94234223b089297b66ecd791906ee Author: Takuto Ikuta <tikuta@chromium.org> Date: Tue Jul 31 04:12:42 2018 Fix to remove /wd4018 (-Wno-sign-compare) for /build/config This is a part of effort to remove /wd4018 warning suppression. Master CL is https://chromium-review.googlesource.com/c/chromium/src/+/1152755 This CL was uploaded by git cl split. R=dpranke@chromium.org Bug: 868248 , 588506 Change-Id: I3c31dc8f72266eda85904c4a860e156fadb2a76e Reviewed-on: https://chromium-review.googlesource.com/1152840 Commit-Queue: Nico Weber <thakis@chromium.org> Reviewed-by: Nico Weber <thakis@chromium.org> Reviewed-by: Dirk Pranke <dpranke@chromium.org> Cr-Commit-Position: refs/heads/master@{#579311} [modify] https://crrev.com/1f75f99ff2f94234223b089297b66ecd791906ee/build/config/compiler/BUILD.gn [modify] https://crrev.com/1f75f99ff2f94234223b089297b66ecd791906ee/chrome/browser/conflicts/third_party_metrics_recorder_win.h
,
Jul 31
Removed the warning.
,
Jul 31
Thanks much for diagnosing and fixing!
,
Jul 31
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/91629b37e25446a5c0b7d6195f70c8d2b29183e6 commit 91629b37e25446a5c0b7d6195f70c8d2b29183e6 Author: Patti <patricialor@chromium.org> Date: Tue Jul 31 04:46:27 2018 Revert "Fix to remove /wd4018 (-Wno-sign-compare) for /build/config" This reverts commit 1f75f99ff2f94234223b089297b66ecd791906ee. Reason for revert: This breaks compile on "Google Chrome Win" builder due to a integer sign mismatch in incompatible_applications_updater_win.cc(289,32). Original change's description: > Fix to remove /wd4018 (-Wno-sign-compare) for /build/config > > This is a part of effort to remove /wd4018 warning suppression. > > Master CL is > https://chromium-review.googlesource.com/c/chromium/src/+/1152755 > > This CL was uploaded by git cl split. > > R=dpranke@chromium.org > > Bug: 868248 , 588506 > Change-Id: I3c31dc8f72266eda85904c4a860e156fadb2a76e > Reviewed-on: https://chromium-review.googlesource.com/1152840 > Commit-Queue: Nico Weber <thakis@chromium.org> > Reviewed-by: Nico Weber <thakis@chromium.org> > Reviewed-by: Dirk Pranke <dpranke@chromium.org> > Cr-Commit-Position: refs/heads/master@{#579311} TBR=thakis@chromium.org,dpranke@chromium.org,pmonette@chromium.org,tikuta@chromium.org Change-Id: Iaed7b5e6f2ff3c53e1846162ca8d14689f7ae5e1 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 868248 , 588506 Reviewed-on: https://chromium-review.googlesource.com/1156144 Reviewed-by: Patti <patricialor@chromium.org> Commit-Queue: Patti <patricialor@chromium.org> Cr-Commit-Position: refs/heads/master@{#579312} [modify] https://crrev.com/91629b37e25446a5c0b7d6195f70c8d2b29183e6/build/config/compiler/BUILD.gn [modify] https://crrev.com/91629b37e25446a5c0b7d6195f70c8d2b29183e6/chrome/browser/conflicts/third_party_metrics_recorder_win.h
,
Jul 31
Hmm, the CL is reverted. I need to do few more works.
,
Jul 31
,
Jul 31
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b166ec24eb53eb20a10b0c00616ae8e569bba97f commit b166ec24eb53eb20a10b0c00616ae8e569bba97f Author: Takuto Ikuta <tikuta@chromium.org> Date: Tue Jul 31 14:39:14 2018 Reland "Fix to remove /wd4018 (-Wno-sign-compare) for /build/config" This is a reland of 1f75f99ff2f94234223b089297b66ecd791906ee Original change's description: > Fix to remove /wd4018 (-Wno-sign-compare) for /build/config > > This is a part of effort to remove /wd4018 warning suppression. > > Master CL is > https://chromium-review.googlesource.com/c/chromium/src/+/1152755 > > This CL was uploaded by git cl split. > > R=dpranke@chromium.org > > Bug: 868248 , 588506 > Change-Id: I3c31dc8f72266eda85904c4a860e156fadb2a76e > Reviewed-on: https://chromium-review.googlesource.com/1152840 > Commit-Queue: Nico Weber <thakis@chromium.org> > Reviewed-by: Nico Weber <thakis@chromium.org> > Reviewed-by: Dirk Pranke <dpranke@chromium.org> > Cr-Commit-Position: refs/heads/master@{#579311} Bug: 868248 , 588506 Change-Id: Ic673677dfc0800b361d97f6516ee61887635d9d4 Reviewed-on: https://chromium-review.googlesource.com/1156164 Commit-Queue: Nico Weber <thakis@chromium.org> Reviewed-by: Nico Weber <thakis@chromium.org> Cr-Commit-Position: refs/heads/master@{#579401} [modify] https://crrev.com/b166ec24eb53eb20a10b0c00616ae8e569bba97f/build/config/compiler/BUILD.gn [modify] https://crrev.com/b166ec24eb53eb20a10b0c00616ae8e569bba97f/chrome/browser/conflicts/module_info_win.h [modify] https://crrev.com/b166ec24eb53eb20a10b0c00616ae8e569bba97f/chrome/browser/conflicts/third_party_metrics_recorder_win.h
,
Jul 31
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e86a32b2eaf4031e1adfb9cd1bdf13f590f73baf commit e86a32b2eaf4031e1adfb9cd1bdf13f590f73baf Author: Dave Tapuska <dtapuska@chromium.org> Date: Tue Jul 31 15:23:57 2018 Revert "Reland "Fix to remove /wd4018 (-Wno-sign-compare) for /build/config"" This reverts commit b166ec24eb53eb20a10b0c00616ae8e569bba97f. Reason for revert: Build failure on https://ci.chromium.org/buildbot/chromium.chrome/Google%20Chrome%20Win/34655 Original change's description: > Reland "Fix to remove /wd4018 (-Wno-sign-compare) for /build/config" > > This is a reland of 1f75f99ff2f94234223b089297b66ecd791906ee > > Original change's description: > > Fix to remove /wd4018 (-Wno-sign-compare) for /build/config > > > > This is a part of effort to remove /wd4018 warning suppression. > > > > Master CL is > > https://chromium-review.googlesource.com/c/chromium/src/+/1152755 > > > > This CL was uploaded by git cl split. > > > > R=dpranke@chromium.org > > > > Bug: 868248 , 588506 > > Change-Id: I3c31dc8f72266eda85904c4a860e156fadb2a76e > > Reviewed-on: https://chromium-review.googlesource.com/1152840 > > Commit-Queue: Nico Weber <thakis@chromium.org> > > Reviewed-by: Nico Weber <thakis@chromium.org> > > Reviewed-by: Dirk Pranke <dpranke@chromium.org> > > Cr-Commit-Position: refs/heads/master@{#579311} > > Bug: 868248 , 588506 > Change-Id: Ic673677dfc0800b361d97f6516ee61887635d9d4 > Reviewed-on: https://chromium-review.googlesource.com/1156164 > Commit-Queue: Nico Weber <thakis@chromium.org> > Reviewed-by: Nico Weber <thakis@chromium.org> > Cr-Commit-Position: refs/heads/master@{#579401} TBR=thakis@chromium.org,dpranke@chromium.org,pmonette@chromium.org,tikuta@chromium.org Change-Id: I4cab2a6abe6cb7b3f953f0ced3f8368f5c97fac9 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 868248 , 588506 Reviewed-on: https://chromium-review.googlesource.com/1156764 Reviewed-by: Dave Tapuska <dtapuska@chromium.org> Commit-Queue: Dave Tapuska <dtapuska@chromium.org> Cr-Commit-Position: refs/heads/master@{#579412} [modify] https://crrev.com/e86a32b2eaf4031e1adfb9cd1bdf13f590f73baf/build/config/compiler/BUILD.gn [modify] https://crrev.com/e86a32b2eaf4031e1adfb9cd1bdf13f590f73baf/chrome/browser/conflicts/module_info_win.h [modify] https://crrev.com/e86a32b2eaf4031e1adfb9cd1bdf13f590f73baf/chrome/browser/conflicts/third_party_metrics_recorder_win.h
,
Aug 1
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/90edfb350bbf34314c6e44650118345b9d6fd105 commit 90edfb350bbf34314c6e44650118345b9d6fd105 Author: Takuto Ikuta <tikuta@chromium.org> Date: Wed Aug 01 01:07:42 2018 Reland "Reland "Fix to remove /wd4018 (-Wno-sign-compare) for /build/config"" This is a reland of b166ec24eb53eb20a10b0c00616ae8e569bba97f I dropped the change for build flags from this. Original change's description: > Reland "Fix to remove /wd4018 (-Wno-sign-compare) for /build/config" > > This is a reland of 1f75f99ff2f94234223b089297b66ecd791906ee > > Original change's description: > > Fix to remove /wd4018 (-Wno-sign-compare) for /build/config > > > > This is a part of effort to remove /wd4018 warning suppression. > > > > Master CL is > > https://chromium-review.googlesource.com/c/chromium/src/+/1152755 > > > > This CL was uploaded by git cl split. > > > > R=dpranke@chromium.org > > > > Bug: 868248 , 588506 > > Change-Id: I3c31dc8f72266eda85904c4a860e156fadb2a76e > > Reviewed-on: https://chromium-review.googlesource.com/1152840 > > Commit-Queue: Nico Weber <thakis@chromium.org> > > Reviewed-by: Nico Weber <thakis@chromium.org> > > Reviewed-by: Dirk Pranke <dpranke@chromium.org> > > Cr-Commit-Position: refs/heads/master@{#579311} > > Bug: 868248 , 588506 > Change-Id: Ic673677dfc0800b361d97f6516ee61887635d9d4 > Reviewed-on: https://chromium-review.googlesource.com/1156164 > Commit-Queue: Nico Weber <thakis@chromium.org> > Reviewed-by: Nico Weber <thakis@chromium.org> > Cr-Commit-Position: refs/heads/master@{#579401} Bug: 868248 , 588506 Change-Id: I41148bf02e6f5d1998b88f38e26022888a3b8623 Reviewed-on: https://chromium-review.googlesource.com/1157664 Commit-Queue: Nico Weber <thakis@chromium.org> Reviewed-by: Nico Weber <thakis@chromium.org> Cr-Commit-Position: refs/heads/master@{#579650} [modify] https://crrev.com/90edfb350bbf34314c6e44650118345b9d6fd105/chrome/browser/conflicts/module_info_win.h [modify] https://crrev.com/90edfb350bbf34314c6e44650118345b9d6fd105/chrome/browser/conflicts/third_party_metrics_recorder_win.h
,
Aug 1
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7721d7fa8cca6e1006137ca5de74c9899aa8ebf3 commit 7721d7fa8cca6e1006137ca5de74c9899aa8ebf3 Author: Takuto Ikuta <tikuta@chromium.org> Date: Wed Aug 01 11:36:22 2018 Remove wd4018 flags I confirmed this can be built with is_chrome_branded=true and is_official_build=true. Bug: 868248 Cq-Include-Trybots: master.tryserver.chromium.win:win_chrome_official Change-Id: I6bfbd22c89dd4d3d655fce0db10448afb6cb2103 Reviewed-on: https://chromium-review.googlesource.com/1157946 Commit-Queue: Nico Weber <thakis@chromium.org> Reviewed-by: Nico Weber <thakis@chromium.org> Cr-Commit-Position: refs/heads/master@{#579762} [modify] https://crrev.com/7721d7fa8cca6e1006137ca5de74c9899aa8ebf3/build/config/compiler/BUILD.gn [modify] https://crrev.com/7721d7fa8cca6e1006137ca5de74c9899aa8ebf3/chrome/browser/conflicts/third_party_blocking_browsertest.cc
,
Aug 1
Flag removal did not break official builder this time. https://ci.chromium.org/buildbot/chromium.chrome/Google%20Chrome%20Win/34719 |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by tikuta@chromium.org
, Jul 27