Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support late resolution of symbols in CSS calc #28391

Conversation

weinig
Copy link
Contributor

@weinig weinig commented May 10, 2024

c7dc51f

Support late resolution of symbols in CSS calc
https://bugs.webkit.org/show_bug.cgi?id=274001

Reviewed by Darin Adler.

This change makes it so that symbols passed to `calc()`, such as done
for relative colors, are kept as symbols until evaluation of the `calc()`
is performed. This will allow us to delay resolving the symbols in
relative colors in cases like an origin color of `currentColor`, where
we don't have the values to resolve the symbol until much later.

To make this possible, a new `CSSCalcExpressionNode` type is added,
`CSSCalcSymbolNode`, which represents the unresolved symbol. Additionally,
instead of passing the `CSSCalcSymbolTable` to the parser, a new
`CSSCalcSymbolsAllowed` table is passed there, and `CSSCalcSymbolTable`
is passed to the evaluation function `doubleValue()`.

Only a few small changes were needed to processing `calc()` to support
these unresolved symbols, primarily in `CSSCalcOperationNode` where
a few places were making assumptions about how child nodes would be
combined by simplification if they had the same types, but with symbols
that is not possible in all cases anymore.

* Source/WebCore/Sources.txt:
* Source/WebCore/WebCore.xcodeproj/project.pbxproj:
    - Add new files.

* Source/WebCore/css/CSSPrimitiveValue.cpp:
(WebCore::CSSPrimitiveValue::doubleValue const):
(WebCore::CSSPrimitiveValue::doubleValueDividingBy100IfPercentage const):
    - Pass empty symbol table to `doubleValue()`.

* Source/WebCore/css/calc/CSSCalcExpressionNode.cpp:
    - Move virtual destructor out of line to avoid vtable duplication.

* Source/WebCore/css/calc/CSSCalcExpressionNode.h:
    - Adds new `CssCalcSymbol` type
    - Adds new `isResolvable()` pure virtual function, which returns false
      if the node, or any of its children, is an unresolved symbol.
    - Adds `CSSCalcSymbolTable` parameter to `doubleValue()`.

* Source/WebCore/css/calc/CSSCalcExpressionNodeParser.cpp:
(WebCore::CSSCalcExpressionNodeParser::parseValue):
* Source/WebCore/css/calc/CSSCalcExpressionNodeParser.h:
(WebCore::CSSCalcExpressionNodeParser::CSSCalcExpressionNodeParser):
    - Utilize CSSCalcSymbolsAllowed to create symbol nodes for symbols
      at parse time.

* Source/WebCore/css/calc/CSSCalcInvertNode.cpp:
(WebCore::CSSCalcInvertNode::isResolvable const):
(WebCore::CSSCalcInvertNode::doubleValue const):
* Source/WebCore/css/calc/CSSCalcInvertNode.h:
    - Implement `isResolvable() and pass symbol table through to children
      in `doubleValue()`.

* Source/WebCore/css/calc/CSSCalcNegateNode.cpp:
(WebCore::CSSCalcNegateNode::isResolvable const):
(WebCore::CSSCalcNegateNode::doubleValue const):
* Source/WebCore/css/calc/CSSCalcNegateNode.h:
    - Implement `isResolvable() and pass symbol table through to children
      in `doubleValue()`.

* Source/WebCore/css/calc/CSSCalcOperationNode.h:
* Source/WebCore/css/calc/CSSCalcOperationNode.cpp:
(WebCore::CSSCalcOperationNode::combineChildren):
    - Adds check for `isResolvable()` before trying to early evaluate
      math functions, as you can't evaluate them if child is a symbol.
    - Passes empty symbol table through to resolvable children via
      `doubleValue()`.

(WebCore::CSSCalcOperationNode::isResolvable const):
    - Implement by checking children for `isResolvable()`.

(WebCore::CSSCalcOperationNode::isZero const):
    - Move definition to implementation file to avoid #including
      CSSCalcSymbolTable in the header.

(WebCore::CSSCalcOperationNode::primitiveType const):
    - Add support for computing the primitive type when there are
      multiple nodes of the same type that have not been combined,
      which can now happen with symbols.

(WebCore::CSSCalcOperationNode::doubleValue const):
    - Pass symbol table through to children.

(WebCore::CSSCalcOperationNode::buildCSSTextRecursive):
    - Pass empty symbol table to `doubleValue()`, which is fine because the
      child has been explicitly checked to be a `CSSCalcPrimitiveValueNode`.

