RTC_CHECK and RTC_DCHECK macros for C

RTC_CHECK and RTC_DCHECK macros for C

So that we don't have to use assert(). Includes one sample call site.

NOTRY=true
BUG=chromium:617124
Committed: https://crrev.com/2e486462e0f79a1ba436fa23495a9ac992fa4d53
Cr-Commit-Position: refs/heads/master@{#13862}

[kwiberg-webrtc, 2016-08-22 11:58:33]

kwiberg@webrtc.org changed reviewers:
+ henrik.lundin@webrtc.org, ossu@webrtc.org, peah@webrtc.org

[kwiberg-webrtc, 2016-08-22 11:58:33]

Oskar: General C/C++ review

Per+Henrik: Does this look like something we want for our C files? It'd mean one
less difference between C and C++ source files (because if I land this, I'll
follow up with a CL that replaces all uses of assert()).

[kwiberg-webrtc, 2016-08-22 12:33:50]

Patchset #2 (id:20001) has been deleted

[kwiberg-webrtc, 2016-08-22 12:39:25]

Forgot to mention the reason I built this: I needed to do this in a couple of
places:

  int success = Foo(...);
  assert(success);

But that causes the compiler to complain about |success| being unused in
non-debug builds. We have a macro for that (ATTRIBUTE_UNUSED), but it's not
currently used anywhere, and it turns out to not actually suppress the warning
on Windows. So I had the choice of doing some ugly hack to confuse the compiler
into thinking the variable wasn't unused after all, or simply porting CHECK and
DCHECK to C.

[ossu, 2016-08-22 13:13:36]

Implementation looks good, but why are we compiling things part of WebRTC as
pure C? Wouldn't it be better/easier just to make those files .cc files?

https://codereview.webrtc.org/2262173002/diff/1/webrtc/base/checks.h
File webrtc/base/checks.h (right):

https://codereview.webrtc.org/2262173002/diff/1/webrtc/base/checks.h#newcode246
webrtc/base/checks.h:246: // C version. Lacks many features compared to the C++
version, but usage
Should it not have them? Like making it printf-like:
RTC_CHECK(monkey_ptr || monkeys == 0, "Expected there to be a monkey; number of
monkeys left: %d", monkeys);

https://codereview.webrtc.org/2262173002/diff/1/webrtc/base/checks.h#newcode263
webrtc/base/checks.h:263: #define RTC_DCHECK(condition)                         
                   \
So changing an RTC_CHECK to RTC_DCHECK causes the condition to not be evaluated
in non-debug builds? Good to know! :)

[kwiberg-webrtc, 2016-08-22 13:20:44]

https://codereview.webrtc.org/2262173002/diff/1/webrtc/base/checks.h
File webrtc/base/checks.h (right):

https://codereview.webrtc.org/2262173002/diff/1/webrtc/base/checks.h#newcode246
webrtc/base/checks.h:246: // C version. Lacks many features compared to the C++
version, but usage
On 2016/08/22 13:13:36, ossu wrote:
> Should it not have them? Like making it printf-like:
> RTC_CHECK(monkey_ptr || monkeys == 0, "Expected there to be a monkey; number
of
> monkeys left: %d", monkeys);

In principle, yes. Given the current number of assert() calls in .c files and us
planning to do ~all future development in C++, no.

https://codereview.webrtc.org/2262173002/diff/1/webrtc/base/checks.h#newcode263
webrtc/base/checks.h:263: #define RTC_DCHECK(condition)                         
                   \
On 2016/08/22 13:13:36, ossu wrote:
> So changing an RTC_CHECK to RTC_DCHECK causes the condition to not be
evaluated
> in non-debug builds? Good to know! :)

Yes. The C implementation mirrors the C++ implementation in that case. (And it
is the indended, documented behavior.)

The only reason we don't ifdef the condition away altogether in non-debug builds
is that that would make the compiler complain about variables only used in the
condition being unused.

[kwiberg-webrtc, 2016-08-22 13:22:42]

On 2016/08/22 13:13:36, ossu wrote:
> Implementation looks good, but why are we compiling things part of WebRTC as
> pure C? Wouldn't it be better/easier just to make those files .cc files?

In principle yes, but there are a lot of them. Thus far we've been content with
converting .c to .cc before starting major changes, and leaving the rest in
place.

[hlundin-webrtc, 2016-08-22 14:42:04]

lgtm

[ossu, 2016-08-23 08:59:43]

On 2016/08/22 13:22:42, kwiberg-webrtc wrote:
> On 2016/08/22 13:13:36, ossu wrote:
> > Implementation looks good, but why are we compiling things part of WebRTC as
> > pure C? Wouldn't it be better/easier just to make those files .cc files?
> 
> In principle yes, but there are a lot of them. Thus far we've been content
with
> converting .c to .cc before starting major changes, and leaving the rest in
> place.

Alright. I guess the reason we'd want to keep stuff as pure C is if it could
(easily) be used separately - like ISAC and ILBC, maybe some signal processing
stuff?
Adding RTC_CHECK to pure C seems fairly limited an effort. I just don't want us
to end up having to support a whole set of duplicate functionality in C.
I guess I'll have to LGTM it then. :)

[kwiberg-webrtc, 2016-08-23 09:07:06]

The CQ bit was checked by kwiberg@webrtc.org

[commit-bot: I haz the power, 2016-08-23 09:07:17]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-08-23 11:07:49]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-23 11:07:50]

Try jobs failed on following builders:
  android_arm64_rel on master.tryserver.webrtc (JOB_TIMED_OUT, no build URL)

[kwiberg-webrtc, 2016-08-23 12:44:34]

Description was changed from

==========
RTC_CHECK and RTC_DCHECK macros for C

So that we don't have to use assert(). Includes one sample call site.

BUG=chromium:617124
==========

to

==========
RTC_CHECK and RTC_DCHECK macros for C

So that we don't have to use assert(). Includes one sample call site.

NOTRY=true
BUG=chromium:617124
==========

[kwiberg-webrtc, 2016-08-23 12:44:40]

The CQ bit was checked by kwiberg@webrtc.org

[commit-bot: I haz the power, 2016-08-23 12:44:51]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-08-23 12:54:29]

Description was changed from

==========
RTC_CHECK and RTC_DCHECK macros for C

So that we don't have to use assert(). Includes one sample call site.

NOTRY=true
BUG=chromium:617124
==========

to

==========
RTC_CHECK and RTC_DCHECK macros for C

So that we don't have to use assert(). Includes one sample call site.

NOTRY=true
BUG=chromium:617124
==========

[commit-bot: I haz the power, 2016-08-23 12:54:30]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2016-08-23 12:54:37]

Description was changed from

==========
RTC_CHECK and RTC_DCHECK macros for C

So that we don't have to use assert(). Includes one sample call site.

NOTRY=true
BUG=chromium:617124
==========

to

==========
RTC_CHECK and RTC_DCHECK macros for C

So that we don't have to use assert(). Includes one sample call site.

NOTRY=true
BUG=chromium:617124

Committed: https://crrev.com/2e486462e0f79a1ba436fa23495a9ac992fa4d53
Cr-Commit-Position: refs/heads/master@{#13862}
==========

[commit-bot: I haz the power, 2016-08-23 12:54:38]

Patchset 2 (id:??) landed as
https://crrev.com/2e486462e0f79a1ba436fa23495a9ac992fa4d53
Cr-Commit-Position: refs/heads/master@{#13862}