Chromium Code Reviews
[email protected] (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(184)

Issue 7493029: Add -Wsign-compare for C++ compilation on linux & friends. (Closed)

Created:
9 years, 5 months ago by Ami GONE FROM CHROMIUM
Modified:
9 years, 5 months ago
Reviewers:
Nico, Evan Martin
CC:
chromium-reviews, fischman+watch_chromium.org, pam+watch_chromium.org, ukai+watch_chromium.org
Visibility:
Public.

Description

Add -Wsign-compare for C++ compilation on linux & friends. g++ -Wall includes -Wsign-compare on C++ source, but clang++ doesn't. (this is a 2nd attempt to land this change, after 79846 had to be rolled back because clang's -Wsign-compare semantics were different from gcc's; this has been fixed in clang 135664) BUG=none TEST=chrome builds w/ rolled clang, trybots Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=93837

Patch Set 1 : . #

Total comments: 1

Patch Set 2 : . #

Unified diffs Side-by-side diffs Delta from patch set Stats (+4 lines, -0 lines) Patch
M build/common.gypi View 1 1 chunk +4 lines, -0 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
Ami GONE FROM CHROMIUM
9 years, 5 months ago (2011-07-22 23:21:57 UTC) #1
Evan Martin
LGTM, I guess -- but what is the benefit? are you looking for better coverage ...
9 years, 5 months ago (2011-07-22 23:25:54 UTC) #2
Ami GONE FROM CHROMIUM
On 2011/07/22 23:25:54, Evan Martin wrote: > LGTM, I guess -- but what is the ...
9 years, 5 months ago (2011-07-22 23:29:09 UTC) #3
Nico
(other than that lgtm) http://codereview.chromium.org/7493029/diff/2001/build/common.gypi File build/common.gypi (right): http://codereview.chromium.org/7493029/diff/2001/build/common.gypi#newcode1221 build/common.gypi:1221: # TODO(fischman): remove this if ...
9 years, 5 months ago (2011-07-22 23:43:58 UTC) #4
Nico
On 2011/07/22 23:43:58, Nico wrote: > (other than that lgtm) > > http://codereview.chromium.org/7493029/diff/2001/build/common.gypi > File ...
9 years, 5 months ago (2011-07-22 23:46:51 UTC) #5
Ami GONE FROM CHROMIUM
9 years, 5 months ago (2011-07-23 00:11:59 UTC) #6
On 2011/07/22 23:46:51, Nico wrote:
> On 2011/07/22 23:43:58, Nico wrote:
> > (other than that lgtm)
> > 
> > http://codereview.chromium.org/7493029/diff/2001/build/common.gypi
> > File build/common.gypi (right):
> > 
> >
http://codereview.chromium.org/7493029/diff/2001/build/common.gypi#newcode1221
> > build/common.gypi:1221: # TODO(fischman): remove this if http://b/4187267
> > obsoletes it.
> > Please no links to internal bugs in public code. File a public shadow bug.
> 
> Ideally on the clang but tracker, I guess.

Done.

Powered by Google App Engine
This is Rietveld 408576698