* Source/WebCore/css/calc/CSSCalcPrimitiveValueNode.cpp:
(WebCore::CSSCalcPrimitiveValueNode::add):
(WebCore::CSSCalcPrimitiveValueNode::doubleValue const):
(WebCore::CSSCalcPrimitiveValueNode::isResolvable const):
* Source/WebCore/css/calc/CSSCalcPrimitiveValueNode.h:
    - Implement `isResolvable() and pass symbol table through to children
      in `doubleValue()`.

* Source/WebCore/css/calc/CSSCalcSymbolNode.cpp: Added.
* Source/WebCore/css/calc/CSSCalcSymbolNode.h: Added.
    - New `calc` node. Only supported for numeric (`doubleValue()`) `calc()`,
      as there are currently no uses of symbols for anything else and adding
      additional support would require significantly more plumbing with
      `CalcExpressionNode` for no benefit.

* Source/WebCore/css/calc/CSSCalcSymbolTable.cpp:
* Source/WebCore/css/calc/CSSCalcSymbolTable.h:
    - Remove WeakPtr support. `CSSCalcSymbolTable` is not stored anywhere
      anymore, just passed through `doubleValue()`.

* Source/WebCore/css/calc/CSSCalcSymbolsAllowed.cpp: Copied from Source/WebCore/css/calc/CSSCalcSymbolTable.cpp.
(WebCore::CSSCalcSymbolsAllowed::CSSCalcSymbolsAllowed):
(WebCore::CSSCalcSymbolsAllowed::get const):
(WebCore::CSSCalcSymbolsAllowed::contains const):
* Source/WebCore/css/calc/CSSCalcSymbolsAllowed.h: Copied from Source/WebCore/css/calc/CSSCalcSymbolTable.h.
    - Added new table for allowed symbols which includes the symbol's
      name and type.

* Source/WebCore/css/calc/CSSCalcValue.cpp:
(WebCore::CSSCalcValue::doubleValue const):
(WebCore::CSSCalcValue::create):
* Source/WebCore/css/calc/CSSCalcValue.h:
    - Pass the `CSSCalcSymbolsAllowed` through the constructor, and
      `CSSCalcSymbolTable` through `doubleValue()`.

* Source/WebCore/css/parser/CSSCalcParser.cpp:
(WebCore::CSSPropertyParserHelpers::CalcParser::CalcParser):
(WebCore::CSSPropertyParserHelpers::consumeCalcRawWithKnownTokenTypeFunction):
* Source/WebCore/css/parser/CSSCalcParser.h:
    - Replace `CSSCalcSymbolTable` with `CSSCalcSymbolsAllowed` for parsing.

* Source/WebCore/css/parser/CSSPropertyParserConsumer+Angle.cpp:
* Source/WebCore/css/parser/CSSPropertyParserConsumer+AngleDefinitions.h:
    - Replace `CSSCalcSymbolTable` with `CSSCalcSymbolsAllowed` for parsing,
      and pass empty values for both to combined `consumeAndResolve()`.

* Source/WebCore/css/parser/CSSPropertyParserConsumer+CSSPrimitiveValueResolver.cpp:
* Source/WebCore/css/parser/CSSPropertyParserConsumer+CSSPrimitiveValueResolver.h:
    - Pass `CSSCalcSymbolTable` to `doubleValue()` for resolution.

* Source/WebCore/css/parser/CSSPropertyParserConsumer+Color.cpp:
(WebCore::CSSPropertyParserHelpers::consumeAbsoluteComponent):
(WebCore::CSSPropertyParserHelpers::consumeRelativeComponent):
(WebCore::CSSPropertyParserHelpers::consumeRelativeComponents):
    - Pass both the `CSSCalcSymbolsAllowed` and `CSSCalcSymbolTable` to shared
      consume/resolve functions. This is temporary, and in a subsequent change
      resolution will be in a distinct place from consuming.

* Source/WebCore/css/parser/CSSPropertyParserConsumer+Integer.cpp:
* Source/WebCore/css/parser/CSSPropertyParserConsumer+IntegerDefinitions.h:
    - Replace `CSSCalcSymbolTable` with `CSSCalcSymbolsAllowed` for parsing,
      and pass empty values for both to combined `consumeAndResolve()`.

* Source/WebCore/css/parser/CSSPropertyParserConsumer+Length.cpp:
* Source/WebCore/css/parser/CSSPropertyParserConsumer+Length.h:
* Source/WebCore/css/parser/CSSPropertyParserConsumer+LengthDefinitions.h:
    - Replace `CSSCalcSymbolTable` with `CSSCalcSymbolsAllowed` for parsing,
      and pass empty values for both to combined `consumeAndResolve()`.

* Source/WebCore/css/parser/CSSPropertyParserConsumer+MetaConsumer.h:
(WebCore::CSSPropertyParserHelpers::MetaConsumer::consume):
    - Replace `CSSCalcSymbolTable` with `CSSCalcSymbolsAllowed` for parsing.

* Source/WebCore/css/parser/CSSPropertyParserConsumer+MetaResolver.h:
(WebCore::CSSPropertyParserHelpers::MetaResolver::consumeAndResolve):
    - Replace `CSSCalcSymbolTable` with `CSSCalcSymbolsAllowed` for parsing,
      and pass both to combined `consumeAndResolve()`.

* Source/WebCore/css/parser/CSSPropertyParserConsumer+None.cpp:
(WebCore::CSSPropertyParserHelpers::NoneKnownTokenTypeIdentConsumer::consume):
* Source/WebCore/css/parser/CSSPropertyParserConsumer+NoneDefinitions.h:
    - Replace `CSSCalcSymbolTable` with `CSSCalcSymbolsAllowed` for parsing.

* Source/WebCore/css/parser/CSSPropertyParserConsumer+Number.cpp:
* Source/WebCore/css/parser/CSSPropertyParserConsumer+NumberDefinitions.h:
    - Replace `CSSCalcSymbolTable` with `CSSCalcSymbolsAllowed` for parsing,
      and pass empty values for both to combined `consumeAndResolve()`.

* Source/WebCore/css/parser/CSSPropertyParserConsumer+Percent.cpp:
* Source/WebCore/css/parser/CSSPropertyParserConsumer+PercentDefinitions.h:
    - Replace `CSSCalcSymbolTable` with `CSSCalcSymbolsAllowed` for parsing,
      and pass empty values for both to combined `consumeAndResolve()`.

* Source/WebCore/css/parser/CSSPropertyParserConsumer+Primitives.cpp:
(WebCore::CSSPropertyParserHelpers::equal): Deleted.
* Source/WebCore/css/parser/CSSPropertyParserConsumer+Primitives.h:
    - Moves UnevaluatedCalc out to its own files.

* Source/WebCore/css/parser/CSSPropertyParserConsumer+RawResolver.cpp:
* Source/WebCore/css/parser/CSSPropertyParserConsumer+RawResolver.h:
    - Pass `CSSCalcSymbolTable` to `doubleValue()` for resolution.
    - Utilize shared support for symbol replacing via `replaceSymbol()`.
    - Utilize shared support for calc evaluation via `evaluateCalc().

* Source/WebCore/css/parser/CSSPropertyParserConsumer+RawTypes.cpp: Added
(WebCore::replaceSymbol):
    - Added shared place for direct symbol (e.g. not in a `calc()`) replacement
      via a symbol table.

* Source/WebCore/css/parser/CSSPropertyParserConsumer+RawTypes.h:
    - Moved raw types out of the CSSPropertyParserHelpers as it is now used
      beyond just the parser helpers and having that long prefix was unwieldily.

* Source/WebCore/css/parser/CSSPropertyParserConsumer+Resolution.cpp:
(WebCore::CSSPropertyParserHelpers::consumeResolution):
* Source/WebCore/css/parser/CSSPropertyParserConsumer+ResolutionDefinitions.h:
    - Replace `CSSCalcSymbolTable` with `CSSCalcSymbolsAllowed` for parsing,
      and pass empty values for both to combined `consumeAndResolve()`.

* Source/WebCore/css/parser/CSSPropertyParserConsumer+Symbol.cpp:
* Source/WebCore/css/parser/CSSPropertyParserConsumer+SymbolDefinitions.h:
    - Replace `CSSCalcSymbolTable` with `CSSCalcSymbolsAllowed` for parsing,
      and pass empty values for both to combined `consumeAndResolve()`.

* Source/WebCore/css/parser/CSSPropertyParserConsumer+Time.cpp:
* Source/WebCore/css/parser/CSSPropertyParserConsumer+Time.h:
* Source/WebCore/css/parser/CSSPropertyParserConsumer+TimeDefinitions.h:
    - Replace `CSSCalcSymbolTable` with `CSSCalcSymbolsAllowed` for parsing,
      and pass empty values for both to combined `consumeAndResolve()`.

