> And I do agree that it can be inconvenient to deal with these warnings in 
> heavily #ifdef’d code. I think there are some good strategies for avoiding 
> that, and I’d like to talk about specific irritating cases so I can propose 
> the strategy I’d push for. Generally the strategy is to push more 
> configuration specifics to the edges rather than spreading them through 
> multiple functions.

A few irritating cases for me:

(1)

static inline void validate(const Range& range)
{
    UNUSED(range);
IF_DEBUG(
    BeginTag* beginTag = LargeChunk::beginTag(range.begin());
    EndTag* endTag = LargeChunk::endTag(range.begin(), range.size());

    BASSERT(!beginTag->isEnd());
    BASSERT(range.size() >= largeMin);
    BASSERT(beginTag->size() == range.size());

    BASSERT(beginTag->size() == endTag->size());
    BASSERT(beginTag->isFree() == endTag->isFree());
    BASSERT(beginTag->hasPhysicalPages() == endTag->hasPhysicalPages());
    BASSERT(static_cast<BoundaryTag*>(endTag) == 
static_cast<BoundaryTag*>(beginTag) || endTag->isEnd());
);
}

(2)

inline void vmValidate(size_t vmSize)
{
    // We use getpagesize() here instead of vmPageSize because vmPageSize is
    // allowed to be larger than the OS's true page size.

    UNUSED(vmSize);
    BASSERT(vmSize);
    BASSERT(vmSize == roundUpToMultipleOf(static_cast<size_t>(getpagesize()), 
vmSize));
}

(3)

inline void vmValidate(void* p, size_t vmSize)
{
    // We use getpagesize() here instead of vmPageSize because vmPageSize is
    // allowed to be larger than the OS's true page size.

    vmValidate(vmSize);
    
    UNUSED(p);
    BASSERT(p);
    BASSERT(p == mask(p, ~(getpagesize() - 1)));
}

The common theme in these cases is that I wanted to write a helper function to 
do some ASSERTs which compiled to nothing in a release build. Compiling the 
function to nothing made its parameters look like errors.

I considered having the validate function return a value, and having the caller 
ASSERT(validate(x)) — that way, the call would compile to nothing, and the 
callee would always appear to do something with its parameters. I didn’t like 
that solution for two reasons:

(1) When debugging or reading a crash report, I wanted the indicated line of 
code to be the specific condition that failed. If the caller ASSERTs after a 
long line of reasoning, the indicated line of code is the non-specific ASSERT 
in the caller. This was a big deal to me.

(2) In cases where one validate needed to call another, I sometimes still had 
an unused parameter in the caller.

Geoff
_______________________________________________
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Reply via email to