[webkit-reviews] review denied: [Bug 117146] Replace use of NDEBUG option with ASSERT_DISABLED in InlineBox : [Attachment 203647] Patch

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has denied Kangil Han
<kangil.han at samsung.com>'s request for review:
Bug 117146: Replace use of NDEBUG option with ASSERT_DISABLED in InlineBox
https://bugs.webkit.org/show_bug.cgi?id=117146

Attachment 203647: Patch
https://bugs.webkit.org/attachment.cgi?id=203647&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=203647&action=review


> Source/WebCore/rendering/InlineBox.cpp:35
> -#ifndef NDEBUG
> +#if !ASSERT_DISABLED
>  #include <stdio.h>
>  #endif

stdio.h is needed for code below that is genuinely DEBUG-specific, so this
change is not correct.

> Source/WebCore/rendering/InlineBox.cpp:95
> +#if !ASSERT_DISABLED
>      ASSERT(inInlineBoxDetach);
> -
> +#endif

This instance doesn't need to be guarded - ASSERT macro itself has the check.

> Source/WebCore/rendering/InlineBox.cpp:101
> -#ifndef NDEBUG
> +#if !ASSERT_DISABLED
>  const char* InlineBox::boxName() const

These functions are DEBUG specific, they are not related to assertions at all.
So, NDEBUG was correct here.

> Source/WebCore/rendering/InlineBox.cpp:395
> -#ifndef NDEBUG
> +#if !ASSERT_DISABLED

Ditto.

> Source/WebCore/rendering/InlineBox.h:111
> -#ifndef NDEBUG
> +#if !ASSERT_DISABLED

Ditto.

> Source/WebCore/rendering/InlineBox.h:439
> -#ifndef NDEBUG
> +#if !ASSERT_DISABLED

Ditto.