* Source/WebCore/css/parser/CSSPropertyParserConsumer+UnevaluatedCalc.cpp: Copied from Source/WebCore/css/parser/CSSPropertyParserConsumer+Primitives.cpp.
(WebCore::CSSPropertyParserHelpers::unevaluatedCalcEqual):
(WebCore::CSSPropertyParserHelpers::unevaluatedCalcSerialization):
(WebCore::CSSPropertyParserHelpers::evaluateCalc):
* Source/WebCore/css/parser/CSSPropertyParserConsumer+UnevaluatedCalc.h: Added.
(WebCore::CSSPropertyParserHelpers::UnevaluatedCalc::operator== const):
(WebCore::CSSPropertyParserHelpers::serializationForCSS):
(WebCore::CSSPropertyParserHelpers::evaluateCalc):
    - Move UnevaluatedCalc to its own files and add support for evaluation that
      passes symbol table through and returns the correct raw type.

* Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:
    - Replace `CSSCalcSymbolTable` with `CSSCalcSymbolsAllowed` for parsing,
      and pass empty values for both to combined `consumeAndResolve()`.

* Source/WebCore/css/parser/CSSPropertyParserHelpers.h:
    - Removed CSSPropertyParserHelpers namespace prefixes from raw types.

* Source/WebCore/css/parser/CSSPropertyParserWorkerSafe.cpp:
    - Added now needed missing #include.

* Source/WebCore/css/typedom/CSSNumericValue.cpp:
(WebCore::reifyMathExpression):
(WebCore::CSSNumericValue::reifyMathExpression):
    - Fill in support for the new calc node. Currently unsupported by the spec,
      so throughs an exception.

* Source/WebCore/style/StyleResolveForFontRaw.cpp:
    - Removed CSSPropertyParserHelpers namespace prefixes from raw types.

Canonical link: https://commits.webkit.org/278635@main

9330e20

Misc iOS, tvOS & watchOS macOS Linux Windows
❌ πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe βœ… πŸ›  wincairo
βœ… πŸ§ͺ bindings βœ… πŸ›  ios-sim βœ… πŸ›  mac-AS-debug   πŸ§ͺ wpe-wk2
βœ… πŸ§ͺ webkitperl βœ… πŸ§ͺ ios-wk2 βœ… πŸ§ͺ api-mac βœ… πŸ§ͺ api-wpe
βœ… πŸ§ͺ ios-wk2-wpt βœ… πŸ§ͺ mac-wk1 ⏳ πŸ›  wpe-skia
  πŸ§ͺ api-ios βœ… πŸ§ͺ mac-wk2 βœ… πŸ›  gtk
βœ… πŸ›  tv βœ… πŸ§ͺ mac-AS-debug-wk2   πŸ§ͺ gtk-wk2
βœ… πŸ›  tv-sim βœ… πŸ§ͺ mac-wk2-stress   πŸ§ͺ api-gtk
βœ… πŸ›  πŸ§ͺ merge βœ… πŸ›  watch
βœ… πŸ›  watch-sim

@weinig weinig self-assigned this May 10, 2024
@weinig weinig added the CSS Cascading Style Sheets implementation label May 10, 2024
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 10, 2024
@weinig weinig removed the merging-blocked Applied to prevent a change from being merged label May 10, 2024
@weinig weinig force-pushed the eng/Support-late-resolution-of-symbols-in-CSS-calc branch from a7c1b10 to 5574449 Compare May 10, 2024 16:52
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 10, 2024
Source/WebCore/css/calc/CSSCalcSymbolNode.h Outdated Show resolved Hide resolved
Source/WebCore/css/calc/CSSCalcSymbolNode.h Outdated Show resolved Hide resolved
Source/WebCore/css/calc/CSSCalcSymbolsAllowed.cpp Outdated Show resolved Hide resolved
Source/WebCore/css/calc/CSSCalcValue.h Outdated Show resolved Hide resolved
@weinig weinig removed the merging-blocked Applied to prevent a change from being merged label May 10, 2024
@weinig weinig force-pushed the eng/Support-late-resolution-of-symbols-in-CSS-calc branch from 5574449 to 9330e20 Compare May 10, 2024 21:36
@weinig weinig added the merge-queue Applied to send a pull request to merge-queue label May 10, 2024
@webkit-commit-queue
Copy link
Collaborator

