Skip to content

Add macro DCHECK in quic_logging_impl.h #6358

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
alyssawilk merged 10 commits into from
Mar 25, 2019
Merged

Add macro DCHECK in quic_logging_impl.h #6358

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
ac872f9
Update quiche dependency.
danzh1989 Mar 20, 2019
717885d
fix use of QuicString
danzh1989 Mar 20, 2019
831bd35
remove logging_impl
danzh1989 Mar 20, 2019
4d4c80d
fix format
danzh1989 Mar 20, 2019
480f8fb
add back quic::
danzh1989 Mar 21, 2019
f3127a2
change to logging.h
danzh1989 Mar 21, 2019
ca4b1d5
implement DCHECK
danzh1989 Mar 21, 2019
c38586b
Merge branch 'master' into dcheck
danzh1989 Mar 22, 2019
0776a2d
move around macro definition
danzh1989 Mar 22, 2019
d5023e3
cosmatic change
danzh1989 Mar 22, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions source/extensions/quic_listeners/quiche/platform/quic_logging_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@
#include "absl/base/optimization.h"
#include "absl/synchronization/mutex.h"

// TODO(wub): Add CHECK/DCHECK and variants, which are not explicitly exposed by quic_logging.h.

// If |condition| is true, use |logstream| to stream the log message and send it to spdlog.
// If |condition| is false, |logstream| will not be instantiated.
// The switch(0) is used to suppress a compiler warning on ambiguous "else".
Expand Down Expand Up @@ -55,8 +53,12 @@
#define QUIC_LOG_WARNING_IS_ON_IMPL() quic::IsLogLevelEnabled(quic::WARNING)
#define QUIC_LOG_ERROR_IS_ON_IMPL() quic::IsLogLevelEnabled(quic::ERROR)

#define CHECK(condition) \
QUIC_LOG_IF_IMPL(FATAL, ABSL_PREDICT_FALSE(!(condition))) << "CHECK failed: " #condition "."

#ifdef NDEBUG
// Release build
#define DCHECK(condition) QUIC_COMPILED_OUT_LOG()
#define QUIC_COMPILED_OUT_LOG() QUIC_LOG_IMPL_INTERNAL(false, quic::NullLogStream().stream())
#define QUIC_DVLOG_IMPL(verbosity) QUIC_COMPILED_OUT_LOG()
#define QUIC_DVLOG_IF_IMPL(verbosity, condition) QUIC_COMPILED_OUT_LOG()
Expand All @@ -66,6 +68,7 @@
#define QUIC_NOTREACHED_IMPL()
#else
// Debug build
#define DCHECK(condition) CHECK(condition)
#define QUIC_DVLOG_IMPL(verbosity) QUIC_VLOG_IMPL(verbosity)
#define QUIC_DVLOG_IF_IMPL(verbosity, condition) QUIC_VLOG_IF_IMPL(verbosity, condition)
#define QUIC_DLOG_IMPL(severity) QUIC_LOG_IMPL(severity)
Expand Down
13 changes: 13 additions & 0 deletions test/extensions/quic_listeners/quiche/platform/quic_platform_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,19 @@ TEST(QuicPlatformTest, QuicDLog) {

#undef VALUE_BY_COMPILE_MODE

TEST(QuicPlatformTest, QuicCHECK) {
CHECK(1 == 1);
CHECK(1 == 1) << " 1 == 1 is forever true.";

EXPECT_DEBUG_DEATH({ DCHECK(false) << " Supposed to fail in debug mode."; },
Copy link
Copy Markdown
Contributor

@wu-bin wu-bin Mar 22, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit, feel free to ignore: It'd be nicer visually if you

  • Put the first 2 CHECKs in adjacent lines
  • Then followed by an empty line
  • Then put the 2 EXPECT_DEBUG_DEATHs in adjacent lines
  • Then followed by an empty line
  • Then put the 2 EXPECT_DEATHs in adjacent lines

Copy link
Copy Markdown
Contributor Author

@danzh2010 danzh2010 Mar 22, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

"CHECK failed:.* Supposed to fail in debug mode.");
EXPECT_DEBUG_DEATH({ DCHECK(false); }, "CHECK failed");

EXPECT_DEATH({ CHECK(false) << " Supposed to fail in all modes."; },
"CHECK failed:.* Supposed to fail in all modes.");
EXPECT_DEATH({ CHECK(false); }, "CHECK failed");
}

// Test the behaviors of the cross products of
//
// {QUIC_LOG, QUIC_DLOG} x {FATAL, DFATAL} x {debug, release}
Expand Down