Replace some asserts with DCHECKs

webrtc/common_audio/resampler/sinc_resampler.h

Index: webrtc/common_audio/resampler/sinc_resampler.h
diff --git a/webrtc/common_audio/resampler/sinc_resampler.h b/webrtc/common_audio/resampler/sinc_resampler.h
index ecfd64c0a647eb38a6fe09d0f23bf056698d7357..0a81723a2751991f950c37cd9383ca5d87ffd657 100644
--- a/webrtc/common_audio/resampler/sinc_resampler.h
+++ b/webrtc/common_audio/resampler/sinc_resampler.h
@@ -38,7 +38,7 @@ class SincResampler {
   // The kernel size can be adjusted for quality (higher is better) at the
   // expense of performance.  Must be a multiple of 32.
   // TODO(dalecurtis): Test performance to see if we can jack this up to 64+.
-  static const size_t kKernelSize = 32;
+  enum : size_t { kKernelSize = 32 };

[kwiberg-webrtc, 2016/11/28 05:17:48]

This is needed because RTC_DCHECK_op takes references to its arguments, I
believe. We could define it in the .cc file instead, but for pure integer
constants the enum hack feels better.

[ossu, 2016/11/28 14:54:25]

I don't get it. Why can't you take a reference to a static const? And how does
making it an enum help? That should have less storage than the static const.
What errors are you seeing?

[kwiberg-webrtc, 2016/11/28 23:01:47]

IIRC it was a linker error. I believe the template type deduction that takes
place for the RTC_DCHECK_op arguments end up using const T& for the arguments,
which the compiler implements by taking their address if they're lvalues, which
static const class members are. But enum values aren't lvalues (right?), so like
for any other rvalue, a const T& binding will involve the compiler making a
temporary on the stack and taking the address of that.

This godbolt experiment seems to agree with what I just said:
https://godbolt.org/g/BcYr84
Interestingly, there's less code for the static constexpr case than for the enum
case precisely because the compiler doesn't have to generate the code to push
the rvalue onto the stack, so maybe we should in general prefer to use static
constexpr members instead of the enum hack even though it forces us to write a
definition in the .cc file? If so, I should probably do that in this CL.

[ossu, 2016/11/29 10:23:48]

Ah, I see! So the problem is that kKernelSize doesn't have storage, but for most
cases this is fine because the compiler resolves its value statically. Adding
the line:
  const size_t SincResampler::kKernelSize;
in sinc_resampler.cc should resolve that issue.

Not sure if making it constexpr would solve it as well. Probably not:
regardless, the thing needs a place in memory if we're to take a reference to
it.

[kwiberg-webrtc, 2016/11/29 10:31:15]

Yes, exactly. (And note that the godbolt example above uses static constexpr,
confirming that it too has this problem.)

I'll change this CL to use that strategy instead.

 
   // Default request size.  Affects how often and for how much SincResampler
   // calls back for input.  Must be greater than kKernelSize.