[webkit-dev] Proposal for coding guidelines: Do not use fall-through switch cases inside #ifdef's

[Jake]

ah ok.


On Sat, Aug 18, 2012 at 10:06 PM, Tim Horton <timothy_horton at apple.com>wrote:

>
> On Aug 18, 2012, at 8:35 PM, Jake <jake at jakeonthenet.com> wrote:
>
> Just a guess but shouldn't the #if line be:
>
> #if defined(ENABLE_CSS3_TEXT_DECORATION) &&
> defined(ENABLE_CSS3_TEXT_DECORATION)
>
>
> No, this is supposed to be "if ENABLE_CSS3_TEXT_DECORATION is defined and
> it is defined to true". The line is correct as it stands.
>
> -Jake
>
> On Sat, Aug 18, 2012 at 8:20 PM, Tim Horton <timothy_horton at apple.com>wrote:
>
>>
>>
>> On Aug 17, 2012, at 11:57 AM, Bruno Abinader <brunoabinader at gmail.com>
>> wrote:
>>
>> > On Fri, Aug 17, 2012 at 2:24 PM, Joe Mason <jmason at rim.com> wrote:
>> >> What do you mean by "precompiler"?  The preprocessor?  Or is this a
>> precompiled headers thing?
>> >
>> > Indeed, s/precompiler/preprocessor :)
>> >
>> >>
>> >> Are you sure there wasn't a typo in the "case
>> CSSPropertyWebkitTextDecorationLine:" line which caused a perfectly normal
>> syntax error when CSS3_TEXT_DECORATION was defined?  Or maybe
>> CSSPropertyWebkitTextDecorationLine was not defined?
>> >
>> > There is no typo as far as I've checked. The following was added to
>> > Source/WebCore/css/CSSPropertyNames.in:
>> >
>> > ...
>> > #if defined(ENABLE_CSS3_TEXT_DECORATION) && ENABLE_CSS3_TEXT_DECORATION
>> > -webkit-text-decoration-line
>> > #endif
>> > ...
>> >
>> >> I dislike this.  Repeated code should be avoided. Fallthrough is a
>> widely used and accepted technique to do that, ever since the days of C. I
>> can't believe there's a compiler that doesn't handle this correctly.
>> >
>> > +1 on that. I've re-checked the build bot output and it seems the
>> > issue is actually caused because CSSPropertyWebkitTextDecorationLine
>> > gets undefined. So I'm starting to believe the fail reason is
>> > something else skipping from my eyes.
>>
>> Without looking at the code, I vaguely think that
>> CSSPropertyWebkitTextDecorationLine comes from a generated file that might
>> not rebuild when it's supposed to. You should try a clean build and see --
>> there've been a few problems with this recently (I ran into it when
>> switching flexbox and regions/exclusions on and off a lot a few months ago,
>> and a colleague ran into it again recently in a similar situation).
>>
>> > The actual bug related to this
>> > issue is https://bugs.webkit.org/show_bug.cgi?id=94108 .
>> >
>> >>
>> >>> Where each switch case is handled individually (if you're curious
>> >>> about it, it is fixed in bug 90493). Said this, I would like to
>> >>
>> >> https://bugs.webkit.org/show_bug.cgi?id=90493 is "[chromium] Don't
>> archive build files generated by VS2010", and doesn't seem to be related.
>> >
>> > My bad :/ The right bug is 94093:
>> > https://bugs.webkit.org/show_bug.cgi?id=94093 .
>> >
>> > --
>> > Bruno de Oliveira Abinader
>> > _______________________________________________
>> > webkit-dev mailing list
>> > webkit-dev at lists.webkit.org
>> > http://lists.webkit.org/mailman/listinfo/webkit-dev
>>
>> _______________________________________________
>> webkit-dev mailing list
>> webkit-dev at lists.webkit.org
>> http://lists.webkit.org/mailman/listinfo/webkit-dev
>>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20120818/22b52e5b/attachment.html>