From c2b4269b77822aee6d07ae427a3341abdbaa8c3f Mon Sep 17 00:00:00 2001 From: cjihrig Date: Mon, 24 Dec 2018 12:10:35 -0500 Subject: [PATCH 1/2] src: use DCHECK_* macros where possible MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/25207 Reviewed-By: Michaël Zasso Reviewed-By: Anna Henningsen Reviewed-By: Ruben Bridgewater Reviewed-By: Tobias Nießen Reviewed-By: Luigi Pinca --- src/aliased_buffer.h | 12 ++++-------- src/base_object-inl.h | 2 +- src/debug_utils.h | 4 +--- src/string_decoder.cc | 10 +++------- src/string_search.h | 4 +--- 5 files changed, 10 insertions(+), 22 deletions(-) diff --git a/src/aliased_buffer.h b/src/aliased_buffer.h index fa610a97744840..0bae974af7a52c 100644 --- a/src/aliased_buffer.h +++ b/src/aliased_buffer.h @@ -198,9 +198,7 @@ class AliasedBuffer { * Set position index to given value. */ inline void SetValue(const size_t index, NativeT value) { -#if defined(DEBUG) && DEBUG - CHECK_LT(index, count_); -#endif + DCHECK_LT(index, count_); buffer_[index] = value; } @@ -208,9 +206,7 @@ class AliasedBuffer { * Get value at position index */ inline const NativeT GetValue(const size_t index) const { -#if defined(DEBUG) && DEBUG - CHECK_LT(index, count_); -#endif + DCHECK_LT(index, count_); return buffer_[index]; } @@ -233,9 +229,9 @@ class AliasedBuffer { // Should only be used on an owning array, not one created as a sub array of // an owning `AliasedBuffer`. void reserve(size_t new_capacity) { + DCHECK_GE(new_capacity, count_); + DCHECK_EQ(byte_offset_, 0); #if defined(DEBUG) && DEBUG - CHECK_GE(new_capacity, count_); - CHECK_EQ(byte_offset_, 0); CHECK(free_buffer_); #endif const v8::HandleScope handle_scope(isolate_); diff --git a/src/base_object-inl.h b/src/base_object-inl.h index cce872739381cf..84c449a30814e2 100644 --- a/src/base_object-inl.h +++ b/src/base_object-inl.h @@ -115,8 +115,8 @@ BaseObject::MakeLazilyInitializedJSTemplate(Environment* env) { auto constructor = [](const v8::FunctionCallbackInfo& args) { #ifdef DEBUG CHECK(args.IsConstructCall()); - CHECK_GT(args.This()->InternalFieldCount(), 0); #endif + DCHECK_GT(args.This()->InternalFieldCount(), 0); args.This()->SetAlignedPointerInInternalField(0, nullptr); }; diff --git a/src/debug_utils.h b/src/debug_utils.h index 034d8a9ab331dc..ed5e42fcbe8e6e 100644 --- a/src/debug_utils.h +++ b/src/debug_utils.h @@ -68,9 +68,7 @@ template inline void FORCE_INLINE Debug(AsyncWrap* async_wrap, const char* format, Args&&... args) { -#ifdef DEBUG - CHECK_NOT_NULL(async_wrap); -#endif + DCHECK_NOT_NULL(async_wrap); DebugCategory cat = static_cast(async_wrap->provider_type()); if (!UNLIKELY(async_wrap->env()->debug_enabled(cat))) diff --git a/src/string_decoder.cc b/src/string_decoder.cc index a83d6623b4b4bb..96c6baa4d815e4 100644 --- a/src/string_decoder.cc +++ b/src/string_decoder.cc @@ -123,11 +123,9 @@ MaybeLocal StringDecoder::DecodeData(Isolate* isolate, body = !prepend.IsEmpty() ? prepend : String::Empty(isolate); prepend = Local(); } else { -#ifdef DEBUG // If not, that means is no character left to finish at this point. - CHECK_EQ(MissingBytes(), 0); - CHECK_EQ(BufferedBytes(), 0); -#endif + DCHECK_EQ(MissingBytes(), 0); + DCHECK_EQ(BufferedBytes(), 0); // See whether there is a character that we may have to cut off and // finish when receiving the next chunk. @@ -136,9 +134,7 @@ MaybeLocal StringDecoder::DecodeData(Isolate* isolate, // This means we'll need to figure out where the character to which // the byte belongs begins. for (size_t i = nread - 1; ; --i) { -#ifdef DEBUG - CHECK_LT(i, nread); -#endif + DCHECK_LT(i, nread); state_[kBufferedBytes]++; if ((data[i] & 0xC0) == 0x80) { // This byte does not start a character (a "trailing" byte). diff --git a/src/string_search.h b/src/string_search.h index 358a4c1b024e67..7d59d89df78329 100644 --- a/src/string_search.h +++ b/src/string_search.h @@ -37,9 +37,7 @@ class Vector { // Access individual vector elements - checks bounds in debug mode. T& operator[](size_t index) const { -#ifdef DEBUG - CHECK(index < length_); -#endif + DCHECK_LT(index, length_); return start_[is_forward_ ? index : (length_ - index - 1)]; } From 4e31a7f3546bb848058a55b3aa629b5f12d1caf9 Mon Sep 17 00:00:00 2001 From: cjihrig Date: Mon, 24 Dec 2018 12:24:08 -0500 Subject: [PATCH 2/2] src: introduce DCHECK macro MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit adds a DCHECK macro for consistency with the other DCHECK_* macros. PR-URL: https://github.com/nodejs/node/pull/25207 Reviewed-By: Michaël Zasso Reviewed-By: Anna Henningsen Reviewed-By: Ruben Bridgewater Reviewed-By: Tobias Nießen Reviewed-By: Luigi Pinca --- src/aliased_buffer.h | 4 +--- src/base_object-inl.h | 4 +--- src/inspector/node_string.h | 4 ---- src/string_decoder.cc | 4 +--- src/util.h | 2 ++ 5 files changed, 5 insertions(+), 13 deletions(-) diff --git a/src/aliased_buffer.h b/src/aliased_buffer.h index 0bae974af7a52c..eae60f4d93a9c0 100644 --- a/src/aliased_buffer.h +++ b/src/aliased_buffer.h @@ -231,9 +231,7 @@ class AliasedBuffer { void reserve(size_t new_capacity) { DCHECK_GE(new_capacity, count_); DCHECK_EQ(byte_offset_, 0); -#if defined(DEBUG) && DEBUG - CHECK(free_buffer_); -#endif + DCHECK(free_buffer_); const v8::HandleScope handle_scope(isolate_); const size_t old_size_in_bytes = sizeof(NativeT) * count_; diff --git a/src/base_object-inl.h b/src/base_object-inl.h index 84c449a30814e2..f1f1498e6c6128 100644 --- a/src/base_object-inl.h +++ b/src/base_object-inl.h @@ -113,9 +113,7 @@ void BaseObject::ClearWeak() { v8::Local BaseObject::MakeLazilyInitializedJSTemplate(Environment* env) { auto constructor = [](const v8::FunctionCallbackInfo& args) { -#ifdef DEBUG - CHECK(args.IsConstructCall()); -#endif + DCHECK(args.IsConstructCall()); DCHECK_GT(args.This()->InternalFieldCount(), 0); args.This()->SetAlignedPointerInInternalField(0, nullptr); }; diff --git a/src/inspector/node_string.h b/src/inspector/node_string.h index 4588364ab12196..504798853675a5 100644 --- a/src/inspector/node_string.h +++ b/src/inspector/node_string.h @@ -73,8 +73,4 @@ extern size_t kNotFound; } // namespace inspector } // namespace node -#ifndef DCHECK - #define DCHECK CHECK - #define DCHECK_LT CHECK_LT -#endif // DCHECK #endif // SRC_INSPECTOR_NODE_STRING_H_ diff --git a/src/string_decoder.cc b/src/string_decoder.cc index 96c6baa4d815e4..ceee1c9d762060 100644 --- a/src/string_decoder.cc +++ b/src/string_decoder.cc @@ -44,9 +44,7 @@ MaybeLocal MakeString(Isolate* isolate, isolate->ThrowException(error); } -#ifdef DEBUG - CHECK(ret.IsEmpty() || ret.ToLocalChecked()->IsString()); -#endif + DCHECK(ret.IsEmpty() || ret.ToLocalChecked()->IsString()); return ret.FromMaybe(Local()).As(); } diff --git a/src/util.h b/src/util.h index 36a2ec9e3a6270..d3835c7e692b7d 100644 --- a/src/util.h +++ b/src/util.h @@ -130,6 +130,7 @@ void DumpBacktrace(FILE* fp); #define CHECK_IMPLIES(a, b) CHECK(!(a) || (b)) #ifdef DEBUG + #define DCHECK(expr) CHECK(expr) #define DCHECK_EQ(a, b) CHECK((a) == (b)) #define DCHECK_GE(a, b) CHECK((a) >= (b)) #define DCHECK_GT(a, b) CHECK((a) > (b)) @@ -140,6 +141,7 @@ void DumpBacktrace(FILE* fp); #define DCHECK_NOT_NULL(val) CHECK((val) != nullptr) #define DCHECK_IMPLIES(a, b) CHECK(!(a) || (b)) #else + #define DCHECK(expr) #define DCHECK_EQ(a, b) #define DCHECK_GE(a, b) #define DCHECK_GT(a, b)