From effd0aeccd603246d8167179da0d2c23117d6bae Mon Sep 17 00:00:00 2001 From: John Stiles Date: Thu, 19 May 2022 10:13:36 -0400 Subject: [PATCH] Reland "Remove DSLWrapper helper class." This reverts commit 37c03c8d73c45ba425203d0debea41f4506ab320. Reason for revert: using vector<> to avoid skia:13339 Original change's description: > Revert "Remove DSLWrapper helper class." > > This reverts commit 3b4f862d05a0ba4a3b1d2cd92a1b95963f0ca885. > > Reason for revert: might have caused chromium:1326848 > > Original change's description: > > Remove DSLWrapper helper class. > > > > Without an `operator=` on our expressions and variables, we no longer > > need to wrap all our expressions with a helper. > > > > Change-Id: I8110079f61c9ad01997f7c4b376db223dc4b6e17 > > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/541063 > > Commit-Queue: John Stiles > > Auto-Submit: John Stiles > > Reviewed-by: Brian Osman > > Change-Id: I7efb3004913f7c85dc551d9740a6b31971de52d2 > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/541736 > Auto-Submit: John Stiles > Bot-Commit: Rubber Stamper > Commit-Queue: Rubber Stamper Change-Id: If6dd5d3187a4623ae732335b8215f4350e45cd2b Reviewed-on: https://skia-review.googlesource.com/c/skia/+/542137 Commit-Queue: John Stiles Reviewed-by: Brian Osman --- include/sksl/BUILD.bazel | 6 --- include/sksl/DSL.h | 1 - include/sksl/DSLExpression.h | 14 ++----- include/sksl/DSLFunction.h | 4 +- include/sksl/DSLWrapper.h | 77 ---------------------------------- public.bzl | 1 - src/sksl/BUILD.bazel | 1 - src/sksl/SkSLDSLParser.cpp | 20 +++++---- src/sksl/SkSLDSLParser.h | 3 +- src/sksl/dsl/BUILD.bazel | 2 - src/sksl/dsl/DSLExpression.cpp | 10 ++--- src/sksl/dsl/DSLFunction.cpp | 7 ++-- tests/BUILD.bazel | 1 - tests/SkSLDSLTest.cpp | 18 +------- 14 files changed, 26 insertions(+), 139 deletions(-) delete mode 100644 include/sksl/DSLWrapper.h diff --git a/include/sksl/BUILD.bazel b/include/sksl/BUILD.bazel index 8b4cf8c832..c6cde3720d 100644 --- a/include/sksl/BUILD.bazel +++ b/include/sksl/BUILD.bazel @@ -136,12 +136,6 @@ generated_cc_atom( ], ) -generated_cc_atom( - name = "DSLWrapper_hdr", - hdrs = ["DSLWrapper.h"], - visibility = ["//:__subpackages__"], -) - generated_cc_atom( name = "DSL_hdr", hdrs = ["DSL.h"], diff --git a/include/sksl/DSL.h b/include/sksl/DSL.h index 1628029406..6b9ebd4727 100644 --- a/include/sksl/DSL.h +++ b/include/sksl/DSL.h @@ -29,7 +29,6 @@ using Modifiers = DSLModifiers; using Parameter = DSLParameter; using Statement = DSLStatement; using Var = DSLVar; -template using Wrapper = DSLWrapper; } // namespace dsl diff --git a/include/sksl/DSLExpression.h b/include/sksl/DSLExpression.h index 2c75c2e2ed..d145c77cd8 100644 --- a/include/sksl/DSLExpression.h +++ b/include/sksl/DSLExpression.h @@ -33,7 +33,6 @@ namespace dsl { class DSLPossibleExpression; class DSLType; class DSLVarBase; -template class DSLWrapper; /** * Represents an expression such as 'cos(x)' or 'a + b'. @@ -134,11 +133,9 @@ public: */ DSLPossibleExpression operator[](DSLExpression index); - DSLPossibleExpression operator()(SkTArray> args, - Position pos = {}); + DSLPossibleExpression operator()(SkTArray args, Position pos = {}); - DSLPossibleExpression operator()(ExpressionArray args, - Position pos = {}); + DSLPossibleExpression operator()(ExpressionArray args, Position pos = {}); /** * Invokes a prefix operator. @@ -198,7 +195,6 @@ private: friend class DSLType; friend class DSLVarBase; friend class DSLWriter; - template friend class DSLWrapper; }; DSLPossibleExpression operator+(DSLExpression left, DSLExpression right); @@ -303,11 +299,9 @@ public: DSLPossibleExpression operator[](DSLExpression index); - DSLPossibleExpression operator()(SkTArray> args, - Position pos = {}); + DSLPossibleExpression operator()(SkTArray args, Position pos = {}); - DSLPossibleExpression operator()(ExpressionArray args, - Position pos = {}); + DSLPossibleExpression operator()(ExpressionArray args, Position pos = {}); DSLPossibleExpression operator++(); diff --git a/include/sksl/DSLFunction.h b/include/sksl/DSLFunction.h index 02960716f0..fc81d814cb 100644 --- a/include/sksl/DSLFunction.h +++ b/include/sksl/DSLFunction.h @@ -27,7 +27,6 @@ class FunctionDeclaration; namespace dsl { class DSLType; -template class DSLWrapper; class DSLFunction { public: @@ -84,8 +83,7 @@ public: /** * Invokes the function with the given arguments. */ - DSLExpression call(SkTArray> args, - Position pos = {}); + DSLExpression call(SkTArray args, Position pos = {}); DSLExpression call(ExpressionArray args, Position pos = {}); diff --git a/include/sksl/DSLWrapper.h b/include/sksl/DSLWrapper.h deleted file mode 100644 index 96daa22dd8..0000000000 --- a/include/sksl/DSLWrapper.h +++ /dev/null @@ -1,77 +0,0 @@ -/* - * Copyright 2021 Google LLC. - * - * Use of this source code is governed by a BSD-style license that can be - * found in the LICENSE file. - */ - -#ifndef SKSL_DSL_WRAPPER -#define SKSL_DSL_WRAPPER - -#include - -namespace SkSL { - -namespace dsl { - -/** - * Several of the DSL classes override operator= in a non-standard fashion to allow for expressions - * like "x = 0" to compile into SkSL code. This makes it impossible to directly use these classes in - * C++ containers which expect standard behavior for operator=. - * - * Wrapper contains a T, where T is a DSL class with non-standard operator=, and provides - * standard behavior for operator=, permitting it to be used in standard containers. - */ -template -class DSLWrapper { -public: - DSLWrapper(T value) { - fValue.swap(value); - } - - DSLWrapper(const DSLWrapper&) = delete; - - DSLWrapper(DSLWrapper&& other) { - fValue.swap(other.fValue); - } - - T& get() { - return fValue; - } - - T& operator*() { - return fValue; - } - - T* operator->() { - return &fValue; - } - - const T& get() const { - return fValue; - } - - const T& operator*() const { - return fValue; - } - - const T* operator->() const { - return &fValue; - } - - DSLWrapper& operator=(const DSLWrapper&) = delete; - - DSLWrapper& operator=(DSLWrapper&& other) { - fValue.swap(other.fValue); - return *this; - } - -private: - T fValue; -}; - -} // namespace dsl - -} // namespace SkSL - -#endif diff --git a/public.bzl b/public.bzl index 3b5f7e8c38..d9f7059022 100644 --- a/public.bzl +++ b/public.bzl @@ -245,7 +245,6 @@ SKIA_PUBLIC_HDRS = [ "include/sksl/DSLSymbols.h", "include/sksl/DSLType.h", "include/sksl/DSLVar.h", - "include/sksl/DSLWrapper.h", "include/sksl/SkSLDebugTrace.h", "include/sksl/SkSLErrorReporter.h", "include/sksl/SkSLOperator.h", diff --git a/src/sksl/BUILD.bazel b/src/sksl/BUILD.bazel index aafbb3758d..604c075ccd 100644 --- a/src/sksl/BUILD.bazel +++ b/src/sksl/BUILD.bazel @@ -427,7 +427,6 @@ generated_cc_atom( "//include/sksl:DSLFunction_hdr", "//include/sksl:DSLSymbols_hdr", "//include/sksl:DSLVar_hdr", - "//include/sksl:DSLWrapper_hdr", "//include/sksl:DSL_hdr", "//include/sksl:SkSLOperator_hdr", "//include/sksl:SkSLVersion_hdr", diff --git a/src/sksl/SkSLDSLParser.cpp b/src/sksl/SkSLDSLParser.cpp index 89a25179f3..eed0c69bb6 100644 --- a/src/sksl/SkSLDSLParser.cpp +++ b/src/sksl/SkSLDSLParser.cpp @@ -18,7 +18,6 @@ #include "include/sksl/DSLFunction.h" #include "include/sksl/DSLSymbols.h" #include "include/sksl/DSLVar.h" -#include "include/sksl/DSLWrapper.h" #include "include/sksl/SkSLOperator.h" #include "include/sksl/SkSLVersion.h" #include "src/sksl/SkSLCompiler.h" @@ -416,7 +415,8 @@ bool DSLParser::functionDeclarationEnd(Position start, const DSLModifiers& modifiers, DSLType type, const Token& name) { - SkTArray> parameters; + // TODO(skia:13339): use SkSTArray<8, DSLParameter> once SkTArray is go/cfi compatible + std::vector parameters; Token lookahead = this->peek(); if (lookahead.fKind == Token::Kind::TK_RPAREN) { // `()` means no parameters at all. @@ -424,9 +424,10 @@ bool DSLParser::functionDeclarationEnd(Position start, // `(void)` also means no parameters at all. this->nextToken(); } else { + parameters.reserve(8); for (;;) { size_t paramIndex = parameters.size(); - std::optional> parameter = this->parameter(paramIndex); + std::optional parameter = this->parameter(paramIndex); if (!parameter) { return false; } @@ -439,12 +440,13 @@ bool DSLParser::functionDeclarationEnd(Position start, if (!this->expect(Token::Kind::TK_RPAREN, "')'")) { return false; } - SkTArray parameterPointers; - for (DSLWrapper& param : parameters) { - parameterPointers.push_back(¶m.get()); + SkSTArray<8, DSLParameter*> parameterPointers; + parameterPointers.reserve_back(parameters.size()); + for (DSLParameter& param : parameters) { + parameterPointers.push_back(¶m); } DSLFunction result(modifiers, type, this->text(name), parameterPointers, - this->rangeFrom(start)); + this->rangeFrom(start)); if (!this->checkNext(Token::Kind::TK_SEMICOLON)) { AutoDSLSymbolTable symbols; for (DSLParameter* var : parameterPointers) { @@ -740,7 +742,7 @@ SkTArray DSLParser::structVarDeclaration(Position start, } /* modifiers type IDENTIFIER (LBRACKET INT_LITERAL RBRACKET)? */ -std::optional> DSLParser::parameter(size_t paramIndex) { +std::optional DSLParser::parameter(size_t paramIndex) { Position pos = this->position(this->peek()); DSLModifiers modifiers = this->modifiers(); std::optional type = this->type(&modifiers); @@ -761,7 +763,7 @@ std::optional> DSLParser::parameter(size_t paramIndex) if (!this->parseArrayDimensions(pos, &type.value())) { return std::nullopt; } - return {{DSLParameter(modifiers, *type, paramText, this->rangeFrom(pos), paramPos)}}; + return DSLParameter(modifiers, *type, paramText, this->rangeFrom(pos), paramPos); } /** EQ INT_LITERAL */ diff --git a/src/sksl/SkSLDSLParser.h b/src/sksl/SkSLDSLParser.h index e732e4df2a..008c5f63c8 100644 --- a/src/sksl/SkSLDSLParser.h +++ b/src/sksl/SkSLDSLParser.h @@ -40,7 +40,6 @@ class DSLBlock; class DSLCase; class DSLGlobalVar; class DSLParameter; -template class DSLWrapper; } @@ -177,7 +176,7 @@ private: dsl::DSLStatement localVarDeclarationEnd(Position position, const dsl::DSLModifiers& mods, dsl::DSLType baseType, Token name); - std::optional> parameter(size_t paramIndex); + std::optional parameter(size_t paramIndex); int layoutInt(); diff --git a/src/sksl/dsl/BUILD.bazel b/src/sksl/dsl/BUILD.bazel index a12417479b..b71153b088 100644 --- a/src/sksl/dsl/BUILD.bazel +++ b/src/sksl/dsl/BUILD.bazel @@ -87,7 +87,6 @@ generated_cc_atom( "//include/sksl:DSLExpression_hdr", "//include/sksl:DSLType_hdr", "//include/sksl:DSLVar_hdr", - "//include/sksl:DSLWrapper_hdr", "//include/sksl:SkSLOperator_hdr", "//src/sksl:SkSLThreadContext_hdr", "//src/sksl/dsl/priv:DSLWriter_hdr", @@ -117,7 +116,6 @@ generated_cc_atom( "//include/sksl:DSLFunction_hdr", "//include/sksl:DSLType_hdr", "//include/sksl:DSLVar_hdr", - "//include/sksl:DSLWrapper_hdr", "//src/sksl:SkSLProgramSettings_hdr", "//src/sksl:SkSLThreadContext_hdr", "//src/sksl/dsl/priv:DSLWriter_hdr", diff --git a/src/sksl/dsl/DSLExpression.cpp b/src/sksl/dsl/DSLExpression.cpp index 012bfa0707..c64c821aa1 100644 --- a/src/sksl/dsl/DSLExpression.cpp +++ b/src/sksl/dsl/DSLExpression.cpp @@ -12,7 +12,6 @@ #include "include/sksl/DSLCore.h" #include "include/sksl/DSLType.h" #include "include/sksl/DSLVar.h" -#include "include/sksl/DSLWrapper.h" #include "include/sksl/SkSLOperator.h" #include "src/sksl/SkSLThreadContext.h" #include "src/sksl/dsl/priv/DSLWriter.h" @@ -206,12 +205,11 @@ DSLExpression DSLExpression::index(DSLExpression index, Position pos) { return DSLExpression(std::move(result), pos); } -DSLPossibleExpression DSLExpression::operator()(SkTArray> args, - Position pos) { +DSLPossibleExpression DSLExpression::operator()(SkTArray args, Position pos) { ExpressionArray converted; converted.reserve_back(args.count()); - for (DSLWrapper& arg : args) { - converted.push_back(arg->release()); + for (DSLExpression& arg : args) { + converted.push_back(arg.release()); } return (*this)(std::move(converted), pos); } @@ -411,7 +409,7 @@ DSLPossibleExpression DSLPossibleExpression::operator[](DSLExpression index) { return DSLExpression(this->release())[std::move(index)]; } -DSLPossibleExpression DSLPossibleExpression::operator()(SkTArray> args, +DSLPossibleExpression DSLPossibleExpression::operator()(SkTArray args, Position pos) { return DSLExpression(this->release())(std::move(args), pos); } diff --git a/src/sksl/dsl/DSLFunction.cpp b/src/sksl/dsl/DSLFunction.cpp index 97a91e7f6c..1ad20d0d20 100644 --- a/src/sksl/dsl/DSLFunction.cpp +++ b/src/sksl/dsl/DSLFunction.cpp @@ -14,7 +14,6 @@ #include "include/private/SkSLString.h" #include "include/sksl/DSLType.h" #include "include/sksl/DSLVar.h" -#include "include/sksl/DSLWrapper.h" #include "src/sksl/SkSLProgramSettings.h" #include "src/sksl/SkSLThreadContext.h" #include "src/sksl/dsl/priv/DSLWriter.h" @@ -119,11 +118,11 @@ void DSLFunction::define(DSLBlock block, Position pos) { ThreadContext::ProgramElements().push_back(std::move(function)); } -DSLExpression DSLFunction::call(SkTArray> args, Position pos) { +DSLExpression DSLFunction::call(SkTArray args, Position pos) { ExpressionArray released; released.reserve_back(args.size()); - for (DSLWrapper& arg : args) { - released.push_back(arg->release()); + for (DSLExpression& arg : args) { + released.push_back(arg.release()); } return this->call(std::move(released)); } diff --git a/tests/BUILD.bazel b/tests/BUILD.bazel index 9536574a54..b85c26cfbf 100644 --- a/tests/BUILD.bazel +++ b/tests/BUILD.bazel @@ -5745,7 +5745,6 @@ generated_cc_atom( "//include/sksl:DSLStatement_hdr", "//include/sksl:DSLType_hdr", "//include/sksl:DSLVar_hdr", - "//include/sksl:DSLWrapper_hdr", "//include/sksl:DSL_hdr", "//include/sksl:SkSLErrorReporter_hdr", "//include/sksl:SkSLPosition_hdr", diff --git a/tests/SkSLDSLTest.cpp b/tests/SkSLDSLTest.cpp index 111b4fbdf5..590f5483c8 100644 --- a/tests/SkSLDSLTest.cpp +++ b/tests/SkSLDSLTest.cpp @@ -23,7 +23,6 @@ #include "include/sksl/DSLStatement.h" #include "include/sksl/DSLType.h" #include "include/sksl/DSLVar.h" -#include "include/sksl/DSLWrapper.h" #include "include/sksl/SkSLErrorReporter.h" #include "include/sksl/SkSLPosition.h" #include "src/gpu/ganesh/GrDirectContextPriv.h" @@ -1272,7 +1271,7 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLCall, r, ctxInfo) { { DSLExpression sqrt(SkSL::ThreadContext::Compiler().convertIdentifier(SkSL::Position(), "sqrt")); - SkTArray> args; + SkTArray args; args.emplace_back(16); EXPECT_EQUAL(sqrt(std::move(args)), "4.0"); // sqrt(16) gets optimized to 4 } @@ -1282,7 +1281,7 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLCall, r, ctxInfo) { "pow")); DSLVar a(kFloat_Type, "a"); DSLVar b(kFloat_Type, "b"); - SkTArray> args; + SkTArray args; args.emplace_back(a); args.emplace_back(b); EXPECT_EQUAL(pow(std::move(args)), "pow(a, b)"); @@ -1993,19 +1992,6 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLStruct, r, ctxInfo) { "struct NestedStruct { int x; SimpleStruct simple; };"); } -DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLWrapper, r, ctxInfo) { - AutoDSLContext context(ctxInfo.directContext()->priv().getGpu()); - std::vector> exprs; - exprs.push_back(DSLExpression(1)); - exprs.emplace_back(2.0); - EXPECT_EQUAL(std::move(*exprs[0]), "1"); - EXPECT_EQUAL(std::move(*exprs[1]), "2.0"); - - std::vector> vars; - vars.emplace_back(DSLVar(kInt_Type, "x")); - REPORTER_ASSERT(r, DSLWriter::Var(*vars[0])->name() == "x"); -} - DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLRTAdjust, r, ctxInfo) { { AutoDSLContext context(ctxInfo.directContext()->priv().getGpu(), default_settings(),