commit b6b7898599d7b085eb4bd570273ad8a5a2a0df91
author John Stiles <johnstiles@google.com> Wed May 18 12:16:19 2022 -0400
committer SkCQ <skcq-be@skia-corp.google.com.iam.gserviceaccount.com> Wed May 18 20:31:49 2022 +0000
tree 85dd4b7bdac53b647f6473f0c561ae8bcd160114
parent b7d4cebf08375363959e0d76fd8f1aa3e68c4cd3

Remove program setting `fAssertDSLObjectsReleased`.

This setting was meant to detect errors in hand-authored DSL programs,
where a DSL expression or statement object was instantiated but wasn't
actually added to the program.

The DSL parser always disabled this flag. We don't plan on creating
large hand-crafted DSL programs going forward. Also, if you fail to add
an expression/statement to your program, your program will be missing
part of its code, which is already pretty self-evident as errors go.

Change-Id: I4c7b89d1f05cc65dc84948d98da8d6576c1be244
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/541739
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>

src/sksl/SkSLDSLParser.cpp

diff --git a/src/sksl/SkSLDSLParser.cpp b/src/sksl/SkSLDSLParser.cpp
index b720d80..89a2517 100644
--- a/src/sksl/SkSLDSLParser.cpp
+++ b/src/sksl/SkSLDSLParser.cpp
@@ -109,9 +109,6 @@
     , fKind(kind)
     , fText(std::make_unique<std::string>(std::move(text)))
     , fPushback(Token::Kind::TK_NONE, /*offset=*/-1, /*length=*/-1) {
-    // We don't want to have to worry about manually releasing all of the objects in the event that
-    // an error occurs
-    fSettings.fAssertDSLObjectsReleased = false;
     fLexer.start(*fText);
 }
 

src/sksl/SkSLProgramSettings.h

diff --git a/src/sksl/SkSLProgramSettings.h b/src/sksl/SkSLProgramSettings.h
index 7967ed8..da9d2e3 100644
--- a/src/sksl/SkSLProgramSettings.h
+++ b/src/sksl/SkSLProgramSettings.h
@@ -77,8 +77,6 @@
     bool fAllowTraceVarInSkVMDebugTrace = true;
     // If true, the DSL should install a memory pool when possible.
     bool fDSLUseMemoryPool = true;
-    // If true, DSL objects assert that they were used prior to destruction
-    bool fAssertDSLObjectsReleased = true;
     // If true, VarDeclaration can be cloned for testing purposes. See VarDeclaration::clone for
     // more information.
     bool fAllowVarDeclarationCloneForTesting = false;

src/sksl/dsl/DSLExpression.cpp

diff --git a/src/sksl/dsl/DSLExpression.cpp b/src/sksl/dsl/DSLExpression.cpp
index fffcddb..ebe8ed6 100644
--- a/src/sksl/dsl/DSLExpression.cpp
+++ b/src/sksl/dsl/DSLExpression.cpp
@@ -110,11 +110,7 @@
     return DSLExpression(SkSL::Poison::Make(pos, ThreadContext::Context()));
 }
 
-DSLExpression::~DSLExpression() {
-    SkASSERTF(!fExpression || !ThreadContext::Settings().fAssertDSLObjectsReleased,
-              "Expression destroyed without being incorporated into program (see "
-              "ProgramSettings::fAssertDSLObjectsReleased)");
-}
+DSLExpression::~DSLExpression() {}
 
 bool DSLExpression::isValid() const {
     return this->hasValue() && !fExpression->is<SkSL::Poison>();

src/sksl/dsl/DSLStatement.cpp

diff --git a/src/sksl/dsl/DSLStatement.cpp b/src/sksl/dsl/DSLStatement.cpp
index e0455a0..4018cf6 100644
--- a/src/sksl/dsl/DSLStatement.cpp
+++ b/src/sksl/dsl/DSLStatement.cpp
@@ -59,11 +59,7 @@
     }
 }
 
-DSLStatement::~DSLStatement() {
-    SkASSERTF(!fStatement || !ThreadContext::Settings().fAssertDSLObjectsReleased,
-              "Statement destroyed without being incorporated into program (see "
-              "ProgramSettings::fAssertDSLObjectsReleased)");
-}
+DSLStatement::~DSLStatement() {}
 
 DSLPossibleStatement::DSLPossibleStatement(std::unique_ptr<SkSL::Statement> statement)
     : fStatement(std::move(statement)) {}

tests/SkSLDSLTest.cpp

diff --git a/tests/SkSLDSLTest.cpp b/tests/SkSLDSLTest.cpp
index 737142d..8a9b1fc 100644
--- a/tests/SkSLDSLTest.cpp
+++ b/tests/SkSLDSLTest.cpp
@@ -2078,15 +2078,6 @@
     REPORTER_ASSERT(r, *program->fSource == source);
 }
 
-DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLReleaseUnused, r, ctxInfo) {
-    SkSL::ProgramSettings settings = default_settings();
-    settings.fAssertDSLObjectsReleased = false;
-    AutoDSLContext context(ctxInfo.directContext()->priv().getGpu(), settings);
-    If(Sqrt(1) > 0, Discard());
-    // Ensure that we can safely destroy statements and expressions despite being unused while
-    // settings.fAssertDSLObjectsReleased is disabled.
-}
-
 DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLPrototypes, r, ctxInfo) {
     AutoDSLContext context(ctxInfo.directContext()->priv().getGpu());
     {