Simplify IsAssignable to only support one assignable var.

Now that ternaries are no longer supported for assignment (as per GLSL
spec), there's no longer any cases where an assignment can target more
than one variable reference at a time. Replace the output vector of
VariableReferences `assignedVars` with a single VariableReference,
`assignedVar`.

Also, allow callers to pass null for the ErrorReporter. This is useful
if inability to assign to an expression does not actually indicate an
error condition.

Change-Id: I146a9d1a488131ac5048c665e4dc880d895a275a
Bug: skia:10767
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319859
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This commit is contained in:
John Stiles 2020-09-25 23:06:26 -04:00 committed by Skia Commit-Bot
parent 6a189f23af
commit a976da7e2f
3 changed files with 39 additions and 26 deletions

View File

@ -207,18 +207,31 @@ private:
using INHERITED = ProgramVisitor;
};
// If a caller doesn't care about errors, we can use this trivial reporter that just counts up.
class TrivialErrorReporter : public ErrorReporter {
public:
void error(int offset, String) override { ++fErrorCount; }
int errorCount() override { return fErrorCount; }
private:
int fErrorCount = 0;
};
// This isn't actually using ProgramVisitor, because it only considers a subset of the fields for
// any given expression kind. For instance, when indexing an array (e.g. `x[1]`), we only want to
// know if the base (`x`) is assignable; the index expression (`1`) doesn't need to be.
class IsAssignableVisitor {
public:
IsAssignableVisitor(std::vector<VariableReference*>* assignableVars, ErrorReporter& errors)
: fAssignableVars(assignableVars)
, fErrors(errors) {}
IsAssignableVisitor(VariableReference** assignableVar, ErrorReporter* errors)
: fAssignableVar(assignableVar), fErrors(errors) {
if (fAssignableVar) {
*fAssignableVar = nullptr;
}
}
bool visit(Expression& expr) {
this->visitExpression(expr);
return fErrors.errorCount() == 0;
return fErrors->errorCount() == 0;
}
void visitExpression(Expression& expr) {
@ -228,10 +241,11 @@ public:
const Variable* var = varRef.fVariable;
if (var->fModifiers.fFlags & (Modifiers::kConst_Flag | Modifiers::kUniform_Flag |
Modifiers::kVarying_Flag)) {
fErrors.error(expr.fOffset,
"cannot modify immutable variable '" + var->fName + "'");
} else if (fAssignableVars) {
fAssignableVars->push_back(&varRef);
fErrors->error(expr.fOffset,
"cannot modify immutable variable '" + var->fName + "'");
} else if (fAssignableVar) {
SkASSERT(*fAssignableVar == nullptr);
*fAssignableVar = &varRef;
}
break;
}
@ -252,13 +266,13 @@ public:
case Expression::Kind::kExternalValue: {
const ExternalValue* var = expr.as<ExternalValueReference>().fValue;
if (!var->canWrite()) {
fErrors.error(expr.fOffset,
"cannot modify immutable external value '" + var->fName + "'");
fErrors->error(expr.fOffset,
"cannot modify immutable external value '" + var->fName + "'");
}
break;
}
default:
fErrors.error(expr.fOffset, "cannot assign to this expression");
fErrors->error(expr.fOffset, "cannot assign to this expression");
break;
}
}
@ -270,22 +284,20 @@ private:
SkASSERT(idx <= 3);
int bit = 1 << idx;
if (bits & bit) {
fErrors.error(swizzle.fOffset,
"cannot write to the same swizzle field more than once");
fErrors->error(swizzle.fOffset,
"cannot write to the same swizzle field more than once");
break;
}
bits |= bit;
}
}
std::vector<VariableReference*>* fAssignableVars;
ErrorReporter& fErrors;
VariableReference** fAssignableVar;
ErrorReporter* fErrors;
using INHERITED = ProgramVisitor;
};
} // namespace
////////////////////////////////////////////////////////////////////////////////
@ -317,9 +329,10 @@ bool Analysis::StatementWritesToVariable(const Statement& stmt, const Variable&
return VariableWriteVisitor(&var).visit(stmt);
}
bool Analysis::IsAssignable(Expression& expr, std::vector<VariableReference*>* assignableVars,
ErrorReporter& errors) {
return IsAssignableVisitor{assignableVars, errors}.visit(expr);
bool Analysis::IsAssignable(Expression& expr, VariableReference** assignableVar,
ErrorReporter* errors) {
TrivialErrorReporter trivialErrors;
return IsAssignableVisitor{assignableVar, errors ? errors : &trivialErrors}.visit(expr);
}
////////////////////////////////////////////////////////////////////////////////

View File

@ -38,8 +38,8 @@ struct Analysis {
static int NodeCount(const FunctionDefinition& function);
static bool StatementWritesToVariable(const Statement& stmt, const Variable& var);
static bool IsAssignable(Expression& expr, std::vector<VariableReference*>* assignableVars,
ErrorReporter& errors);
static bool IsAssignable(Expression& expr, VariableReference** assignableVar,
ErrorReporter* errors = nullptr);
};
/**

View File

@ -2811,12 +2811,12 @@ void IRGenerator::checkValid(const Expression& expr) {
}
bool IRGenerator::setRefKind(Expression& expr, VariableReference::RefKind kind) {
std::vector<VariableReference*> assignableVars;
if (!Analysis::IsAssignable(expr, &assignableVars, fErrors)) {
VariableReference* assignableVar = nullptr;
if (!Analysis::IsAssignable(expr, &assignableVar, &fErrors)) {
return false;
}
for (VariableReference* v : assignableVars) {
v->setRefKind(kind);
if (assignableVar) {
assignableVar->setRefKind(kind);
}
return true;
}