@webkit-commit-queue webkit-commit-queue added merging-blocked Applied to prevent a change from being merged and removed merge-queue Applied to send a pull request to merge-queue labels May 10, 2024
@nt1m nt1m added merge-queue Applied to send a pull request to merge-queue and removed merging-blocked Applied to prevent a change from being merged labels May 10, 2024
https://bugs.webkit.org/show_bug.cgi?id=274001

Reviewed by Darin Adler.

This change makes it so that symbols passed to `calc()`, such as done
for relative colors, are kept as symbols until evaluation of the `calc()`
is performed. This will allow us to delay resolving the symbols in
relative colors in cases like an origin color of `currentColor`, where
we don't have the values to resolve the symbol until much later.

To make this possible, a new `CSSCalcExpressionNode` type is added,
`CSSCalcSymbolNode`, which represents the unresolved symbol. Additionally,
instead of passing the `CSSCalcSymbolTable` to the parser, a new
`CSSCalcSymbolsAllowed` table is passed there, and `CSSCalcSymbolTable`
is passed to the evaluation function `doubleValue()`.

Only a few small changes were needed to processing `calc()` to support
these unresolved symbols, primarily in `CSSCalcOperationNode` where
a few places were making assumptions about how child nodes would be
combined by simplification if they had the same types, but with symbols
that is not possible in all cases anymore.

* Source/WebCore/Sources.txt:
* Source/WebCore/WebCore.xcodeproj/project.pbxproj:
    - Add new files.

* Source/WebCore/css/CSSPrimitiveValue.cpp:
(WebCore::CSSPrimitiveValue::doubleValue const):
(WebCore::CSSPrimitiveValue::doubleValueDividingBy100IfPercentage const):
    - Pass empty symbol table to `doubleValue()`.

* Source/WebCore/css/calc/CSSCalcExpressionNode.cpp:
    - Move virtual destructor out of line to avoid vtable duplication.

* Source/WebCore/css/calc/CSSCalcExpressionNode.h:
    - Adds new `CssCalcSymbol` type
    - Adds new `isResolvable()` pure virtual function, which returns false
      if the node, or any of its children, is an unresolved symbol.
    - Adds `CSSCalcSymbolTable` parameter to `doubleValue()`.

* Source/WebCore/css/calc/CSSCalcExpressionNodeParser.cpp:
(WebCore::CSSCalcExpressionNodeParser::parseValue):
* Source/WebCore/css/calc/CSSCalcExpressionNodeParser.h:
(WebCore::CSSCalcExpressionNodeParser::CSSCalcExpressionNodeParser):
    - Utilize CSSCalcSymbolsAllowed to create symbol nodes for symbols
      at parse time.

* Source/WebCore/css/calc/CSSCalcInvertNode.cpp:
(WebCore::CSSCalcInvertNode::isResolvable const):
(WebCore::CSSCalcInvertNode::doubleValue const):
* Source/WebCore/css/calc/CSSCalcInvertNode.h:
    - Implement `isResolvable() and pass symbol table through to children
      in `doubleValue()`.

* Source/WebCore/css/calc/CSSCalcNegateNode.cpp:
(WebCore::CSSCalcNegateNode::isResolvable const):
(WebCore::CSSCalcNegateNode::doubleValue const):
* Source/WebCore/css/calc/CSSCalcNegateNode.h:
    - Implement `isResolvable() and pass symbol table through to children
      in `doubleValue()`.

* Source/WebCore/css/calc/CSSCalcOperationNode.h:
* Source/WebCore/css/calc/CSSCalcOperationNode.cpp:
(WebCore::CSSCalcOperationNode::combineChildren):
    - Adds check for `isResolvable()` before trying to early evaluate
      math functions, as you can't evaluate them if child is a symbol.
    - Passes empty symbol table through to resolvable children via
      `doubleValue()`.

(WebCore::CSSCalcOperationNode::isResolvable const):
    - Implement by checking children for `isResolvable()`.

(WebCore::CSSCalcOperationNode::isZero const):
    - Move definition to implementation file to avoid #including
      CSSCalcSymbolTable in the header.

(WebCore::CSSCalcOperationNode::primitiveType const):
    - Add support for computing the primitive type when there are
      multiple nodes of the same type that have not been combined,
      which can now happen with symbols.

