Skip to content

Commit

Permalink
[skip ci] Do not consume component value on visitor failure (facebook…
Browse files Browse the repository at this point in the history 
…#48985)

Summary:

This reverts some of the behavior I added in D68357624, since peeking a component value is non-obviously more expensive than manually copying the parser, and needing to peek will be a pain for flat lists of values (like for box-shadow).

Changelog: [internal]

Differential Revision: D68733518
  • Loading branch information
@NickGerleman @facebook-github-bot
NickGerleman authored and facebook-github-bot committed Feb 3, 2025
1 parent 84540f8 commit 8c73665
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 167 deletions.
123 changes: 38 additions & 85 deletions packages/react-native/ReactCommon/react/renderer/css/CSSSyntaxParser.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,21 @@ struct CSSSimpleBlock {
CSSTokenType openBracketType{};
};

/**
* Describes a valid return type for a CSSSyntaxParser visitor
*/
template <typename T>
concept CSSSyntaxVisitorReturn =
std::is_default_constructible_v<T> && std::equality_comparable<T>;

/**
* A CSSFunctionVisitor is called to start parsing a function component value.
* At this point, the Parser is positioned at the start of the function
* component value list. It is expected that the visitor finishes before the end
* of the function block.
*/
template <typename T, typename ReturnT>
concept CSSFunctionVisitor =
concept CSSFunctionVisitor = CSSSyntaxVisitorReturn<ReturnT> &&
requires(T visitor, CSSFunctionBlock func, CSSSyntaxParser& blockParser) {
{ visitor(func, blockParser) } -> std::convertible_to<ReturnT>;
};
Expand All @@ -52,7 +59,7 @@ concept CSSFunctionVisitor =
* component value.
*/
template <typename T, typename ReturnT>
concept CSSPreservedTokenVisitor =
concept CSSPreservedTokenVisitor = CSSSyntaxVisitorReturn<ReturnT> &&
requires(T visitor, CSSPreservedToken token) {
{ visitor(token) } -> std::convertible_to<ReturnT>;
};
Expand All @@ -63,7 +70,7 @@ concept CSSPreservedTokenVisitor =
* of the block.
*/
template <typename T, typename ReturnT>
concept CSSSimpleBlockVisitor =
concept CSSSimpleBlockVisitor = CSSSyntaxVisitorReturn<ReturnT> &&
requires(T visitor, CSSSimpleBlock block, CSSSyntaxParser& blockParser) {
{ visitor(block, blockParser) } -> std::convertible_to<ReturnT>;
};
Expand All @@ -72,16 +79,17 @@ concept CSSSimpleBlockVisitor =
* Any visitor for a component value.
*/
template <typename T, typename ReturnT>
concept CSSComponentValueVisitor = CSSFunctionVisitor<T, ReturnT> ||
CSSPreservedTokenVisitor<T, ReturnT> || CSSSimpleBlockVisitor<T, ReturnT>;
concept CSSComponentValueVisitor = CSSSyntaxVisitorReturn<ReturnT> &&
(CSSFunctionVisitor<T, ReturnT> || CSSPreservedTokenVisitor<T, ReturnT> ||
CSSSimpleBlockVisitor<T, ReturnT>);

/**
* Represents a variadic set of CSSComponentValueVisitor with no more than one
* of a specific type of visitor.
*/
template <typename ReturnT, typename... VisitorsT>
concept CSSUniqueComponentValueVisitors =
(CSSComponentValueVisitor<VisitorsT, ReturnT> && ... && true) &&
concept CSSUniqueComponentValueVisitors = CSSSyntaxVisitorReturn<ReturnT> &&
(CSSComponentValueVisitor<VisitorsT, ReturnT> && ...) &&
((CSSFunctionVisitor<VisitorsT, ReturnT> ? 1 : 0) + ... + 0) <= 1 &&
((CSSPreservedTokenVisitor<VisitorsT, ReturnT> ? 1 : 0) + ... + 0) <= 1 &&
((CSSSimpleBlockVisitor<VisitorsT, ReturnT> ? 1 : 0) + ... + 0) <= 1;
Expand All @@ -106,7 +114,9 @@ enum class CSSDelimiter {
* https://www.w3.org/TR/css-syntax-3/#component-value
*/
class CSSSyntaxParser {
template <typename ReturnT, CSSComponentValueVisitor<ReturnT>... VisitorsT>
template <
CSSSyntaxVisitorReturn ReturnT,
CSSComponentValueVisitor<ReturnT>... VisitorsT>
friend struct CSSComponentValueVisitorDispatcher;

public:
Expand All @@ -130,6 +140,11 @@ class CSSSyntaxParser {
* higher-level data structure, and continue parsing within its scope using
* the same underlying CSSSyntaxParser.
*
* The state of the parser is reset if a visitor returns a default-constructed
* value for the given return type, even if it previously advanced the parser.
* If no visitor returns a non-default-constructed value, the component value
* will not be consumed.
*
* https://www.w3.org/TR/css-syntax-3/#consume-component-value
*
* @param <ReturnT> caller-specified return type of visitors. This type will
Expand All @@ -142,47 +157,16 @@ 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 = std::nullptr_t>
template <CSSSyntaxVisitorReturn ReturnT>
constexpr ReturnT consumeComponentValue(
CSSDelimiter delimiter,
const CSSComponentValueVisitor<ReturnT> auto&... visitors)
requires(CSSUniqueComponentValueVisitors<ReturnT, decltype(visitors)...>);

template <typename ReturnT = std::nullptr_t>
template <CSSSyntaxVisitorReturn ReturnT>
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(
CSSDelimiter 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 @@ -226,7 +210,9 @@ class CSSSyntaxParser {
CSSTokenType terminator_{CSSTokenType::EndOfFile};
};

template <typename ReturnT, CSSComponentValueVisitor<ReturnT>... VisitorsT>
template <
CSSSyntaxVisitorReturn ReturnT,
CSSComponentValueVisitor<ReturnT>... VisitorsT>
struct CSSComponentValueVisitorDispatcher {
CSSSyntaxParser& parser;

Expand Down Expand Up @@ -275,6 +261,7 @@ struct CSSComponentValueVisitorDispatcher {
break;
}

parser = originalParser;
return ReturnT{};
}

Expand Down Expand Up @@ -331,15 +318,6 @@ struct CSSComponentValueVisitorDispatcher {
return false;
}

constexpr ReturnT peekComponentValue(
CSSDelimiter 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 All @@ -357,7 +335,8 @@ struct CSSComponentValueVisitorDispatcher {
auto functionValue = visitor({name}, blockParser);
parser.advanceToBlockParser(blockParser);
parser.consumeWhitespace();
if (parser.peek().type() == CSSTokenType::CloseParen) {
if (parser.peek().type() == CSSTokenType::CloseParen &&
functionValue != ReturnT{}) {
parser.consumeToken();
return functionValue;
}
Expand All @@ -369,11 +348,6 @@ struct CSSComponentValueVisitorDispatcher {
}

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

return {};
}

Expand All @@ -388,7 +362,7 @@ struct CSSComponentValueVisitorDispatcher {
auto blockValue = visitor({openBracketType}, blockParser);
parser.advanceToBlockParser(blockParser);
parser.consumeWhitespace();
if (parser.peek().type() == endToken) {
if (parser.peek().type() == endToken && blockValue != ReturnT{}) {
parser.consumeToken();
return blockValue;
}
Expand All @@ -399,29 +373,27 @@ struct CSSComponentValueVisitorDispatcher {
}

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

constexpr std::optional<ReturnT> visitPreservedToken(
const CSSComponentValueVisitor<ReturnT> auto& visitor,
const CSSComponentValueVisitor<ReturnT> auto&... rest) {
if constexpr (CSSPreservedTokenVisitor<decltype(visitor), ReturnT>) {
return visitor(parser.consumeToken());
auto ret = visitor(parser.consumeToken());
if (ret != ReturnT{}) {
return ret;
}
}
return visitPreservedToken(rest...);
}

constexpr std::optional<ReturnT> visitPreservedToken() {
parser.consumeToken();
return {};
}
};

template <typename ReturnT>
template <CSSSyntaxVisitorReturn ReturnT>
constexpr ReturnT CSSSyntaxParser::consumeComponentValue(
CSSDelimiter delimiter,
const CSSComponentValueVisitor<ReturnT> auto&... visitors)
Expand All @@ -432,31 +404,12 @@ constexpr ReturnT CSSSyntaxParser::consumeComponentValue(
.consumeComponentValue(delimiter, visitors...);
}

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

template <typename ReturnT>
constexpr ReturnT CSSSyntaxParser::peekComponentValue(
CSSDelimiter 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>(CSSDelimiter::None, visitors...);
}

} // namespace facebook::react
117 changes: 35 additions & 82 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 @@ -382,88 +382,6 @@ 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>(
CSSDelimiter::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>(
CSSDelimiter::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>(
CSSDelimiter::Whitespace, [](const CSSPreservedToken& token) {
EXPECT_EQ(token.type(), CSSTokenType::Ident);
EXPECT_EQ(token.stringValue(), "bar");
return token.stringValue();
});

EXPECT_EQ(identValue, "bar");
}

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

Expand Down Expand Up @@ -606,4 +524,39 @@ TEST(CSSSyntaxParser, delimeter_not_consumed_when_no_component_value) {
EXPECT_TRUE(hasComma);
}

TEST(CSSSyntaxParser, component_value_not_consumed_on_visitor_failure) {
CSSSyntaxParser parser{"foo"};

bool visitor1Attempted = false;
bool visitor1Ret =
parser.consumeComponentValue<bool>([&](const CSSPreservedToken& token) {
EXPECT_EQ(token.stringValue(), "foo");
visitor1Attempted = true;
return false;
});

EXPECT_TRUE(visitor1Attempted);
EXPECT_FALSE(visitor1Ret);

bool visitor2Attempted = false;
parser.consumeComponentValue<bool>([&](const CSSPreservedToken& token) {
EXPECT_EQ(token.stringValue(), "foo");
visitor2Attempted = true;
return true;
});

EXPECT_TRUE(visitor2Attempted);
EXPECT_TRUE(visitor2Attempted);

bool visitor3Attempted = false;
bool visitor3Ret =
parser.consumeComponentValue<bool>([&](const CSSPreservedToken& token) {
visitor3Attempted = true;
return true;
});

EXPECT_FALSE(visitor3Attempted);
EXPECT_FALSE(visitor3Ret);
}

} // namespace facebook::react

0 comments on commit 8c73665

Please sign in to comment.