WSL should be fine with &foo()[i] if foo() returns a []
authorfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 2 Oct 2017 22:01:02 +0000 (22:01 +0000)
committerfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 2 Oct 2017 22:01:02 +0000 (22:01 +0000)
https://bugs.webkit.org/show_bug.cgi?id=177704

Reviewed by Saam Barati.

Previously, we'd determine if a property access expression (base.field or base[index]) was an lvalue by
asking if its base was an lvalue. This is right in all cases except if the base is of type []. Then, the
property access expression is an lvalue so long as there is a setter or ander.

This fixes the issue and adds a test.

Also, this makes error messages in the case that something is not an lvalue a lot better. If something
is not an lvalue because we could not find anders or setters, then we will tell you why we could not
find them.

* WebGPUShadingLanguageRI/Checker.js:
(Checker.prototype.visitAssignment):
(Checker.prototype.visitReadModifyWriteExpression):
(Checker.prototype.visitMakePtrExpression):
(Checker.prototype._finishVisitingPropertyAccess):
* WebGPUShadingLanguageRI/DotExpression.js:
(DotExpression.prototype.get fieldName):
(DotExpression.prototype.get isLValue): Deleted.
(DotExpression.prototype.get addressSpace): Deleted.
* WebGPUShadingLanguageRI/IndexExpression.js:
(IndexExpression.prototype.get index):
(IndexExpression.prototype.get isLValue): Deleted.
(IndexExpression.prototype.get addressSpace): Deleted.
* WebGPUShadingLanguageRI/PropertyAccessExpression.js:
(PropertyAccessExpression):
(PropertyAccessExpression.prototype.get isLValue):
(PropertyAccessExpression.prototype.set isLValue):
* WebGPUShadingLanguageRI/PropertyResolver.js:
(PropertyResolver.prototype._visitRValuesWithinLValue.RValueFinder.prototype.visitMakeArrayRefExpression):
(PropertyResolver.prototype._visitRValuesWithinLValue.RValueFinder):
(PropertyResolver.prototype._visitRValuesWithinLValue):
* WebGPUShadingLanguageRI/Test.js:
(tests.storeNullArrayRef):
(tests.andReturnedArrayRef):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@222750 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Tools/ChangeLog
Tools/WebGPUShadingLanguageRI/Checker.js
Tools/WebGPUShadingLanguageRI/DotExpression.js
Tools/WebGPUShadingLanguageRI/IndexExpression.js
Tools/WebGPUShadingLanguageRI/PropertyAccessExpression.js
Tools/WebGPUShadingLanguageRI/PropertyResolver.js
Tools/WebGPUShadingLanguageRI/Test.js
Tools/WebGPUShadingLanguageRI/Value.js

index 2379d92..e7fff86 100644 (file)
@@ -1,3 +1,45 @@
+2017-10-02  Filip Pizlo  <fpizlo@apple.com>
+
+        WSL should be fine with &foo()[i] if foo() returns a []
+        https://bugs.webkit.org/show_bug.cgi?id=177704
+
+        Reviewed by Saam Barati.
+        
+        Previously, we'd determine if a property access expression (base.field or base[index]) was an lvalue by
+        asking if its base was an lvalue. This is right in all cases except if the base is of type []. Then, the
+        property access expression is an lvalue so long as there is a setter or ander.
+        
+        This fixes the issue and adds a test.
+        
+        Also, this makes error messages in the case that something is not an lvalue a lot better. If something
+        is not an lvalue because we could not find anders or setters, then we will tell you why we could not
+        find them.
+
+        * WebGPUShadingLanguageRI/Checker.js:
+        (Checker.prototype.visitAssignment):
+        (Checker.prototype.visitReadModifyWriteExpression):
+        (Checker.prototype.visitMakePtrExpression):
+        (Checker.prototype._finishVisitingPropertyAccess):
+        * WebGPUShadingLanguageRI/DotExpression.js:
+        (DotExpression.prototype.get fieldName):
+        (DotExpression.prototype.get isLValue): Deleted.
+        (DotExpression.prototype.get addressSpace): Deleted.
+        * WebGPUShadingLanguageRI/IndexExpression.js:
+        (IndexExpression.prototype.get index):
+        (IndexExpression.prototype.get isLValue): Deleted.
+        (IndexExpression.prototype.get addressSpace): Deleted.
+        * WebGPUShadingLanguageRI/PropertyAccessExpression.js:
+        (PropertyAccessExpression):
+        (PropertyAccessExpression.prototype.get isLValue):
+        (PropertyAccessExpression.prototype.set isLValue):
+        * WebGPUShadingLanguageRI/PropertyResolver.js:
+        (PropertyResolver.prototype._visitRValuesWithinLValue.RValueFinder.prototype.visitMakeArrayRefExpression):
+        (PropertyResolver.prototype._visitRValuesWithinLValue.RValueFinder):
+        (PropertyResolver.prototype._visitRValuesWithinLValue):
+        * WebGPUShadingLanguageRI/Test.js:
+        (tests.storeNullArrayRef):
+        (tests.andReturnedArrayRef):
+
 2017-10-02  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         Guard iOS webkitGetAsEntry API tests on older iOS versions
