Bug 77960 – CSS3 calc() - simple parse time evaluation

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Mike Lawther]

CSS3 calc() - simple parse time evaluation

[Mike Lawther]

Created attachment 125806 [details]
Patch

[Ojan Vafai]

Comment on attachment 125806 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=125806&action=review

I know it's a pain to post these smaller patches, but these are 1000x easier to review. :)

> Source/WebCore/css/CSSCalculationValue.cpp:117
> +
> +        ASSERT_NOT_REACHED();
> +        return 0;

Does the compiler complain if you leave this out? If not, I'd just delete this since you will get a warning if a new Calc type is added since your switch above doesn't have a "default" clause (== a good thing!).

> Source/WebCore/css/CSSCalculationValue.cpp:206
> +        case CalcMod:
> +            if (rightValue)
> +                return static_cast<int>(leftValue) % static_cast<int>(rightValue);
> +            return std::numeric_limits<double>::quiet_NaN();

Wasn't mod dropped from the spec?

> Source/WebCore/css/CSSCalculationValue.cpp:209
> +        ASSERT_NOT_REACHED();
> +        return 0;

Ditto re: deleting the unreachable code if the compiler doesn't complain.

[Mike Lawther]

Comment on attachment 125806 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=125806&action=review

>> Source/WebCore/css/CSSCalculationValue.cpp:117
>> +        return 0;
> 
> Does the compiler complain if you leave this out? If not, I'd just delete this since you will get a warning if a new Calc type is added since your switch above doesn't have a "default" clause (== a good thing!).

Nope, it doesn't. Removed.

>> Source/WebCore/css/CSSCalculationValue.cpp:206
>> +            return std::numeric_limits<double>::quiet_NaN();
> 
> Wasn't mod dropped from the spec?

Indeed it has been. I've removed the evaluation code, and I'll get rid of the enum value in another patch.

>> Source/WebCore/css/CSSCalculationValue.cpp:209
>> +        return 0;
> 
> Ditto re: deleting the unreachable code if the compiler doesn't complain.

Removed.

[Mike Lawther]

Created attachment 125956 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 125956 [details]
Patch for landing

Clearing flags on attachment: 125956

Committed r107030: <http://trac.webkit.org/changeset/107030>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.