Revert of Test RTC_DCHECK_IS_ON instead of checking DCHECK_ALWAYS_ON everywhere (patchset #2 id:20001 of https://codereview.webrtc.org/2384693002/ )
Reason for revert:
This CL breaks FYI bots with a compile error.
Sample error:
jingle/glue/thread_wrapper.cc -o obj/jingle/jingle_glue/thread_wrapper.o
In file included from ../../jingle/glue/thread_wrapper.cc:5:
In file included from ../../jingle/glue/thread_wrapper.h:16:
In file included from ../../base/message_loop/message_loop.h:17:
In file included from ../../base/memory/ref_counted.h:19:
../../base/logging.h:598:1: error: call to 'MakeCheckOpString' is ambiguous
DEFINE_CHECK_OP_IMPL(EQ, ==)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../base/logging.h:592:17: note: expanded from macro 'DEFINE_CHECK_OP_IMPL'
else return MakeCheckOpString(v1, v2, names); \
^~~~~~~~~~~~~~~~~
../../jingle/glue/thread_wrapper.cc:46:3: note: in instantiation of function template specialization 'logging::CheckEQImpl<rtc::Thread *, jingle_glue::JingleThreadWrapper *>' requested here
DCHECK_EQ(rtc::Thread::Current(), current());
^
../../base/logging.h:748:31: note: expanded from macro 'DCHECK_EQ'
#define DCHECK_EQ(val1, val2) DCHECK_OP(EQ, ==, val1, val2)
^
../../base/logging.h:721:18: note: expanded from macro 'DCHECK_OP'
::logging::Check##name##Impl((val1), (val2), \
^
<scratch space>:102:1: note: expanded from here
CheckEQImpl
^
../../base/logging.h:555:14: note: candidate function [with t1 = rtc::Thread *, t2 = jingle_glue::JingleThreadWrapper *]
std::string* MakeCheckOpString(const t1& v1, const t2& v2, const char* names) {
^
../../third_party/webrtc/base/checks.h:122:14: note: candidate function [with t1 = rtc::Thread *, t2 = jingle_glue::JingleThreadWrapper *]
std::string* MakeCheckOpString(const t1& v1, const t2& v2, const char* names) {
^
In file included from ../../jingle/glue/thread_wrapper.cc:5:
In file included from ../../jingle/glue/thread_wrapper.h:16:
In file included from ../../base/message_loop/message_loop.h:17:
In file included from ../../base/memory/ref_counted.h:19:
../../base/logging.h:598:1: error: call to 'MakeCheckOpString' is ambiguous
DEFINE_CHECK_OP_IMPL(EQ, ==)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../base/logging.h:592:17: note: expanded from macro 'DEFINE_CHECK_OP_IMPL'
else return MakeCheckOpString(v1, v2, names); \
^~~~~~~~~~~~~~~~~
../../jingle/glue/thread_wrapper.cc:81:3: note: in instantiation of function template specialization 'logging::CheckEQImpl<jingle_glue::JingleThreadWrapper *, jingle_glue::JingleThreadWrapper *>' requested here
DCHECK_EQ(this, JingleThreadWrapper::current());
^
../../base/logging.h:748:31: note: expanded from macro 'DCHECK_EQ'
#define DCHECK_EQ(val1, val2) DCHECK_OP(EQ, ==, val1, val2)
^
../../base/logging.h:721:18: note: expanded from macro 'DCHECK_OP'
::logging::Check##name##Impl((val1), (val2), \
^
<scratch space>:5:1: note: expanded from here
CheckEQImpl
^
../../base/logging.h:555:14: note: candidate function [with t1 = jingle_glue::JingleThreadWrapper *, t2 = jingle_glue::JingleThreadWrapper *]
std::string* MakeCheckOpString(const t1& v1, const t2& v2, const char* names) {
^
../../third_party/webrtc/base/checks.h:122:14: note: candidate function [with t1 = jingle_glue::JingleThreadWrapper *, t2 = jingle_glue::JingleThreadWrapper *]
std::string* MakeCheckOpString(const t1& v1, const t2& v2, const char* names) {
^
In file included from ../../jingle/glue/thread_wrapper.cc:5:
In file included from ../../jingle/glue/thread_wrapper.h:16:
In file included from ../../base/message_loop/message_loop.h:17:
In file included from ../../base/memory/ref_counted.h:19:
../../base/logging.h:598:1: error: call to 'MakeCheckOpString' is ambiguous
DEFINE_CHECK_OP_IMPL(EQ, ==)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../base/logging.h:592:17: note: expanded from macro 'DEFINE_CHECK_OP_IMPL'
else return MakeCheckOpString(v1, v2, names); \
^~~~~~~~~~~~~~~~~
../../jingle/glue/thread_wrapper.cc:82:3: note: in instantiation of function template specialization 'logging::CheckEQImpl<jingle_glue::JingleThreadWrapper *, rtc::Thread *>' requested here
DCHECK_EQ(this, rtc::Thread::Current());
^
../../base/logging.h:748:31: note: expanded from macro 'DCHECK_EQ'
#define DCHECK_EQ(val1, val2) DCHECK_OP(EQ, ==, val1, val2)
^
../../base/logging.h:721:18: note: expanded from macro 'DCHECK_OP'
::logging::Check##name##Impl((val1), (val2), \
^
<scratch space>:12:1: note: expanded from here
CheckEQImpl
^
../../base/logging.h:555:14: note: candidate function [with t1 = jingle_glue::JingleThreadWrapper *, t2 = rtc::Thread *]
std::string* MakeCheckOpString(const t1& v1, const t2& v2, const char* names) {
^
../../third_party/webrtc/base/checks.h:122:14: note: candidate function [with t1 = jingle_glue::JingleThreadWrapper *, t2 = rtc::Thread *]
std::string* MakeCheckOpString(const t1& v1, const t2& v2, const char* names) {
^
3 errors generated.
Original issue's description:
> Test RTC_DCHECK_IS_ON instead of checking DCHECK_ALWAYS_ON everywhere
>
> The former is always defined (by webrtc/base/checks.h) to either 0 or
> 1, whereas the latter isn't necessarily defined.
>
> NOTRY=true
> BUG=webrtc:6451
>
> Committed: https://crrev.com/ab0b929321d37669165d5795268fa10a8c97ec5b
> Cr-Commit-Position: refs/heads/master@{#14474}
TBR=ossu@webrtc.org,kwiberg@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:6451
Review-Url: https://codereview.webrtc.org/2384083004
Cr-Commit-Position: refs/heads/master@{#14480}
diff --git a/webrtc/api/statscollector.cc b/webrtc/api/statscollector.cc
index 766e949..4dd5305 100644
--- a/webrtc/api/statscollector.cc
+++ b/webrtc/api/statscollector.cc
@@ -398,7 +398,7 @@
uint32_t ssrc) {
RTC_DCHECK(pc_->session()->signaling_thread()->IsCurrent());
RTC_DCHECK(audio_track != NULL);
-#if RTC_DCHECK_IS_ON
+#if (!defined(NDEBUG) || defined(DCHECK_ALWAYS_ON))
for (const auto& track : local_audio_tracks_)
RTC_DCHECK(track.first != audio_track || track.second != ssrc);
#endif
diff --git a/webrtc/api/statstypes.cc b/webrtc/api/statstypes.cc
index 339d5bb..eb6b30c 100644
--- a/webrtc/api/statstypes.cc
+++ b/webrtc/api/statstypes.cc
@@ -276,7 +276,7 @@
case kFloat:
return value_.float_ == other.value_.float_;
case kStaticString: {
-#if RTC_DCHECK_IS_ON
+#if (!defined(NDEBUG) || defined(DCHECK_ALWAYS_ON))
if (value_.static_string_ != other.value_.static_string_) {
RTC_DCHECK(strcmp(value_.static_string_, other.value_.static_string_) !=
0)
@@ -306,7 +306,7 @@
return value_.string_->compare(value) == 0;
if (type_ != kStaticString)
return false;
-#if RTC_DCHECK_IS_ON
+#if (!defined(NDEBUG) || defined(DCHECK_ALWAYS_ON))
if (value_.static_string_ != value)
RTC_DCHECK(strcmp(value_.static_string_, value) != 0)
<< "Duplicate global?";
diff --git a/webrtc/base/buffer.h b/webrtc/base/buffer.h
index 44f941e..6ae49f7 100644
--- a/webrtc/base/buffer.h
+++ b/webrtc/base/buffer.h
@@ -349,7 +349,7 @@
// Called when *this has been moved from. Conceptually it's a no-op, but we
// can mutate the state slightly to help subsequent sanity checks catch bugs.
void OnMovedFrom() {
-#if RTC_DCHECK_IS_ON
+#ifdef NDEBUG
// Make *this consistent and empty. Shouldn't be necessary, but better safe
// than sorry.
size_ = 0;
diff --git a/webrtc/base/checks.h b/webrtc/base/checks.h
index 2d0e8af..a48971f 100644
--- a/webrtc/base/checks.h
+++ b/webrtc/base/checks.h
@@ -13,10 +13,7 @@
#include "webrtc/typedefs.h"
-// If you for some reson need to know if DCHECKs are on, test the value of
-// RTC_DCHECK_IS_ON. (Test its value, not if it's defined; it'll always be
-// defined, to either a true or a false value.)
-#if !defined(NDEBUG) || defined(DCHECK_ALWAYS_ON)
+#if (!defined(NDEBUG) || defined(DCHECK_ALWAYS_ON))
#define RTC_DCHECK_IS_ON 1
#else
#define RTC_DCHECK_IS_ON 0
@@ -93,9 +90,9 @@
? static_cast<void>(0) \
: rtc::FatalMessageVoidify() & rtc::FatalMessage("", 0).stream()
-// RTC_CHECK dies with a fatal error if condition is not true. It is *not*
-// controlled by NDEBUG or anything else, so the check will be executed
-// regardless of compilation mode.
+// RTC_CHECK dies with a fatal error if condition is not true. It is *not*
+// controlled by NDEBUG, so the check will be executed regardless of
+// compilation mode.
//
// We make sure RTC_CHECK et al. always evaluates their arguments, as
// doing RTC_CHECK(FunctionWithSideEffect()) is a common idiom.
diff --git a/webrtc/base/criticalsection.h b/webrtc/base/criticalsection.h
index a0f9a6b..bf77e2b 100644
--- a/webrtc/base/criticalsection.h
+++ b/webrtc/base/criticalsection.h
@@ -12,7 +12,6 @@
#define WEBRTC_BASE_CRITICALSECTION_H_
#include "webrtc/base/atomicops.h"
-#include "webrtc/base/checks.h"
#include "webrtc/base/constructormagic.h"
#include "webrtc/base/thread_annotations.h"
#include "webrtc/base/platform_thread_types.h"
@@ -38,7 +37,11 @@
#include <dispatch/dispatch.h>
#endif
-#define CS_DEBUG_CHECKS RTC_DCHECK_IS_ON
+#if (!defined(NDEBUG) || defined(DCHECK_ALWAYS_ON))
+#define CS_DEBUG_CHECKS 1
+#else
+#define CS_DEBUG_CHECKS 0
+#endif
#if CS_DEBUG_CHECKS
#define CS_DEBUG_CODE(x) x
diff --git a/webrtc/base/sequenced_task_checker.h b/webrtc/base/sequenced_task_checker.h
index 4df5b54..9fc3b4a 100644
--- a/webrtc/base/sequenced_task_checker.h
+++ b/webrtc/base/sequenced_task_checker.h
@@ -12,9 +12,18 @@
#define WEBRTC_BASE_SEQUENCED_TASK_CHECKER_H_
// Apart from debug builds, we also enable the sequence checker in
-// builds with RTC_DCHECK_IS_ON so that trybots and waterfall bots
+// builds with DCHECK_ALWAYS_ON so that trybots and waterfall bots
// with this define will get the same level of checking as debug bots.
-#define ENABLE_SEQUENCED_TASK_CHECKER RTC_DCHECK_IS_ON
+//
+// Note that this does not perfectly match situations where RTC_DCHECK is
+// enabled. For example a non-official release build may have
+// DCHECK_ALWAYS_ON undefined (and therefore SequencedTaskChecker would be
+// disabled) but have RTC_DCHECKs enabled at runtime.
+#if (!defined(NDEBUG) || defined(DCHECK_ALWAYS_ON))
+#define ENABLE_SEQUENCED_TASK_CHECKER 1
+#else
+#define ENABLE_SEQUENCED_TASK_CHECKER 0
+#endif
#include "webrtc/base/checks.h"
#include "webrtc/base/constructormagic.h"
diff --git a/webrtc/base/sequenced_task_checker_unittest.cc b/webrtc/base/sequenced_task_checker_unittest.cc
index 8588648..20a4c0f 100644
--- a/webrtc/base/sequenced_task_checker_unittest.cc
+++ b/webrtc/base/sequenced_task_checker_unittest.cc
@@ -198,7 +198,7 @@
EXPECT_TRUE(done_event.Wait(1000));
}
-#if RTC_DCHECK_IS_ON
+#if !defined(NDEBUG) || DCHECK_ALWAYS_ON
TEST(SequencedTaskCheckerTest, MethodNotAllowedOnDifferentThreadInDebug) {
RunMethodOnDifferentThread(false);
}
@@ -208,7 +208,7 @@
}
#endif
-#if RTC_DCHECK_IS_ON
+#if !defined(NDEBUG) || DCHECK_ALWAYS_ON
TEST(SequencedTaskCheckerTest, MethodNotAllowedOnDifferentTaskQueueInDebug) {
RunMethodOnDifferentTaskQueue(false);
}
@@ -218,7 +218,7 @@
}
#endif
-#if RTC_DCHECK_IS_ON
+#if !defined(NDEBUG) || DCHECK_ALWAYS_ON
TEST(SequencedTaskCheckerTest, DetachFromTaskQueueInDebug) {
DetachThenCallFromDifferentTaskQueue(false);
}
diff --git a/webrtc/base/stringutils.cc b/webrtc/base/stringutils.cc
index 48830e4..9580253 100644
--- a/webrtc/base/stringutils.cc
+++ b/webrtc/base/stringutils.cc
@@ -77,7 +77,7 @@
} else if (srclen >= buflen) {
srclen = buflen - 1;
}
-#if RTC_DCHECK_IS_ON
+#if !defined(NDEBUG)
// Double check that characters are not UTF-8
for (size_t pos = 0; pos < srclen; ++pos)
RTC_DCHECK_LT(static_cast<unsigned char>(source[pos]), 128);
diff --git a/webrtc/base/thread_checker.h b/webrtc/base/thread_checker.h
index 5914282..3ae6e2c 100644
--- a/webrtc/base/thread_checker.h
+++ b/webrtc/base/thread_checker.h
@@ -14,10 +14,19 @@
#define WEBRTC_BASE_THREAD_CHECKER_H_
// Apart from debug builds, we also enable the thread checker in
-// builds with RTC_DCHECK_IS_ON so that trybots and waterfall bots
+// builds with DCHECK_ALWAYS_ON so that trybots and waterfall bots
// with this define will get the same level of thread checking as
// debug bots.
-#define ENABLE_THREAD_CHECKER RTC_DCHECK_IS_ON
+//
+// Note that this does not perfectly match situations where RTC_DCHECK is
+// enabled. For example a non-official release build may have
+// DCHECK_ALWAYS_ON undefined (and therefore ThreadChecker would be
+// disabled) but have RTC_DCHECKs enabled at runtime.
+#if (!defined(NDEBUG) || defined(DCHECK_ALWAYS_ON))
+#define ENABLE_THREAD_CHECKER 1
+#else
+#define ENABLE_THREAD_CHECKER 0
+#endif
#include "webrtc/base/checks.h"
#include "webrtc/base/constructormagic.h"
diff --git a/webrtc/base/thread_checker_unittest.cc b/webrtc/base/thread_checker_unittest.cc
index dee1653..76462fd 100644
--- a/webrtc/base/thread_checker_unittest.cc
+++ b/webrtc/base/thread_checker_unittest.cc
@@ -21,7 +21,11 @@
// Duplicated from base/threading/thread_checker.h so that we can be
// good citizens there and undef the macro.
-#define ENABLE_THREAD_CHECKER RTC_DCHECK_IS_ON
+#if !defined(NDEBUG) || defined(DCHECK_ALWAYS_ON)
+#define ENABLE_THREAD_CHECKER 1
+#else
+#define ENABLE_THREAD_CHECKER 0
+#endif
namespace rtc {
diff --git a/webrtc/common_audio/resampler/push_resampler.cc b/webrtc/common_audio/resampler/push_resampler.cc
index 9f329c4..b26774d 100644
--- a/webrtc/common_audio/resampler/push_resampler.cc
+++ b/webrtc/common_audio/resampler/push_resampler.cc
@@ -29,7 +29,7 @@
size_t num_channels) {
// The below checks are temporarily disabled on WEBRTC_WIN due to problems
// with clang debug builds.
-#if !defined(WEBRTC_WIN) && defined(__clang__)
+#if !defined(WEBRTC_WIN) && defined(__clang__) && !defined(NDEBUG)
RTC_DCHECK_GT(src_sample_rate_hz, 0);
RTC_DCHECK_GT(dst_sample_rate_hz, 0);
RTC_DCHECK_GT(num_channels, 0u);
@@ -46,11 +46,11 @@
// with clang debug builds.
// TODO(tommi): Re-enable when we've figured out what the problem is.
// http://crbug.com/615050
-#if !defined(WEBRTC_WIN) && defined(__clang__)
+#if !defined(WEBRTC_WIN) && defined(__clang__) && !defined(NDEBUG)
const size_t src_size_10ms = src_sample_rate * num_channels / 100;
const size_t dst_size_10ms = dst_sample_rate * num_channels / 100;
- RTC_DCHECK_EQ(src_length, src_size_10ms);
- RTC_DCHECK_GE(dst_capacity, dst_size_10ms);
+ RTC_CHECK_EQ(src_length, src_size_10ms);
+ RTC_CHECK_GE(dst_capacity, dst_size_10ms);
#endif
}
}
diff --git a/webrtc/media/sctp/sctpdataengine.cc b/webrtc/media/sctp/sctpdataengine.cc
index 0804fed..a724b2a 100644
--- a/webrtc/media/sctp/sctpdataengine.cc
+++ b/webrtc/media/sctp/sctpdataengine.cc
@@ -118,7 +118,7 @@
// Helper for logging SCTP messages.
void DebugSctpPrintf(const char* format, ...) {
-#if RTC_DCHECK_IS_ON
+#if (!defined(NDEBUG) || defined(DCHECK_ALWAYS_ON))
char s[255];
va_list ap;
va_start(ap, format);
diff --git a/webrtc/modules/audio_coding/acm2/audio_coding_module_unittest_oldapi.cc b/webrtc/modules/audio_coding/acm2/audio_coding_module_unittest_oldapi.cc
index 344bc3b..60e90f7 100644
--- a/webrtc/modules/audio_coding/acm2/audio_coding_module_unittest_oldapi.cc
+++ b/webrtc/modules/audio_coding/acm2/audio_coding_module_unittest_oldapi.cc
@@ -317,8 +317,8 @@
// with clang debug builds.
// TODO(tommi): Re-enable when we've figured out what the problem is.
// http://crbug.com/615050
-#if !defined(WEBRTC_WIN) && defined(__clang__) && RTC_DCHECK_IS_ON && \
- GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID)
+#if !defined(WEBRTC_WIN) && defined(__clang__) && !defined(NDEBUG)
+#if RTC_DCHECK_IS_ON && GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID)
TEST_F(AudioCodingModuleTestOldApi, FailOnZeroDesiredFrequency) {
AudioFrame audio_frame;
bool muted;
@@ -326,6 +326,7 @@
"dst_sample_rate_hz");
}
#endif
+#endif
// Checks that the transport callback is invoked once for each speech packet.
// Also checks that the frame type is kAudioFrameSpeech.
diff --git a/webrtc/modules/audio_device/android/opensles_player.cc b/webrtc/modules/audio_device/android/opensles_player.cc
index 9850d00..a63b8c1 100644
--- a/webrtc/modules/audio_device/android/opensles_player.cc
+++ b/webrtc/modules/audio_device/android/opensles_player.cc
@@ -141,7 +141,7 @@
RETURN_ON_ERROR((*player_)->SetPlayState(player_, SL_PLAYSTATE_STOPPED), -1);
// Clear the buffer queue to flush out any remaining data.
RETURN_ON_ERROR((*simple_buffer_queue_)->Clear(simple_buffer_queue_), -1);
-#if RTC_DCHECK_IS_ON
+#ifndef NDEBUG
// Verify that the buffer queue is in fact cleared as it should.
SLAndroidSimpleBufferQueueState buffer_queue_state;
(*simple_buffer_queue_)->GetState(simple_buffer_queue_, &buffer_queue_state);
diff --git a/webrtc/modules/utility/source/process_thread_impl.cc b/webrtc/modules/utility/source/process_thread_impl.cc
index 3332cd6..66534de 100644
--- a/webrtc/modules/utility/source/process_thread_impl.cc
+++ b/webrtc/modules/utility/source/process_thread_impl.cc
@@ -133,7 +133,7 @@
RTC_DCHECK(thread_checker_.CalledOnValidThread());
RTC_DCHECK(module);
-#if RTC_DCHECK_IS_ON
+#if (!defined(NDEBUG) || defined(DCHECK_ALWAYS_ON))
{
// Catch programmer error.
rtc::CritScope lock(&lock_);