[Webkit-unassigned] [Bug 203312] Implement the CSS exponent functions: pow(), sqrt(), hypot()

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=203312

Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |darin at apple.com

--- Comment #3 from Darin Adler <darin at apple.com> ---
Comment on attachment 417716
  --> https://bugs.webkit.org/attachment.cgi?id=417716
Patch

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

Thanks for contributing this. Looks generally good.

Preliminary comments before EWS run is done, so not setting review+ or review- yet.

Not sure we have covered enough of the edge cases in our tests.

> Source/WebCore/css/CSSCalculationValue.cpp:914
> +    

Extra blank line here.

> Source/WebCore/css/CSSCalculationValue.cpp:916
> +    for (auto child : values) {

Should be auto& instead of auto so we don’t churn reference counts.

> Source/WebCore/css/CSSCalculationValue.cpp:930
> +    if (expectedCategory != CalculationCategory::Number && expectedCategory != CalculationCategory::Length && expectedCategory != CalculationCategory::Percent)

Helper function so we don’t include this twice, here and in the loop.

> Source/WebCore/css/CSSCalculationValue.cpp:1679
> +        double base = children[0];
> +        double exp = children[1];
> +        return std::pow(base, exp);

I don’t think the local variables help us here.

> Source/WebCore/css/CSSCalculationValue.cpp:1685
> +        double val = children[0];

WebKit coding style says to call this "value", not "val".

> Source/WebCore/css/CSSCalculationValue.cpp:1833
> -
> +        

Please don’t add trailing whitespace.

> Source/WebCore/platform/CalculationValue.cpp:183
> +        float val = m_children[0]->evaluate(maxValue);

WebKit coding style says to call this "value", not "val".

> LayoutTests/fast/css/calc-parsing.html:61
> +            testExpression('calc(sqrt(100px)', '999px', '999px');

How is this working given we forgot the end parenthesis?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20210115/67795143/attachment-0001.htm>