Remove always-true DCHECK entries that fail build when dcheck_always_on=false.

Remove always-true DCHECK entries that fail build when dcheck_always_on=false.

Our bots always build with dcheck_always_on=true until http://crbug.com/652197
is fixed. This allows compile errors like this to sneak in:
error: comparison of unsigned expression >= 0 is always true.

BUG=webrtc:6517
NOTRY=True

[kjellander_webrtc, 2016-10-26 12:43:36]

Description was changed from

==========
Remove always-true DCHECK entries that fail build when dcheck_always_on=false.

Our bots always build with dcheck_always_on=true until crbug.com/652197
is fixed. This allows compile errors like this to sneak in:
error: comparison of unsigned expression >= 0 is always true.

BUG=webrtc:6517
NOTRY=True
==========

to

==========
Remove always-true DCHECK entries that fail build when dcheck_always_on=false.

Our bots always build with dcheck_always_on=true until http://crbug.com/652197
is fixed. This allows compile errors like this to sneak in:
error: comparison of unsigned expression >= 0 is always true.

BUG=webrtc:6517
NOTRY=True
==========

[kjellander_webrtc, 2016-10-26 12:44:05]

kjellander@webrtc.org changed reviewers:
+ henrik.lundin@webrtc.org

[kwiberg-webrtc, 2016-10-26 13:40:45]

kwiberg@webrtc.org changed reviewers:
+ kwiberg@webrtc.org

[kwiberg-webrtc, 2016-10-26 13:40:46]

IMO, the problem is that the DCHECK macros avoid the warning only in checking
mode. It would be better to make the non-checking mode do that too. Because
checking that an index is >= 0 isn't an unreasonable thing to do...

I can't seem to reproduce the error just by building with is_debug=false
dcheck_always_on=false, though. Do I need to do something else?

[kjellander_webrtc, 2016-10-26 13:51:11]

On 2016/10/26 13:40:46, kwiberg-webrtc wrote:
> IMO, the problem is that the DCHECK macros avoid the warning only in checking
> mode. It would be better to make the non-checking mode do that too. Because
> checking that an index is >= 0 isn't an unreasonable thing to do...

If the variable checked is of type size_t, i.e. an unsigned integer type, it
will always be >=0, right? Isn't that what the warning is about (and that all
warnings are errors for us)?

> I can't seem to reproduce the error just by building with is_debug=false
> dcheck_always_on=false, though. Do I need to do something else?

I think you need to build for Android so it'll use GCC instead. That's how I
reproed.

[hlundin-webrtc, 2016-10-27 10:49:44]

lgtm

[kwiberg-webrtc, 2016-10-27 11:05:47]

On 2016/10/27 10:49:44, hlundin-webrtc wrote:
> lgtm

I'd really like to solve this by fixing the DCHECK macros instead. Here's an
attempt to do so: https://codereview.webrtc.org/2455943002/

Because I don't think it's useful for the compiler to force us to remove DCHECKs
just because their condition can be verified statically. That causes us to
remove the check, and once that's happened, future code changes are free to
violate the removed check and no one will know...

[kjellander_webrtc, 2016-10-27 19:57:50]

On 2016/10/27 11:05:47, kwiberg-webrtc wrote:
> On 2016/10/27 10:49:44, hlundin-webrtc wrote:
> > lgtm
> 
> I'd really like to solve this by fixing the DCHECK macros instead. Here's an
> attempt to do so: https://codereview.webrtc.org/2455943002/
> 
> Because I don't think it's useful for the compiler to force us to remove
DCHECKs
> just because their condition can be verified statically. That causes us to
> remove the check, and once that's happened, future code changes are free to
> violate the removed check and no one will know...

Awesome. I'll be happy to go with your proper fix instead.

[kjellander_webrtc, 2016-10-28 05:10:50]

On 2016/10/27 19:57:50, kjellander_webrtc wrote:
> On 2016/10/27 11:05:47, kwiberg-webrtc wrote:
> > On 2016/10/27 10:49:44, hlundin-webrtc wrote:
> > > lgtm
> > 
> > I'd really like to solve this by fixing the DCHECK macros instead. Here's an
> > attempt to do so: https://codereview.webrtc.org/2455943002/
> > 
> > Because I don't think it's useful for the compiler to force us to remove
> DCHECKs
> > just because their condition can be verified statically. That causes us to
> > remove the check, and once that's happened, future code changes are free to
> > violate the removed check and no one will know...
> 
> Awesome. I'll be happy to go with your proper fix instead.

Discarding this in favor for https://codereview.webrtc.org/2455943002/ which has
now landed.