(WebCore::CSSCalcOperationNode::doubleValue const):
    - Pass symbol table through to children.

(WebCore::CSSCalcOperationNode::buildCSSTextRecursive):
    - Pass empty symbol table to `doubleValue()`, which is fine because the
      child has been explicitly checked to be a `CSSCalcPrimitiveValueNode`.

* Source/WebCore/css/calc/CSSCalcPrimitiveValueNode.cpp:
(WebCore::CSSCalcPrimitiveValueNode::add):
(WebCore::CSSCalcPrimitiveValueNode::doubleValue const):
(WebCore::CSSCalcPrimitiveValueNode::isResolvable const):
* Source/WebCore/css/calc/CSSCalcPrimitiveValueNode.h:
    - Implement `isResolvable() and pass symbol table through to children
      in `doubleValue()`.

* Source/WebCore/css/calc/CSSCalcSymbolNode.cpp: Added.
* Source/WebCore/css/calc/CSSCalcSymbolNode.h: Added.
    - New `calc` node. Only supported for numeric (`doubleValue()`) `calc()`,
      as there are currently no uses of symbols for anything else and adding
      additional support would require significantly more plumbing with
      `CalcExpressionNode` for no benefit.

* Source/WebCore/css/calc/CSSCalcSymbolTable.cpp:
* Source/WebCore/css/calc/CSSCalcSymbolTable.h:
    - Remove WeakPtr support. `CSSCalcSymbolTable` is not stored anywhere
      anymore, just passed through `doubleValue()`.

* Source/WebCore/css/calc/CSSCalcSymbolsAllowed.cpp: Copied from Source/WebCore/css/calc/CSSCalcSymbolTable.cpp.
(WebCore::CSSCalcSymbolsAllowed::CSSCalcSymbolsAllowed):
(WebCore::CSSCalcSymbolsAllowed::get const):
(WebCore::CSSCalcSymbolsAllowed::contains const):
* Source/WebCore/css/calc/CSSCalcSymbolsAllowed.h: Copied from Source/WebCore/css/calc/CSSCalcSymbolTable.h.
    - Added new table for allowed symbols which includes the symbol's
      name and type.

* Source/WebCore/css/calc/CSSCalcValue.cpp:
(WebCore::CSSCalcValue::doubleValue const):
(WebCore::CSSCalcValue::create):
* Source/WebCore/css/calc/CSSCalcValue.h:
    - Pass the `CSSCalcSymbolsAllowed` through the constructor, and
      `CSSCalcSymbolTable` through `doubleValue()`.

* Source/WebCore/css/parser/CSSCalcParser.cpp:
(WebCore::CSSPropertyParserHelpers::CalcParser::CalcParser):
(WebCore::CSSPropertyParserHelpers::consumeCalcRawWithKnownTokenTypeFunction):
* Source/WebCore/css/parser/CSSCalcParser.h:
    - Replace `CSSCalcSymbolTable` with `CSSCalcSymbolsAllowed` for parsing.

* Source/WebCore/css/parser/CSSPropertyParserConsumer+Angle.cpp:
* Source/WebCore/css/parser/CSSPropertyParserConsumer+AngleDefinitions.h:
    - Replace `CSSCalcSymbolTable` with `CSSCalcSymbolsAllowed` for parsing,
      and pass empty values for both to combined `consumeAndResolve()`.

* Source/WebCore/css/parser/CSSPropertyParserConsumer+CSSPrimitiveValueResolver.cpp:
* Source/WebCore/css/parser/CSSPropertyParserConsumer+CSSPrimitiveValueResolver.h:
    - Pass `CSSCalcSymbolTable` to `doubleValue()` for resolution.

* Source/WebCore/css/parser/CSSPropertyParserConsumer+Color.cpp:
(WebCore::CSSPropertyParserHelpers::consumeAbsoluteComponent):
(WebCore::CSSPropertyParserHelpers::consumeRelativeComponent):
(WebCore::CSSPropertyParserHelpers::consumeRelativeComponents):
    - Pass both the `CSSCalcSymbolsAllowed` and `CSSCalcSymbolTable` to shared
      consume/resolve functions. This is temporary, and in a subsequent change
      resolution will be in a distinct place from consuming.

* Source/WebCore/css/parser/CSSPropertyParserConsumer+Integer.cpp:
* Source/WebCore/css/parser/CSSPropertyParserConsumer+IntegerDefinitions.h:
    - Replace `CSSCalcSymbolTable` with `CSSCalcSymbolsAllowed` for parsing,
      and pass empty values for both to combined `consumeAndResolve()`.

* Source/WebCore/css/parser/CSSPropertyParserConsumer+Length.cpp:
* Source/WebCore/css/parser/CSSPropertyParserConsumer+Length.h:
* Source/WebCore/css/parser/CSSPropertyParserConsumer+LengthDefinitions.h:
    - Replace `CSSCalcSymbolTable` with `CSSCalcSymbolsAllowed` for parsing,
      and pass empty values for both to combined `consumeAndResolve()`.

* Source/WebCore/css/parser/CSSPropertyParserConsumer+MetaConsumer.h:
(WebCore::CSSPropertyParserHelpers::MetaConsumer::consume):
    - Replace `CSSCalcSymbolTable` with `CSSCalcSymbolsAllowed` for parsing.

* Source/WebCore/css/parser/CSSPropertyParserConsumer+MetaResolver.h:
(WebCore::CSSPropertyParserHelpers::MetaResolver::consumeAndResolve):
    - Replace `CSSCalcSymbolTable` with `CSSCalcSymbolsAllowed` for parsing,
      and pass both to combined `consumeAndResolve()`.

* Source/WebCore/css/parser/CSSPropertyParserConsumer+None.cpp:
(WebCore::CSSPropertyParserHelpers::NoneKnownTokenTypeIdentConsumer::consume):
* Source/WebCore/css/parser/CSSPropertyParserConsumer+NoneDefinitions.h:
    - Replace `CSSCalcSymbolTable` with `CSSCalcSymbolsAllowed` for parsing.

* Source/WebCore/css/parser/CSSPropertyParserConsumer+Number.cpp:
* Source/WebCore/css/parser/CSSPropertyParserConsumer+NumberDefinitions.h:
    - Replace `CSSCalcSymbolTable` with `CSSCalcSymbolsAllowed` for parsing,
      and pass empty values for both to combined `consumeAndResolve()`.

* Source/WebCore/css/parser/CSSPropertyParserConsumer+Percent.cpp:
* Source/WebCore/css/parser/CSSPropertyParserConsumer+PercentDefinitions.h:
    - Replace `CSSCalcSymbolTable` with `CSSCalcSymbolsAllowed` for parsing,
      and pass empty values for both to combined `consumeAndResolve()`.

* Source/WebCore/css/parser/CSSPropertyParserConsumer+Primitives.cpp:
(WebCore::CSSPropertyParserHelpers::equal): Deleted.
* Source/WebCore/css/parser/CSSPropertyParserConsumer+Primitives.h:
    - Moves UnevaluatedCalc out to its own files.

* Source/WebCore/css/parser/CSSPropertyParserConsumer+RawResolver.cpp:
* Source/WebCore/css/parser/CSSPropertyParserConsumer+RawResolver.h:
    - Pass `CSSCalcSymbolTable` to `doubleValue()` for resolution.
    - Utilize shared support for symbol replacing via `replaceSymbol()`.
    - Utilize shared support for calc evaluation via `evaluateCalc().

