[webkit-dev] Style proposal for branch style for macros

[Mark Rowe]

On Apr 20, 2014, at 13:19, Mark Rowe <mrowe at apple.com> wrote:

> 
> On Apr 20, 2014, at 13:08, Filip Pizlo <fpizlo at apple.com> wrote:
> 
>> 
>> 
>> On Apr 20, 2014, at 12:56 PM, Mark Rowe <mrowe at apple.com> wrote:
>> 
>>> 
>>> On Apr 19, 2014, at 13:09, Filip Pizlo <fpizlo at apple.com> wrote:
>>> 
>>>> Hey everyone,
>>>> 
>>>> When guarding code with macros that are always defined, such as ASSERT_DISABLED (it's always either 0 or 1), we have a choice between:
>>>> 
>>>> 	if (!ASSERT_DISABLED) {
>>>> 	    // do things
>>>> 	}
>>>> 
>>>> and:
>>>> 
>>>> #if !ASSERT_DISABLED
>>>> 	// do things
>>>> #endif
>>>> 
>>>> I'd like to propose that anytime the normal if would be semantically equivalent to the preprocessor #if, the normal if should be used.
>>>> 
>>>> We don't lose any compiler optimization, since even at -O0, the compiler will constant fold the normal if.  We do gain a lot of clarity, since the control flow of normal if statements is subject to proper indentation.
>>>> 
>>>> The "semantically equivalent" requirement still allows for #if to be used for thinngs like:
>>>> 
>>>> - Guarding the placement of fields in a class.
>>>> - Guarding the definitions of other macros (in the case of ASSERT_DISABLED, we define ASSERT in different ways guarded by #if's)
>>>> - Guarding the definition/declaration/inclusion of entire functions, classes, and other top-level constructs.
>>>> 
>>>> Thoughts?
>>> 
>>> It’d be one thing if we could adopt this approach everywhere, but as you note there are numerous situations in which it won’t compile. What’s worse is that there are situations in which it’ll silently give unintended behavior. 
>> 
>> Can you give an example of this?
> 
> Won’t compile:
> 
> if (!MAYBE_DEFINED) {
>     …
> }
> 
> if (SOMETHING) {
>     void doSomething() { }
> }
> 
> if (SOMETHING) {
>     #include “Something.h"
> }

It’s also not possible to determine from context alone whether #if or if should be used. When #if is used to remove members or functions you’re then forced to use #if, rather than if, around all uses of those members or functions since the alternative won’t compile.

An example lifted from Vector.h:

#if !ASSERT_DISABLED
template<typename T> struct ValueCheck<Vector<T>> {
    typedef Vector<T> TraitType;
    static void checkConsistency(const Vector<T>& v)
    {
        v.checkConsistency();
    }
};
#endif

template<typename T, size_t inlineCapacity, typename OverflowHandler>
inline void Vector<T, inlineCapacity, OverflowHandler>::checkConsistency()
{
    if (!ASSERT_DISABLED) {
        for (size_t i = 0; i < size(); ++i)
            ValueCheck<T>::checkConsistency(at(i));
    }
}


This won’t compile with assertions disabled because ValueCheck::checkConsistency doesn’t exist.

- Mark

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-dev/attachments/20140420/6affee7d/attachment.html>