Migrate IRGenerator::setRefKind to Analysis::MakeAssignmentExpr.
Fundamentally this function wasn't a generic "set the refKind on an
expression" at all; it was intended specifically as "this expression
must be IsAssignable and contain a VariableReference." Also, it needs
nothing at all from the IRGenerator. Moving it out is important because
it will be used by Make functions for things like VariableReference.
Change-Id: I52d613989cd5fb0feb2e98ae454943cdacb9404b
Bug: skia:11342
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/376841
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
diff --git a/src/sksl/SkSLAnalysis.cpp b/src/sksl/SkSLAnalysis.cpp
index 8002ea5..77bd11c 100644
--- a/src/sksl/SkSLAnalysis.cpp
+++ b/src/sksl/SkSLAnalysis.cpp
@@ -528,6 +528,21 @@
RefKindWriter{refKind}.visitExpression(*expr);
}
+bool Analysis::MakeAssignmentExpr(Expression* expr,
+ VariableReference::RefKind kind,
+ ErrorReporter* errors) {
+ Analysis::AssignmentInfo info;
+ if (!Analysis::IsAssignable(*expr, &info, errors)) {
+ return false;
+ }
+ if (!info.fAssignedVar) {
+ errors->error(expr->fOffset, "can't assign to expression '" + expr->description() + "'");
+ return false;
+ }
+ info.fAssignedVar->setRefKind(kind);
+ return true;
+}
+
bool Analysis::IsTrivialExpression(const Expression& expr) {
return expr.is<IntLiteral>() ||
expr.is<FloatLiteral>() ||
diff --git a/src/sksl/SkSLAnalysis.h b/src/sksl/SkSLAnalysis.h
index 190593b..d33f3c8 100644
--- a/src/sksl/SkSLAnalysis.h
+++ b/src/sksl/SkSLAnalysis.h
@@ -65,7 +65,12 @@
static bool IsAssignable(Expression& expr, AssignmentInfo* info,
ErrorReporter* errors = nullptr);
+ // Updates the `refKind` field of exactly one VariableReference inside `expr`.
+ // `expr` must be `IsAssignable`; returns an error otherwise.
+ static bool MakeAssignmentExpr(Expression* expr, VariableRefKind kind, ErrorReporter* errors);
+
// Updates the `refKind` field of every VariableReference found within `expr`.
+ // `expr` is allowed to have zero, one, or multiple VariableReferences.
static void UpdateRefKind(Expression* expr, VariableRefKind refKind);
// A "trivial" expression is one where we'd feel comfortable cloning it multiple times in
diff --git a/src/sksl/SkSLIRGenerator.cpp b/src/sksl/SkSLIRGenerator.cpp
index 7534900..a0e0ae2 100644
--- a/src/sksl/SkSLIRGenerator.cpp
+++ b/src/sksl/SkSLIRGenerator.cpp
@@ -1824,9 +1824,12 @@
return nullptr;
}
bool isAssignment = op.isAssignment();
- if (isAssignment && !this->setRefKind(*left, op.kind() != Token::Kind::TK_EQ
- ? VariableReference::RefKind::kReadWrite
- : VariableReference::RefKind::kWrite)) {
+ if (isAssignment &&
+ !Analysis::MakeAssignmentExpr(left.get(),
+ op.kind() != Token::Kind::TK_EQ
+ ? VariableReference::RefKind::kReadWrite
+ : VariableReference::RefKind::kWrite,
+ &fContext.fErrors)) {
return nullptr;
}
if (!determine_binary_type(fContext, this->settings().fAllowNarrowingConversions, op,
@@ -2020,9 +2023,11 @@
}
const Modifiers& paramModifiers = function.parameters()[i]->modifiers();
if (paramModifiers.fFlags & Modifiers::kOut_Flag) {
- if (!this->setRefKind(*arguments[i], paramModifiers.fFlags & Modifiers::kIn_Flag
- ? VariableReference::RefKind::kReadWrite
- : VariableReference::RefKind::kPointer)) {
+ if (!Analysis::MakeAssignmentExpr(arguments[i].get(),
+ paramModifiers.fFlags & Modifiers::kIn_Flag
+ ? VariableReference::RefKind::kReadWrite
+ : VariableReference::RefKind::kPointer,
+ &fContext.fErrors)) {
return nullptr;
}
}
@@ -2164,7 +2169,8 @@
"' cannot operate on '" + baseType.displayName() + "'");
return nullptr;
}
- if (!this->setRefKind(*base, VariableReference::RefKind::kReadWrite)) {
+ if (!Analysis::MakeAssignmentExpr(base.get(), VariableReference::RefKind::kReadWrite,
+ &fContext.fErrors)) {
return nullptr;
}
break;
@@ -2175,7 +2181,8 @@
"' cannot operate on '" + baseType.displayName() + "'");
return nullptr;
}
- if (!this->setRefKind(*base, VariableReference::RefKind::kReadWrite)) {
+ if (!Analysis::MakeAssignmentExpr(base.get(), VariableReference::RefKind::kReadWrite,
+ &fContext.fErrors)) {
return nullptr;
}
break;
@@ -2505,7 +2512,8 @@
"' cannot operate on '" + baseType.displayName() + "'");
return nullptr;
}
- if (!this->setRefKind(*base, VariableReference::RefKind::kReadWrite)) {
+ if (!Analysis::MakeAssignmentExpr(base.get(), VariableReference::RefKind::kReadWrite,
+ &fContext.fErrors)) {
return nullptr;
}
return std::make_unique<PostfixExpression>(std::move(base), op);
@@ -2534,17 +2542,6 @@
}
}
-bool IRGenerator::setRefKind(Expression& expr, VariableReference::RefKind kind) {
- Analysis::AssignmentInfo info;
- if (!Analysis::IsAssignable(expr, &info, &this->errorReporter())) {
- return false;
- }
- if (info.fAssignedVar) {
- info.fAssignedVar->setRefKind(kind);
- }
- return true;
-}
-
void IRGenerator::findAndDeclareBuiltinVariables() {
class BuiltinVariableScanner : public ProgramVisitor {
public:
