[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

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

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

A few other minor comments.

>> Source/WebCore/css/CSSCalculationValue.cpp:792
>> +    bool canHaveOnlyChild() const { return m_operator == CalcOperator::Hypot || m_operator == CalcOperator::Sqrt; }
> 
> This name is a bit ambiguous. Is it "can have only one child"? Maybe allowsSingleChild()?

I think this name isn’t quite right. This returns true for operators that are not identity functions when they have a single child. Not, as the title implies, the only operators that allow a single child. So we should use a different name. Maybe we should also reverse the sense of the function. It’s about an optimization in how we implement the other functions like min and max.

> Source/WebCore/css/CSSCalculationValue.cpp:935
> +        auto currentCategory = values[i]->category();
> +        if (currentCategory != expectedCategory)
> +            return nullptr;

Probably reads slightly better without the local variable.

> Source/WebCore/css/CSSCalculationValue.cpp:1670
> +    case CalcOperator::Pow: {

The items above don’t use braces when they don’t have local variables. This could omit braces to match those.

> Source/WebCore/css/CSSCalculationValue.cpp:1676
> +    }
> +    
> +    case CalcOperator::Sqrt: {

These blank lines are different than the formatting above. Let’s try to be consistent. I don’t mind the blank lines, but I would like the entire switch statement to stay consistent.

> Source/WebCore/css/CSSCalculationValue.cpp:1685
> +    }
> +    
> +    case CalcOperator::Hypot: {

Ditto.

-- 
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/858f6ec6/attachment.htm>