* Source/WebCore/css/parser/CSSPropertyParserConsumer+RawTypes.cpp: Added
(WebCore::replaceSymbol):
    - Added shared place for direct symbol (e.g. not in a `calc()`) replacement
      via a symbol table.

* Source/WebCore/css/parser/CSSPropertyParserConsumer+RawTypes.h:
    - Moved raw types out of the CSSPropertyParserHelpers as it is now used
      beyond just the parser helpers and having that long prefix was unwieldily.

* Source/WebCore/css/parser/CSSPropertyParserConsumer+Resolution.cpp:
(WebCore::CSSPropertyParserHelpers::consumeResolution):
* Source/WebCore/css/parser/CSSPropertyParserConsumer+ResolutionDefinitions.h:
    - Replace `CSSCalcSymbolTable` with `CSSCalcSymbolsAllowed` for parsing,
      and pass empty values for both to combined `consumeAndResolve()`.

* Source/WebCore/css/parser/CSSPropertyParserConsumer+Symbol.cpp:
* Source/WebCore/css/parser/CSSPropertyParserConsumer+SymbolDefinitions.h:
    - Replace `CSSCalcSymbolTable` with `CSSCalcSymbolsAllowed` for parsing,
      and pass empty values for both to combined `consumeAndResolve()`.

* Source/WebCore/css/parser/CSSPropertyParserConsumer+Time.cpp:
* Source/WebCore/css/parser/CSSPropertyParserConsumer+Time.h:
* Source/WebCore/css/parser/CSSPropertyParserConsumer+TimeDefinitions.h:
    - Replace `CSSCalcSymbolTable` with `CSSCalcSymbolsAllowed` for parsing,
      and pass empty values for both to combined `consumeAndResolve()`.

* Source/WebCore/css/parser/CSSPropertyParserConsumer+UnevaluatedCalc.cpp: Copied from Source/WebCore/css/parser/CSSPropertyParserConsumer+Primitives.cpp.
(WebCore::CSSPropertyParserHelpers::unevaluatedCalcEqual):
(WebCore::CSSPropertyParserHelpers::unevaluatedCalcSerialization):
(WebCore::CSSPropertyParserHelpers::evaluateCalc):
* Source/WebCore/css/parser/CSSPropertyParserConsumer+UnevaluatedCalc.h: Added.
(WebCore::CSSPropertyParserHelpers::UnevaluatedCalc::operator== const):
(WebCore::CSSPropertyParserHelpers::serializationForCSS):
(WebCore::CSSPropertyParserHelpers::evaluateCalc):
    - Move UnevaluatedCalc to its own files and add support for evaluation that
      passes symbol table through and returns the correct raw type.

* Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:
    - Replace `CSSCalcSymbolTable` with `CSSCalcSymbolsAllowed` for parsing,
      and pass empty values for both to combined `consumeAndResolve()`.

* Source/WebCore/css/parser/CSSPropertyParserHelpers.h:
    - Removed CSSPropertyParserHelpers namespace prefixes from raw types.

* Source/WebCore/css/parser/CSSPropertyParserWorkerSafe.cpp:
    - Added now needed missing #include.

* Source/WebCore/css/typedom/CSSNumericValue.cpp:
(WebCore::reifyMathExpression):
(WebCore::CSSNumericValue::reifyMathExpression):
    - Fill in support for the new calc node. Currently unsupported by the spec,
      so throughs an exception.

* Source/WebCore/style/StyleResolveForFontRaw.cpp:
    - Removed CSSPropertyParserHelpers namespace prefixes from raw types.

Canonical link: https://commits.webkit.org/278635@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Support-late-resolution-of-symbols-in-CSS-calc branch from 9330e20 to c7dc51f Compare May 10, 2024 23:12
@webkit-commit-queue
Copy link
Collaborator

Committed 278635@main (c7dc51f): https://commits.webkit.org/278635@main

Reviewed commits have been landed. Closing PR #28391 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit c7dc51f into WebKit:main May 10, 2024
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label May 10, 2024
@rkirsling
Copy link
Member

Hmm, this fails to build on PlayStation with the following error:

CSSPropertyParserConsumer+CSSPrimitiveValueResolver.h:70:96: error: call to consteval function 'WebCore::computeMinimumValue' is not a constant expression
        auto value = clampTo<IntType>(std::floor(std::max(calc.calc->doubleValue(symbolTable), computeMinimumValue(integerRange)) + 0.5));
                                                                                               ^
CSSPropertyParserConsumer+CSSPrimitiveValueResolver.h:70:116: note: subexpression not valid in a constant expression
        auto value = clampTo<IntType>(std::floor(std::max(calc.calc->doubleValue(symbolTable), computeMinimumValue(integerRange)) + 0.5));
                                                                                                                   ^

I'm not sure why this would be different for us though, since this is presumably a C++20 compiler feature and not something dependent on libc++ completeness.

@rkirsling
Copy link
Member

rkirsling commented May 11, 2024

I see. The issue is that this code works with clang 10, and it works with clang 15+, but it doesn't work for any major version in between. This is...not ideal.

https://godbolt.org/z/KGW1zjaxa

@darinadler
Copy link
Member

Maybe we can find a workaround?

@rkirsling
Copy link
Member

It suffices to put consteval back to constexpr, if we're okay with that. Just seems like a shame.

@rkirsling
Copy link
Member

rkirsling commented May 11, 2024

Oh interesting. This is actually the very first usage of consteval in the codebase. Perhaps if we want to use it, we should add a macro to Compiler.h?

Edit: Opened #28428 for the imminent fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CSS Cascading Style Sheets implementation
Projects
None yet
7 participants