index 7c6fb2f..8d55c50 100644 (file)
@@ -329,9 +329,9 @@ class Checker extends Visitor {
     
     visitAssignment(node)
     {
-        if (!node.lhs.isLValue)
-            throw new WTypeError(node.origin.originString, "LHS of assignment is not an LValue: " + node.lhs);
         let lhsType = node.lhs.visit(this);
+        if (!node.lhs.isLValue)
+            throw new WTypeError(node.origin.originString, "LHS of assignment is not an LValue: " + node.lhs + node.lhs.notLValueReasonString);
         let rhsType = node.rhs.visit(this);
         if (!lhsType.equalsWithCommit(rhsType))
             throw new WTypeError(node.origin.originString, "Type mismatch in assignment: " + lhsType + " versus " + rhsType);
@@ -346,9 +346,9 @@ class Checker extends Visitor {
     
     visitReadModifyWriteExpression(node)
     {
-        if (!node.lValue.isLValue)
-            throw new WTypeError(node.origin.originString, "LHS of read-modify-write is not an LValue: " + node.lValue);
         let lhsType = node.lValue.visit(this);
+        if (!node.lValue.isLValue)
+            throw new WTypeError(node.origin.originString, "LHS of read-modify-write is not an LValue: " + node.lValue + node.lValue.notLValueReasonString);
         node.oldValueVar.type = lhsType;
         node.newValueVar.type = lhsType;
         node.oldValueVar.visit(this);
@@ -379,10 +379,9 @@ class Checker extends Visitor {
     
     visitMakePtrExpression(node)
     {
-        if (!node.lValue.isLValue)
-            throw new WTypeError(node.origin.originString, "Operand to & is not an LValue: " + node.lValue);
-        
         let elementType = node.lValue.visit(this).unifyNode;
+        if (!node.lValue.isLValue)
+            throw new WTypeError(node.origin.originString, "Operand to & is not an LValue: " + node.lValue + node.lValue.notLValueReasonString);
         
         return new PtrType(node.origin, node.lValue.addressSpace, elementType);
     }
@@ -396,7 +395,7 @@ class Checker extends Visitor {
         }
         
         if (!node.lValue.isLValue)
-            throw new WTypeError(node.origin.originString, "Operand to @ is not an LValue: " + node.lValue);
+            throw new WTypeError(node.origin.originString, "Operand to @ is not an LValue: " + node.lValue + node.lValue.notLValueReasonString);
         
         if (elementType.isArray) {
             node.numElements = elementType.numElements;
@@ -456,9 +455,9 @@ class Checker extends Visitor {
             throw new WTypeError(
                 node.origin.originString,
                 "Cannot resolve access; tried by-value:\n" +
-                errorForGet.message + "\n" +
+                errorForGet.typeErrorMessage + "\n" +
                 "and tried by-pointer:\n" +
-                errorForAnd.message);
+                errorForAnd.typeErrorMessage);
         }
         
         if (node.resultTypeForGet && node.resultTypeForAnd
@@ -478,6 +477,24 @@ class Checker extends Visitor {
             node.errorForSet = e;
         }
         
+        // OK, now we need to determine if we are an lvalue. We are an lvalue if we can be assigned to. We can
+        // be assigned to if we have an ander or setter. But it's weirder than that. We also need the base to be
+        // an lvalue, except unless the base is an array reference.
+        if (!node.callForAnd && !node.callForSet) {
+            node.isLValue = false;
+            node.notLValueReason =
+                "Have neither ander nor setter. Tried setter:\n" +
+                node.errorForSet.typeErrorMessage + "\n" +
+                "and tried ander:\n" +
+                errorForAnd.typeErrorMessage;
+        } else if (!node.base.isLValue && !baseType.isArrayRef) {
+            node.isLValue = false;
+            node.notLValueReason = "Base of property access is neither a lvalue nor an array reference";
+        } else {
+            node.isLValue = true;
+            node.addressSpace = node.base.isLValue ? node.base.addressSpace : baseType.addressSpace;
+        }
+        
         return node.resultType;
     }
     
index c1fbade..110de8c 100644 (file)
@@ -33,8 +33,6 @@ class DotExpression extends PropertyAccessExpression {
     
     get struct() { return this.base; }
     get fieldName() { return this._fieldName; }
-    get isLValue() { return this.struct.isLValue; }
-    get addressSpace() { return this.struct.addressSpace; }
     
     get getFuncName() { return "operator." + this.fieldName; }
     get andFuncName() { return "operator&." + this.fieldName; }
index 61d23da..b0b7f91 100644 (file)
@@ -33,8 +33,6 @@ class IndexExpression extends PropertyAccessExpression {
     
     get array() { return this.base; }
     get index() { return this._index; }
-    get isLValue() { return this.array.isLValue; }
-    get addressSpace() { return this.array.addressSpace; }
     
     get getFuncName() { return "operator[]"; }
     get andFuncName() { return "operator&[]"; }
index 09e426b..ee4ad90 100644 (file)
@@ -29,9 +29,16 @@ class PropertyAccessExpression extends Expression {
     {
         super(origin);
         this.base = base;
+        this._isLValue = null; // We use null to indicate that we don't know yet.
+        this.addressSpace = null;
+        this._notLValueReason = null;
     }
     
     get resultType() { return this.resultTypeForGet ? this.resultTypeForGet : this.resultTypeForAnd; }
+    get isLValue() { return this._isLValue; }
+    set isLValue(value) { this._isLValue = value; }
+    get notLValueReason() { return this._notLValueReason; }
+    set notLValueReason(value) { this._notLValueReason = value; }
     
     rewriteAfterCloning()
     {
index 6f80800..57c600c 100644 (file)
@@ -54,6 +54,11 @@ class PropertyResolver extends Visitor {
             {
                 node.target.visit(this);
             }
+            
+            visitMakeArrayRefExpression(node)
+            {
+                visit(node.lValue);
+            }
         }
         
         node.visit(new RValueFinder());
index 49c38e2..9459100 100644 (file)
@@ -845,7 +845,7 @@ tests.storeNullArrayRef = function()
         () => doPrep(`
             void foo() { null[0u] = 42; }
         `),
-        (e) => e instanceof WTypeError && e.message.indexOf("LHS of assignment is not an LValue") != -1);
+        (e) => e instanceof WTypeError && e.message.indexOf("Cannot resolve access") != -1);
 }
 
 tests.returnNullArrayRef = function()
@@ -2752,7 +2752,7 @@ tests.assignLength = function()
                 (@array).length = 42;
             }
         `),
-        (e) => e instanceof WTypeError && e.message.indexOf("LHS of assignment is not an LValue") != -1);
+        (e) => e instanceof WTypeError && e.message.indexOf("Have neither ander nor setter") != -1);
 }
 
 tests.assignLengthHelper = function()
@@ -2769,7 +2769,7 @@ tests.assignLengthHelper = function()
                 bar(@array);
             }
         `),
-        (e) => e instanceof WTypeError && e.message.indexOf("Cannot emit set because: Did not find any functions named operator.length=") != -1);
+        (e) => e instanceof WTypeError && e.message.indexOf("Have neither ander nor setter") != -1);
 }
 
 tests.simpleGetter = function()
@@ -6553,6 +6553,24 @@ tests.indexAnderWithArrayRefInProtocol = function()
     checkInt(program, callFunction(program, "foo", [], []), 1234);
 }
 
+tests.andReturnedArrayRef = function()
+{
+    let program = doPrep(`
+        thread int[] getArray()
+        {
+            int[10] x;
+            x[5] = 354;
+            return @x;
+        }
+        int foo()
+        {
+            thread int^ ptr = &getArray()[5];
+            return ^ptr;
+        }
+    `);
+    checkInt(program, callFunction(program, "foo", [], []), 354);
+}
+
 okToTest = true;
 
 let testFilter = /.*/; // run everything by default
index 89e108d..3f50dbf 100644 (file)
@@ -28,6 +28,15 @@ class Value extends Node {
     get kind() { return Value; }
     get isConstexpr() { return false; }
     get isLValue() { return false; }
+    get notLValueReason() { return null; }
+    
+    get notLValueReasonString()
+    {
+        let result = this.notLValueReason;
+        if (result)
+            return "\n" + result;
+        return "";
+    }
     
     become(otherValue)
     {