Added full position tracking to Swizzle

No visible effect yet, but this will enable better error reporting in a
future CL.

Change-Id: I09e1c5d3bb423a7ce42701f15c4bb142b0a9473c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524638
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>

include/sksl/SkSLPosition.h

@@ -31,6 +31,7 @@ public:
     }
 
     static Position Range(int startOffset, int endOffset) {
+        SkASSERT(startOffset <= endOffset);
         Position result;
         result.fStartOffsetOrLine = startOffset;
         result.fEndOffset = endOffset;
@@ -59,6 +60,15 @@ public:
         return fEndOffset;
     }
 
+    // Returns the position from this through, and including the entirety of, end.
+    Position rangeThrough(Position end) const {
+        if (fEndOffset == -1 || end.fEndOffset == -1) {
+            return *this;
+        }
+        SkASSERT(this->startOffset() <= end.startOffset() && this->endOffset() <= end.endOffset());
+        return Range(this->startOffset(), end.endOffset());
+    }
+
     bool operator==(const Position& other) const {
         return fStartOffsetOrLine == other.fStartOffsetOrLine &&
                 fEndOffset == other.fEndOffset;

src/sksl/SkSLDSLParser.cpp

@@ -1708,11 +1708,12 @@ DSLExpression DSLParser::swizzle(Position pos, DSLExpression base,
         }
     }
     switch (length) {
-        case 1: return dsl::Swizzle(std::move(base), components[0]);
-        case 2: return dsl::Swizzle(std::move(base), components[0], components[1]);
-        case 3: return dsl::Swizzle(std::move(base), components[0], components[1], components[2]);
+        case 1: return dsl::Swizzle(std::move(base), components[0], pos);
+        case 2: return dsl::Swizzle(std::move(base), components[0], components[1], pos);
+        case 3: return dsl::Swizzle(std::move(base), components[0], components[1], components[2],
+                                    pos);
         case 4: return dsl::Swizzle(std::move(base), components[0], components[1], components[2],
-                                    components[3]);
+                                    components[3], pos);
         default: SkUNREACHABLE;
     }
 }

src/sksl/SkSLInliner.cpp

