Diff
Modified: trunk/Source/WebCore/ChangeLog (248649 => 248650)
--- trunk/Source/WebCore/ChangeLog 2019-08-14 01:03:08 UTC (rev 248649)
+++ trunk/Source/WebCore/ChangeLog 2019-08-14 01:05:27 UTC (rev 248650)
@@ -1,3 +1,34 @@
+2019-08-13 Robin Morisset <rmoris...@apple.com>
+
+ [WHLSL] Don't generate empty comma expressions for bare ';'
+ https://bugs.webkit.org/show_bug.cgi?id=200681
+
+ Reviewed by Myles C. Maxfield.
+
+ Currently we emit a comma _expression_ with no sub-_expression_ for bare ';', as well as for the initialization of for loops with no initializers.
+ This crashes the Checker, as it tries to access the last sub-_expression_ of comma expressions.
+ Instead we should generate an empty statement block for that case.
+
+ This problem was found (and originally fixed before the commit was reverted) in https://bugs.webkit.org/show_bug.cgi?id=199726.
+ I am just isolating the fix here for easier review and debugging.
+
+ New test: LayoutTests/webgpu/whlsl/for-loop.html
+
+ * Modules/webgpu/WHLSL/AST/WHLSLForLoop.h:
+ * Modules/webgpu/WHLSL/Metal/WHLSLFunctionWriter.cpp:
+ (WebCore::WHLSL::Metal::FunctionDefinitionWriter::visit):
+ * Modules/webgpu/WHLSL/WHLSLASTDumper.cpp:
+ (WebCore::WHLSL::ASTDumper::visit):
+ * Modules/webgpu/WHLSL/WHLSLChecker.cpp:
+ (WebCore::WHLSL::Checker::visit):
+ * Modules/webgpu/WHLSL/WHLSLParser.cpp:
+ (WebCore::WHLSL::Parser::parseForLoop):
+ (WebCore::WHLSL::Parser::parseStatement):
+ (WebCore::WHLSL::Parser::parseEffectfulExpression):
+ * Modules/webgpu/WHLSL/WHLSLParser.h:
+ * Modules/webgpu/WHLSL/WHLSLVisitor.cpp:
+ (WebCore::WHLSL::Visitor::visit):
+
2019-08-13 Daniel Bates <daba...@apple.com>
Focus rings are black
Modified: trunk/Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLForLoop.h (248649 => 248650)
--- trunk/Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLForLoop.h 2019-08-14 01:03:08 UTC (rev 248649)
+++ trunk/Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLForLoop.h 2019-08-14 01:05:27 UTC (rev 248650)
@@ -45,7 +45,7 @@
class ForLoop final : public Statement {
WTF_MAKE_FAST_ALLOCATED;
public:
- ForLoop(CodeLocation location, Variant<UniqueRef<Statement>, UniqueRef<_expression_>>&& initialization, std::unique_ptr<_expression_>&& condition, std::unique_ptr<_expression_>&& increment, UniqueRef<Statement>&& body)
+ ForLoop(CodeLocation location, UniqueRef<Statement>&& initialization, std::unique_ptr<_expression_>&& condition, std::unique_ptr<_expression_>&& increment, UniqueRef<Statement>&& body)
: Statement(location, Kind::ForLoop)
, m_initialization(WTFMove(initialization))
, m_condition(WTFMove(condition))
@@ -59,13 +59,13 @@
ForLoop(const ForLoop&) = delete;
ForLoop(ForLoop&&) = default;
- Variant<UniqueRef<Statement>, UniqueRef<_expression_>>& initialization() { return m_initialization; }
+ UniqueRef<Statement>& initialization() { return m_initialization; }
_expression_* condition() { return m_condition.get(); }
_expression_* increment() { return m_increment.get(); }
Statement& body() { return m_body; }
private:
- Variant<UniqueRef<Statement>, UniqueRef<_expression_>> m_initialization;
+ UniqueRef<Statement> m_initialization;
std::unique_ptr<_expression_> m_condition;
std::unique_ptr<_expression_> m_increment;
UniqueRef<Statement> m_body;
Modified: trunk/Source/WebCore/Modules/webgpu/WHLSL/Metal/WHLSLFunctionWriter.cpp (248649 => 248650)
--- trunk/Source/WebCore/Modules/webgpu/WHLSL/Metal/WHLSLFunctionWriter.cpp 2019-08-14 01:03:08 UTC (rev 248649)
+++ trunk/Source/WebCore/Modules/webgpu/WHLSL/Metal/WHLSLFunctionWriter.cpp 2019-08-14 01:05:27 UTC (rev 248650)
@@ -356,14 +356,7 @@
void FunctionDefinitionWriter::visit(AST::ForLoop& forLoop)
{
m_stringBuilder.append("{\n");
-
- WTF::visit(WTF::makeVisitor([&](AST::Statement& statement) {
- checkErrorAndVisit(statement);
- }, [&](UniqueRef<AST::_expression_>& _expression_) {
- checkErrorAndVisit(_expression_);
- takeLastValue(); // We don't need to do anything with the result.
- }), forLoop.initialization());
-
+ checkErrorAndVisit(forLoop.initialization());
emitLoop(LoopConditionLocation::BeforeBody, forLoop.condition(), forLoop.increment(), forLoop.body());
m_stringBuilder.append("}\n");
}
Modified: trunk/Source/WebCore/Modules/webgpu/WHLSL/WHLSLASTDumper.cpp (248649 => 248650)
--- trunk/Source/WebCore/Modules/webgpu/WHLSL/WHLSLASTDumper.cpp 2019-08-14 01:03:08 UTC (rev 248649)
+++ trunk/Source/WebCore/Modules/webgpu/WHLSL/WHLSLASTDumper.cpp 2019-08-14 01:05:27 UTC (rev 248650)
@@ -409,11 +409,7 @@
void ASTDumper::visit(AST::ForLoop& forLoop)
{
m_out.print("for (");
- WTF::visit(WTF::makeVisitor([&](UniqueRef<AST::Statement>& statement) {
- visit(statement);
- }, [&](UniqueRef<AST::_expression_>& _expression_) {
- visit(_expression_);
- }), forLoop.initialization());
+ visit(forLoop.initialization());
m_out.print("; ");
if (forLoop.condition())
visit(*forLoop.condition());
Modified: trunk/Source/WebCore/Modules/webgpu/WHLSL/WHLSLChecker.cpp (248649 => 248650)
--- trunk/Source/WebCore/Modules/webgpu/WHLSL/WHLSLChecker.cpp 2019-08-14 01:03:08 UTC (rev 248649)
+++ trunk/Source/WebCore/Modules/webgpu/WHLSL/WHLSLChecker.cpp 2019-08-14 01:05:27 UTC (rev 248650)
@@ -1443,11 +1443,7 @@
void Checker::visit(AST::ForLoop& forLoop)
{
- WTF::visit(WTF::makeVisitor([&](UniqueRef<AST::Statement>& statement) {
- checkErrorAndVisit(statement);
- }, [&](UniqueRef<AST::_expression_>& _expression_) {
- checkErrorAndVisit(_expression_);
- }), forLoop.initialization());
+ checkErrorAndVisit(forLoop.initialization());
if (hasError())
return;
if (forLoop.condition()) {
Modified: trunk/Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp (248649 => 248650)
--- trunk/Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp 2019-08-14 01:03:08 UTC (rev 248649)
+++ trunk/Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp 2019-08-14 01:05:27 UTC (rev 248650)
@@ -1163,7 +1163,7 @@
CONSUME_TYPE(origin, For);
CONSUME_TYPE(leftParenthesis, LeftParenthesis);
- auto parseRemainder = [&](Variant<UniqueRef<AST::Statement>, UniqueRef<AST::_expression_>>&& initialization) -> Expected<AST::ForLoop, Error> {
+ auto parseRemainder = [&](UniqueRef<AST::Statement>&& initialization) -> Expected<AST::ForLoop, Error> {
CONSUME_TYPE(semicolon, Semicolon);
std::unique_ptr<AST::_expression_> condition(nullptr);
@@ -1356,13 +1356,13 @@
}
{
- auto effectfulExpression = backtrackingScope<Expected<UniqueRef<AST::_expression_>, Error>>([&]() -> Expected<UniqueRef<AST::_expression_>, Error> {
+ auto effectfulExpression = backtrackingScope<Expected<UniqueRef<AST::Statement>, Error>>([&]() -> Expected<UniqueRef<AST::Statement>, Error> {
PARSE(result, EffectfulExpression);
CONSUME_TYPE(semicolon, Semicolon);
return result;
});
if (effectfulExpression)
- return { makeUniqueRef<AST::EffectfulExpressionStatement>(WTFMove(*effectfulExpression)) };
+ return WTFMove(*effectfulExpression);
}
PARSE(variableDeclarations, VariableDeclarations);
@@ -1370,13 +1370,13 @@
return { makeUniqueRef<AST::VariableDeclarationsStatement>(WTFMove(*variableDeclarations)) };
}
-auto Parser::parseEffectfulExpression() -> Expected<UniqueRef<AST::_expression_>, Error>
+auto Parser::parseEffectfulExpression() -> Expected<UniqueRef<AST::Statement>, Error>
{
PEEK(origin);
- Vector<UniqueRef<AST::_expression_>> expressions;
if (origin->type == Token::Type::Semicolon)
- return { makeUniqueRef<AST::CommaExpression>(*origin, WTFMove(expressions)) };
+ return { makeUniqueRef<AST::Block>(*origin, Vector<UniqueRef<AST::Statement>>()) };
+ Vector<UniqueRef<AST::_expression_>> expressions;
PARSE(effectfulExpression, EffectfulAssignment);
expressions.append(WTFMove(*effectfulExpression));
@@ -1386,10 +1386,11 @@
}
if (expressions.size() == 1)
- return WTFMove(expressions[0]);
+ return { makeUniqueRef<AST::EffectfulExpressionStatement>(WTFMove(expressions[0])) };
unsigned endOffset = m_lexer.peek().startOffset();
CodeLocation location(origin->startOffset(), endOffset);
- return { makeUniqueRef<AST::CommaExpression>(location, WTFMove(expressions)) };
+ auto commaExpression = makeUniqueRef<AST::CommaExpression>(location, WTFMove(expressions));
+ return { makeUniqueRef<AST::EffectfulExpressionStatement>(WTFMove(commaExpression)) };
}
auto Parser::parseEffectfulAssignment() -> Expected<UniqueRef<AST::_expression_>, Error>
Modified: trunk/Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.h (248649 => 248650)
--- trunk/Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.h 2019-08-14 01:03:08 UTC (rev 248649)
+++ trunk/Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.h 2019-08-14 01:05:27 UTC (rev 248650)
@@ -131,7 +131,7 @@
Expected<AST::VariableDeclarationsStatement, Error> parseVariableDeclarations();
Expected<UniqueRef<AST::Statement>, Error> parseStatement();
- Expected<UniqueRef<AST::_expression_>, Error> parseEffectfulExpression();
+ Expected<UniqueRef<AST::Statement>, Error> parseEffectfulExpression();
Expected<UniqueRef<AST::_expression_>, Error> parseEffectfulAssignment();
struct SuffixExpression {
SuffixExpression(UniqueRef<AST::_expression_>&& result, bool success)
Modified: trunk/Source/WebCore/Modules/webgpu/WHLSL/WHLSLVisitor.cpp (248649 => 248650)
--- trunk/Source/WebCore/Modules/webgpu/WHLSL/WHLSLVisitor.cpp 2019-08-14 01:03:08 UTC (rev 248649)
+++ trunk/Source/WebCore/Modules/webgpu/WHLSL/WHLSLVisitor.cpp 2019-08-14 01:05:27 UTC (rev 248650)
@@ -461,11 +461,7 @@
void Visitor::visit(AST::ForLoop& forLoop)
{
- WTF::visit(WTF::makeVisitor([&](UniqueRef<AST::Statement>& statement) {
- checkErrorAndVisit(statement);
- }, [&](UniqueRef<AST::_expression_>& _expression_) {
- checkErrorAndVisit(_expression_);
- }), forLoop.initialization());
+ checkErrorAndVisit(forLoop.initialization());
if (forLoop.condition())
checkErrorAndVisit(*forLoop.condition());
if (forLoop.increment())
