Skip to content

Commit

Permalink
Simplify "rollback" on optional or invalid syntax (facebook#48825)
Browse files Browse the repository at this point in the history 
Summary:

Right now we preserve the state of the CSSSyntaxParser across multiple data type parse attempts, so long that a data type parser consumes an additional component value. This requires data type parsers to be careful to not consume additional forward tokens if it may lead to parse error. We can make this model a lot simpler by instead resetting the parser to original state on data type parse error.

We also introduce `peekComponentValue`, and visitor-less `consumeComponentValue` as a convenience, to allow data type parsers to view future component values without advancing, even if the data type parser does return a value, without needing to manually clone the parser.

Changelog: [Internal]

Reviewed By: lenaic

Differential Revision: D68357624
  • Loading branch information
@NickGerleman @facebook-github-bot
NickGerleman authored and facebook-github-bot committed Jan 21, 2025
1 parent 9afad52 commit 212fdf8
Show file tree
Hide file tree
Showing 4 changed files with 168 additions and 11 deletions.
10 changes: 4 additions & 6 deletions packages/react-native/ReactCommon/react/renderer/css/CSSRatio.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,26 +35,24 @@ struct CSSDataTypeParser<CSSRatio> {
if (isValidRatioPart(token.numericValue())) {
float numerator = token.numericValue();

CSSSyntaxParser lookaheadParser{parser};

auto hasSolidus = lookaheadParser.consumeComponentValue<bool>(
auto hasSolidus = parser.peekComponentValue<bool>(
CSSComponentValueDelimiter::Whitespace,
[&](const CSSPreservedToken& token) {
return token.type() == CSSTokenType::Delim &&
token.stringValue() == "/";
});

if (!hasSolidus) {
parser = lookaheadParser;
return CSSRatio{numerator, 1.0f};
}

parser.consumeComponentValue(CSSComponentValueDelimiter::Whitespace);

auto denominator = parseNextCSSValue<CSSNumber>(
lookaheadParser, CSSComponentValueDelimiter::Whitespace);
parser, CSSComponentValueDelimiter::Whitespace);

if (std::holds_alternative<CSSNumber>(denominator) &&
isValidRatioPart(std::get<CSSNumber>(denominator).value)) {
parser = lookaheadParser;
return CSSRatio{numerator, std::get<CSSNumber>(denominator).value};
}
}
Expand Down
78 changes: 73 additions & 5 deletions packages/react-native/ReactCommon/react/renderer/css/CSSSyntaxParser.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

#include <concepts>
#include <optional>
#include <vector>

#include <react/renderer/css/CSSTokenizer.h>

Expand Down Expand Up @@ -82,7 +81,7 @@ concept CSSComponentValueVisitor = CSSFunctionVisitor<T, ReturnT> ||
*/
template <typename ReturnT, typename... VisitorsT>
concept CSSUniqueComponentValueVisitors =
(CSSComponentValueVisitor<VisitorsT, ReturnT> && ...) &&
(CSSComponentValueVisitor<VisitorsT, ReturnT> && ... && true) &&
((CSSFunctionVisitor<VisitorsT, ReturnT> ? 1 : 0) + ... + 0) <= 1 &&
((CSSPreservedTokenVisitor<VisitorsT, ReturnT> ? 1 : 0) + ... + 0) <= 1 &&
((CSSSimpleBlockVisitor<VisitorsT, ReturnT> ? 1 : 0) + ... + 0) <= 1;
Expand Down Expand Up @@ -140,17 +139,47 @@ class CSSSyntaxParser {
* @returns the visitor returned value, or a default constructed value if no
* visitor was matched, or a syntax error occurred.
*/
template <typename ReturnT>
template <typename ReturnT = std::nullptr_t>
constexpr ReturnT consumeComponentValue(
CSSComponentValueDelimiter delimiter,
const CSSComponentValueVisitor<ReturnT> auto&... visitors)
requires(CSSUniqueComponentValueVisitors<ReturnT, decltype(visitors)...>);

template <typename ReturnT>
template <typename ReturnT = std::nullptr_t>
constexpr ReturnT consumeComponentValue(
const CSSComponentValueVisitor<ReturnT> auto&... visitors)
requires(CSSUniqueComponentValueVisitors<ReturnT, decltype(visitors)...>);

/**
* Peek at the next component value without consuming it. The component value
* is provided to a passed in "visitor", typically a lambda which accepts the
* component value in a new scope. The visitor may read this component
* parameter into a higher-level data structure, and continue parsing within
* its scope using the same underlying CSSSyntaxParser.
*
* https://www.w3.org/TR/css-syntax-3/#consume-component-value
*
* @param <ReturnT> caller-specified return type of visitors. This type will
* be set to its default constructed state if consuming a component value with
* no matching visitors, or syntax error
* @param visitors A unique list of CSSComponentValueVisitor to be called on a
* match
* @param delimiter The expected delimeter to occur before the next component
* value
* @returns the visitor returned value, or a default constructed value if no
* visitor was matched, or a syntax error occurred.
*/
template <typename ReturnT = std::nullptr_t>
constexpr ReturnT peekComponentValue(
CSSComponentValueDelimiter delimiter,
const CSSComponentValueVisitor<ReturnT> auto&... visitors)
requires(CSSUniqueComponentValueVisitors<ReturnT, decltype(visitors)...>);

template <typename ReturnT = std::nullptr_t>
constexpr ReturnT peekComponentValue(
const CSSComponentValueVisitor<ReturnT> auto&... visitors)
requires(CSSUniqueComponentValueVisitors<ReturnT, decltype(visitors)...>);

/**
* The parser is considered finished when there are no more remaining tokens
* to be processed
Expand Down Expand Up @@ -262,6 +291,15 @@ struct CSSComponentValueVisitorDispatcher {
return ReturnT{};
}

constexpr ReturnT peekComponentValue(
CSSComponentValueDelimiter delimiter,
const VisitorsT&... visitors) {
auto originalParser = parser;
auto ret = consumeComponentValue(delimiter, visitors...);
parser = originalParser;
return ret;
}

constexpr std::optional<ReturnT> visitFunction(
const CSSComponentValueVisitor<ReturnT> auto& visitor,
const CSSComponentValueVisitor<ReturnT> auto&... rest) {
Expand Down Expand Up @@ -291,6 +329,11 @@ struct CSSComponentValueVisitorDispatcher {
}

constexpr std::optional<ReturnT> visitFunction() {
while (parser.peek().type() != CSSTokenType::CloseParen) {
parser.consumeToken();
}
parser.consumeToken();

return {};
}

Expand All @@ -315,7 +358,11 @@ struct CSSComponentValueVisitorDispatcher {
return visitSimpleBlock(endToken, rest...);
}

constexpr std::optional<ReturnT> visitSimpleBlock(CSSTokenType /*endToken*/) {
constexpr std::optional<ReturnT> visitSimpleBlock(CSSTokenType endToken) {
while (parser.peek().type() != endToken) {
parser.consumeToken();
}
parser.consumeToken();
return {};
}

Expand All @@ -329,6 +376,7 @@ struct CSSComponentValueVisitorDispatcher {
}

constexpr std::optional<ReturnT> visitPreservedToken() {
parser.consumeToken();
return {};
}
};
Expand All @@ -353,4 +401,24 @@ constexpr ReturnT CSSSyntaxParser::consumeComponentValue(
CSSComponentValueDelimiter::None, visitors...);
}

template <typename ReturnT>
constexpr ReturnT CSSSyntaxParser::peekComponentValue(
CSSComponentValueDelimiter delimiter,
const CSSComponentValueVisitor<ReturnT> auto&... visitors)
requires(CSSUniqueComponentValueVisitors<ReturnT, decltype(visitors)...>)
{
return CSSComponentValueVisitorDispatcher<ReturnT, decltype(visitors)...>{
*this}
.peekComponentValue(delimiter, visitors...);
}

template <typename ReturnT>
constexpr ReturnT CSSSyntaxParser::peekComponentValue(
const CSSComponentValueVisitor<ReturnT> auto&... visitors)
requires(CSSUniqueComponentValueVisitors<ReturnT, decltype(visitors)...>)
{
return peekComponentValue<ReturnT>(
CSSComponentValueDelimiter::None, visitors...);
}

} // namespace facebook::react
6 changes: 6 additions & 0 deletions packages/react-native/ReactCommon/react/renderer/css/CSSValueParser.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,11 @@ class CSSValueParser {
CSSValidDataTypeParser... RestParserT>
constexpr ReturnT tryConsumePreservedToken(const CSSPreservedToken& token) {
if constexpr (CSSPreservedTokenSink<ParserT>) {
auto currentParser = parser_;
if (auto ret = ParserT::consumePreservedToken(token, parser_)) {
return *ret;
}
parser_ = currentParser;
}

if constexpr (CSSSimplePreservedTokenSink<ParserT>) {
Expand All @@ -104,9 +106,11 @@ class CSSValueParser {
const CSSSimpleBlock& block,
CSSSyntaxParser& blockParser) {
if constexpr (CSSSimpleBlockSink<ParserT>) {
auto currentParser = blockParser;
if (auto ret = ParserT::consumeSimpleBlock(block, blockParser)) {
return *ret;
}
blockParser = currentParser;
}

return tryConsumeSimpleBlock<ReturnT, RestParserT...>(block, blockParser);
Expand All @@ -127,9 +131,11 @@ class CSSValueParser {
const CSSFunctionBlock& func,
CSSSyntaxParser& blockParser) {
if constexpr (CSSFunctionBlockSink<ParserT>) {
auto currentParser = blockParser;
if (auto ret = ParserT::consumeFunctionBlock(func, blockParser)) {
return *ret;
}
blockParser = currentParser;
}

return tryConsumeFunctionBlock<ReturnT, RestParserT...>(func, blockParser);
Expand Down
85 changes: 85 additions & 0 deletions packages/react-native/ReactCommon/react/renderer/css/tests/CSSSyntaxParserTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -392,4 +392,89 @@ TEST(CSSSyntaxParser, unconsumed_simple_block_args) {
EXPECT_EQ(funcValue, std::nullopt);
}

TEST(CSSSyntaxParser, peek_does_not_consume) {
CSSSyntaxParser parser{"foo()"};
auto funcValue = parser.peekComponentValue<std::optional<std::string_view>>(
[&](const CSSFunctionBlock& function, CSSSyntaxParser& /*blockParser*/) {
EXPECT_EQ(function.name, "foo");
return function.name;
});

EXPECT_EQ(funcValue, "foo");

auto funcValue2 = parser.peekComponentValue<std::optional<std::string_view>>(
[&](const CSSFunctionBlock& function, CSSSyntaxParser& /*blockParser*/) {
EXPECT_EQ(function.name, "foo");
return function.name;
});

EXPECT_EQ(funcValue2, "foo");

auto funcValue3 =
parser.consumeComponentValue<std::optional<std::string_view>>(
[&](const CSSFunctionBlock& function,
CSSSyntaxParser& /*blockParser*/) {
EXPECT_EQ(function.name, "foo");
return function.name;
});

EXPECT_EQ(funcValue3, "foo");

auto funcValue4 = parser.peekComponentValue<std::optional<std::string_view>>(
[&](const CSSFunctionBlock& function, CSSSyntaxParser& /*blockParser*/) {
EXPECT_EQ(function.name, "foo");
return function.name;
});

EXPECT_EQ(funcValue4, std::nullopt);
}

TEST(CSSSyntaxParser, preserved_token_without_visitor_consumed) {
CSSSyntaxParser parser{"foo bar"};

parser.consumeComponentValue();

auto identValue = parser.consumeComponentValue<std::string_view>(
CSSComponentValueDelimiter::Whitespace,
[](const CSSPreservedToken& token) {
EXPECT_EQ(token.type(), CSSTokenType::Ident);
EXPECT_EQ(token.stringValue(), "bar");
return token.stringValue();
});

EXPECT_EQ(identValue, "bar");
}

TEST(CSSSyntaxParser, function_without_visitor_consumed) {
CSSSyntaxParser parser{"foo(a, b, c) bar"};

parser.consumeComponentValue();

auto identValue = parser.consumeComponentValue<std::string_view>(
CSSComponentValueDelimiter::Whitespace,
[](const CSSPreservedToken& token) {
EXPECT_EQ(token.type(), CSSTokenType::Ident);
EXPECT_EQ(token.stringValue(), "bar");
return token.stringValue();
});

EXPECT_EQ(identValue, "bar");
}

TEST(CSSSyntaxParser, simple_block_without_visitor_consumed) {
CSSSyntaxParser parser{"{a foo(abc)} bar"};

parser.consumeComponentValue();

auto identValue = parser.consumeComponentValue<std::string_view>(
CSSComponentValueDelimiter::Whitespace,
[](const CSSPreservedToken& token) {
EXPECT_EQ(token.type(), CSSTokenType::Ident);
EXPECT_EQ(token.stringValue(), "bar");
return token.stringValue();
});

EXPECT_EQ(identValue, "bar");
}

} // namespace facebook::react

0 comments on commit 212fdf8

Please sign in to comment.