@@ -434,7 +434,7 @@ std::unique_ptr<Expression> Inliner::inlineExpression(Position pos,
             return expression.clone();
         case Expression::Kind::kSwizzle: {
             const Swizzle& s = expression.as<Swizzle>();
-            return Swizzle::Make(*fContext, expr(s.base()), s.components());
+            return Swizzle::Make(*fContext, pos, expr(s.base()), s.components());
         }
         case Expression::Kind::kTernary: {
             const TernaryExpression& t = expression.as<TernaryExpression>();

src/sksl/SkSLRehydrator.cpp

@@ -495,6 +495,7 @@ ExpressionArray Rehydrator::expressionArray() {
 }
 
 std::unique_ptr<Expression> Rehydrator::expression() {
+    Position pos;
     int kind = this->readU8();
     switch (kind) {
         case Rehydrator::kBinary_Command: {
@@ -505,63 +506,55 @@ std::unique_ptr<Expression> Rehydrator::expression() {
         }
         case Rehydrator::kBoolLiteral_Command: {
             bool value = this->readU8();
-            return Literal::MakeBool(this->context(), Position(), value);
+            return Literal::MakeBool(this->context(), pos, value);
         }
         case Rehydrator::kConstructorArray_Command: {
             const Type* type = this->type();
-            return ConstructorArray::Make(this->context(), Position(), *type,
-                    this->expressionArray());
+            return ConstructorArray::Make(this->context(), pos, *type, this->expressionArray());
         }
         case Rehydrator::kConstructorArrayCast_Command: {
             const Type* type = this->type();
             ExpressionArray args = this->expressionArray();
             SkASSERT(args.size() == 1);
-            return ConstructorArrayCast::Make(this->context(), Position(), *type,
-                                              std::move(args[0]));
+            return ConstructorArrayCast::Make(this->context(), pos, *type, std::move(args[0]));
         }
         case Rehydrator::kConstructorCompound_Command: {
             const Type* type = this->type();
-            return ConstructorCompound::Make(this->context(), Position(), *type,
-                                             this->expressionArray());
+            return ConstructorCompound::Make(this->context(), pos, *type, this->expressionArray());
         }
         case Rehydrator::kConstructorDiagonalMatrix_Command: {
             const Type* type = this->type();
             ExpressionArray args = this->expressionArray();
             SkASSERT(args.size() == 1);
-            return ConstructorDiagonalMatrix::Make(this->context(), Position(), *type,
-                                                   std::move(args[0]));
+            return ConstructorDiagonalMatrix::Make(this->context(), pos, *type, std::move(args[0]));
         }
         case Rehydrator::kConstructorMatrixResize_Command: {
             const Type* type = this->type();
             ExpressionArray args = this->expressionArray();
             SkASSERT(args.size() == 1);
-            return ConstructorMatrixResize::Make(this->context(), Position(), *type,
-                                                 std::move(args[0]));
+            return ConstructorMatrixResize::Make(this->context(), pos, *type, std::move(args[0]));
         }
         case Rehydrator::kConstructorScalarCast_Command: {
             const Type* type = this->type();
             ExpressionArray args = this->expressionArray();
             SkASSERT(args.size() == 1);
-            return ConstructorScalarCast::Make(this->context(), Position(), *type,
-                    std::move(args[0]));
+            return ConstructorScalarCast::Make(this->context(), pos, *type, std::move(args[0]));
         }
         case Rehydrator::kConstructorSplat_Command: {
             const Type* type = this->type();
             ExpressionArray args = this->expressionArray();
             SkASSERT(args.size() == 1);
-            return ConstructorSplat::Make(this->context(), Position(), *type, std::move(args[0]));
+            return ConstructorSplat::Make(this->context(), pos, *type, std::move(args[0]));
         }
         case Rehydrator::kConstructorStruct_Command: {
             const Type* type = this->type();
-            return ConstructorStruct::Make(this->context(), Position(), *type,
-                    this->expressionArray());
+            return ConstructorStruct::Make(this->context(), pos, *type, this->expressionArray());
         }
         case Rehydrator::kConstructorCompoundCast_Command: {
             const Type* type = this->type();
             ExpressionArray args = this->expressionArray();
             SkASSERT(args.size() == 1);
-            return ConstructorCompoundCast::Make(this->context(), Position(), *type,
-                    std::move(args[0]));
+            return ConstructorCompoundCast::Make(this->context(), pos, *type, std::move(args[0]));
         }
         case Rehydrator::kFieldAccess_Command: {
             std::unique_ptr<Expression> base = this->expression();
@@ -574,7 +567,7 @@ std::unique_ptr<Expression> Rehydrator::expression() {
             int32_t floatBits = this->readS32();
             float value;
             memcpy(&value, &floatBits, sizeof(value));
-            return Literal::MakeFloat(Position(), value, type);
+            return Literal::MakeFloat(pos, value, type);
         }
         case Rehydrator::kFunctionCall_Command: {
             const Type* type = this->type();
@@ -592,7 +585,7 @@ std::unique_ptr<Expression> Rehydrator::expression() {
                 SkASSERT(false);
                 return nullptr;
             }
-            return FunctionCall::Make(this->context(), Position(), type, *f, std::move(args));
+            return FunctionCall::Make(this->context(), pos, type, *f, std::move(args));
         }
         case Rehydrator::kIndex_Command: {
             std::unique_ptr<Expression> base = this->expression();
@@ -603,10 +596,10 @@ std::unique_ptr<Expression> Rehydrator::expression() {
             const Type* type = this->type();
             if (type->isUnsigned()) {
                 unsigned int value = this->readU32();
-                return Literal::MakeInt(Position(), value, type);
+                return Literal::MakeInt(pos, value, type);
             } else {
                 int value = this->readS32();
-                return Literal::MakeInt(Position(), value, type);
+                return Literal::MakeInt(pos, value, type);
             }
         }
         case Rehydrator::kPostfix_Command: {
@@ -630,7 +623,7 @@ std::unique_ptr<Expression> Rehydrator::expression() {
             for (int i = 0; i < count; ++i) {
                 components.push_back(this->readU8());
             }
-            return Swizzle::Make(this->context(), std::move(base), components);
+            return Swizzle::Make(this->context(), pos, std::move(base), components);
         }
         case Rehydrator::kTernary_Command: {
             std::unique_ptr<Expression> test = this->expression();

src/sksl/dsl/DSLCore.cpp

@@ -314,7 +314,7 @@ public:
 
     static DSLExpression Swizzle(DSLExpression base, SkSL::SwizzleComponent::Type a,
                                  Position pos) {
-        return DSLExpression(Swizzle::Convert(ThreadContext::Context(), base.release(),
+        return DSLExpression(Swizzle::Convert(ThreadContext::Context(), pos, base.release(),
                                               ComponentArray{a}),
                              pos);
     }
@@ -323,7 +323,7 @@ public:
                                  SkSL::SwizzleComponent::Type a,
                                  SkSL::SwizzleComponent::Type b,
                                  Position pos) {
-        return DSLExpression(Swizzle::Convert(ThreadContext::Context(), base.release(),
+        return DSLExpression(Swizzle::Convert(ThreadContext::Context(), pos, base.release(),
                                               ComponentArray{a, b}),
                              pos);
     }
@@ -333,7 +333,7 @@ public:
                                  SkSL::SwizzleComponent::Type b,
                                  SkSL::SwizzleComponent::Type c,
                                  Position pos) {
-        return DSLExpression(Swizzle::Convert(ThreadContext::Context(), base.release(),
+        return DSLExpression(Swizzle::Convert(ThreadContext::Context(), pos, base.release(),
                                               ComponentArray{a, b, c}),
                              pos);
     }
@@ -344,7 +344,7 @@ public:
                                  SkSL::SwizzleComponent::Type c,
                                  SkSL::SwizzleComponent::Type d,
                                  Position pos) {
-        return DSLExpression(Swizzle::Convert(ThreadContext::Context(), base.release(),
+        return DSLExpression(Swizzle::Convert(ThreadContext::Context(), pos, base.release(),
                                               ComponentArray{a,b,c,d}),
                              pos);
     }

src/sksl/ir/SkSLBinaryExpression.cpp

@@ -45,8 +45,9 @@ static std::unique_ptr<Expression> rewrite_matrix_vector_multiply(const Context&
         std::unique_ptr<Expression> matN = IndexExpression::Make(
                 context, left.clone(), Literal::MakeInt(context, left.fPosition, n));
         // Get vec[N] with a swizzle expression.
-        std::unique_ptr<Expression> vecN = Swizzle::Make(
-                context, right.clone(), ComponentArray{(SkSL::SwizzleComponent::Type)n});
+        std::unique_ptr<Expression> vecN = Swizzle::Make(context,
+                left.fPosition.rangeThrough(right.fPosition), right.clone(),
+                ComponentArray{(SkSL::SwizzleComponent::Type)n});
         // Multiply them together.
         const Type* matNType = &matN->type();
         std::unique_ptr<Expression> product =

src/sksl/ir/SkSLIndexExpression.cpp

@@ -98,10 +98,12 @@ std::unique_ptr<Expression> IndexExpression::Make(const Context& context,
     if (indexExpr->isIntLiteral()) {
         SKSL_INT indexValue = indexExpr->as<Literal>().intValue();
         if (!index_out_of_range(context, indexValue, *base)) {
+            Position pos = base->fPosition;
             if (baseType.isVector()) {
                 // Constant array indexes on vectors can be converted to swizzles: `v[2]` --> `v.z`.
                 // Swizzling is harmless and can unlock further simplifications for some base types.
-                return Swizzle::Make(context, std::move(base), ComponentArray{(int8_t)indexValue});
+                return Swizzle::Make(context, pos, std::move(base),
+                        ComponentArray{(int8_t)indexValue});
             }
 
             if (baseType.isArray() && !base->hasSideEffects()) {
@@ -143,7 +145,6 @@ std::unique_ptr<Expression> IndexExpression::Make(const Context& context,
                 }
 
                 if (!ctorArgs.empty()) {
-                    Position pos = ctorArgs.front()->fPosition;
                     return ConstructorCompound::Make(context, pos, vecType, std::move(ctorArgs));
                 }
             }

src/sksl/ir/SkSLSwizzle.cpp

@@ -107,6 +107,7 @@ static std::string mask_string(const ComponentArray& components) {
 }
 
 static std::unique_ptr<Expression> optimize_constructor_swizzle(const Context& context,
+                                                                Position pos,
                                                                 const AnyConstructor& base,
                                                                 ComponentArray components) {
     auto baseArguments = base.argumentSpan();
@@ -220,19 +221,20 @@ static std::unique_ptr<Expression> optimize_constructor_swizzle(const Context& c
         if (reorderedArg.fComponents.empty()) {
             newArgs.push_back(std::move(newArg));
         } else {
-            newArgs.push_back(Swizzle::Make(context, std::move(newArg),
+            newArgs.push_back(Swizzle::Make(context, pos, std::move(newArg),
                                             reorderedArg.fComponents));
         }
     }
 
     // Wrap the new argument list in a constructor.
     return Constructor::Convert(context,
-                                base.fPosition,
+                                pos,
                                 componentType.toCompound(context, swizzleSize, /*rows=*/1),
                                 std::move(newArgs));
 }
 
 std::unique_ptr<Expression> Swizzle::Convert(const Context& context,
+                                             Position pos,
                                              std::unique_ptr<Expression> base,
                                              std::string_view maskString) {
     ComponentArray components;
@@ -257,12 +259,12 @@ std::unique_ptr<Expression> Swizzle::Convert(const Context& context,
             case 'q': components.push_back(SwizzleComponent::Q);    break;
             case 'B': components.push_back(SwizzleComponent::UB);   break;
             default:
-                context.fErrors->error(base->fPosition,
+                context.fErrors->error(pos,
                                        String::printf("invalid swizzle component '%c'", field));
                 return nullptr;
         }
     }
-    return Convert(context, std::move(base), std::move(components));
+    return Convert(context, pos, std::move(base), std::move(components));
 }
 
 // Swizzles are complicated due to constant components. The most difficult case is a mask like
@@ -271,15 +273,14 @@ std::unique_ptr<Expression> Swizzle::Convert(const Context& context,
 // secondary swizzle to put them back into the right order, so in this case we end up with
 // 'float4(base.xw, 1, 0).xzyw'.
 std::unique_ptr<Expression> Swizzle::Convert(const Context& context,
+                                             Position pos,
                                              std::unique_ptr<Expression> base,
                                              ComponentArray inComponents) {
     if (!validate_swizzle_domain(inComponents)) {
-        context.fErrors->error(base->fPosition,
-                "invalid swizzle mask '" + mask_string(inComponents) + "'");
+        context.fErrors->error(pos, "invalid swizzle mask '" + mask_string(inComponents) + "'");
         return nullptr;
     }
 
-    const Position pos = base->fPosition;
     const Type& baseType = base->type();
 
     if (!baseType.isVector() && !baseType.isScalar()) {
@@ -363,7 +364,7 @@ std::unique_ptr<Expression> Swizzle::Convert(const Context& context,
     //   scalar.x0x0 -> type2(scalar)
     //   vector.zyx  -> vector.zyx
     //   vector.x0y0 -> vector.xy
-    std::unique_ptr<Expression> expr = Swizzle::Make(context, std::move(base), maskComponents);
+    std::unique_ptr<Expression> expr = Swizzle::Make(context, pos, std::move(base), maskComponents);
 
     // If we have processed the entire swizzle, we're done.
     if (maskComponents.count() == inComponents.count()) {
@@ -428,10 +429,11 @@ std::unique_ptr<Expression> Swizzle::Convert(const Context& context,
         return nullptr;
     }
 
-    return Swizzle::Make(context, std::move(expr), swizzleComponents);
+    return Swizzle::Make(context, pos, std::move(expr), swizzleComponents);
 }
 
 std::unique_ptr<Expression> Swizzle::Make(const Context& context,
+                                          Position pos,
                                           std::unique_ptr<Expression> expr,
                                           ComponentArray components) {
     const Type& exprType = expr->type();
@@ -450,7 +452,6 @@ std::unique_ptr<Expression> Swizzle::Make(const Context& context,
     // SkSL supports splatting a scalar via `scalar.xxxx`, but not all versions of GLSL allow this.
     // Replace swizzles with equivalent splat constructors (`scalar.xxx` --> `half3(value)`).
     if (exprType.isScalar()) {
-        Position pos = expr->fPosition;
         return ConstructorSplat::Make(context, pos,
                                       exprType.toCompound(context, components.size(), /*rows=*/1),
                                       std::move(expr));
@@ -480,7 +481,7 @@ std::unique_ptr<Expression> Swizzle::Make(const Context& context,
 
         // It may actually be possible to further simplify this swizzle. Go again.
         // (e.g. `color.abgr.abgr` --> `color.rgba` --> `color`.)
-        return Swizzle::Make(context, std::move(base.base()), combined);
+        return Swizzle::Make(context, pos, std::move(base.base()), combined);
     }
 
     // If we are swizzling a constant expression, we can use its value instead here (so that
@@ -501,13 +502,13 @@ std::unique_ptr<Expression> Swizzle::Make(const Context& context,
     // Optimize swizzles of constructors.
     if (value->isAnyConstructor()) {
         const AnyConstructor& ctor = value->asAnyConstructor();
-        if (auto replacement = optimize_constructor_swizzle(context, ctor, components)) {
+        if (auto replacement = optimize_constructor_swizzle(context, pos, ctor, components)) {
             return replacement;
         }
     }
 
     // The swizzle could not be simplified, so apply the requested swizzle to the base expression.
-    return std::make_unique<Swizzle>(context, std::move(expr), components);
+    return std::make_unique<Swizzle>(context, pos, std::move(expr), components);
 }
 
 }  // namespace SkSL

src/sksl/ir/SkSLSwizzle.h

@@ -22,9 +22,9 @@ namespace SkSL {
 struct Swizzle final : public Expression {
     inline static constexpr Kind kExpressionKind = Kind::kSwizzle;
 
-    Swizzle(const Context& context, std::unique_ptr<Expression> base,
+    Swizzle(const Context& context, Position pos, std::unique_ptr<Expression> base,
             const ComponentArray& components)
-            : INHERITED(base->fPosition, kExpressionKind,
+            : INHERITED(pos, kExpressionKind,
                         &base->type().componentType().toCompound(context, components.size(), 1))
             , fBase(std::move(base))
             , fComponents(components) {
@@ -35,16 +35,19 @@ struct Swizzle final : public Expression {
     // errors via ErrorReporter, and returns an expression that combines constructors and native
     // swizzles (comprised solely of X/Y/W/Z).
     static std::unique_ptr<Expression> Convert(const Context& context,
+                                               Position pos,
                                                std::unique_ptr<Expression> base,
                                                ComponentArray inComponents);
 
     static std::unique_ptr<Expression> Convert(const Context& context,
+                                               Position pos,
                                                std::unique_ptr<Expression> base,
                                                std::string_view maskString);
 
     // Swizzle::Make does not permit ZERO or ONE in the component array, just X/Y/Z/W; errors are
     // reported via ASSERT.
     static std::unique_ptr<Expression> Make(const Context& context,
+                                            Position pos,
                                             std::unique_ptr<Expression> expr,
                                             ComponentArray inComponents);
 
@@ -65,8 +68,8 @@ struct Swizzle final : public Expression {
     }
 
     std::unique_ptr<Expression> clone() const override {
-        return std::unique_ptr<Expression>(new Swizzle(&this->type(), this->base()->clone(),
-                                                       this->components()));
+        return std::unique_ptr<Expression>(new Swizzle(fPosition, &this->type(),
+                this->base()->clone(), this->components()));
     }
 
     std::string description() const override {
@@ -78,8 +81,9 @@ struct Swizzle final : public Expression {
     }
 
 private:
-    Swizzle(const Type* type, std::unique_ptr<Expression> base, const ComponentArray& components)
-        : INHERITED(base->fPosition, kExpressionKind, type)
+    Swizzle(Position pos, const Type* type, std::unique_ptr<Expression> base,
+            const ComponentArray& components)
+        : INHERITED(pos, kExpressionKind, type)
         , fBase(std::move(base))
         , fComponents(components) {
         SkASSERT(this->components().size() >= 1 && this->components().size() <= 4);