Title: [221487] trunk/Tools
Revision
221487
Author
fpi...@apple.com
Date
2017-09-01 11:48:27 -0700 (Fri, 01 Sep 2017)

Log Message

WSL Rewriter should be an identity on things that aren't inside function bodies
https://bugs.webkit.org/show_bug.cgi?id=176208

Reviewed by Myles Maxfield.
        
Previously, if the Rewriter encountered a FunctionDef, StructType, NativeType, etc., then it
would either crash or try to rewrite them. That's unfortunate because we use the Rewriter to
rewrite struct and function bodies. If a function calls another function, then rewriting the
caller should not mean also rewriting the callee. Previously we "fixed" this by religiously
wrapping references to types with TypeDef and doing other such hacks. But that's subtly wrong.
It only worked because Rewriter wasn't rewriting TypeRef.type. I think that Rewriter has to
rewrite that in the long run because it may refer to another TypeRef, and it may be an
instantiation that is using types that themselves need to be rewritten.

* WebGPUShadingLanguageRI/Checker.js:
(Checker.prototype.visitProtocolDecl.set throw):
* WebGPUShadingLanguageRI/NullType.js:
(NullType):
* WebGPUShadingLanguageRI/Rewriter.js:
(Rewriter.prototype.visitFuncDef):
(Rewriter.prototype.visitNativeFunc):
(Rewriter.prototype.visitNativeFuncInstance):
(Rewriter.prototype.visitNativeType):
(Rewriter.prototype.visitTypeDef):
(Rewriter.prototype.visitStructType):
(Rewriter.prototype.visitTypeVariable):
(Rewriter.prototype.visitConstexprTypeParameter):
(Rewriter.prototype.visitNativeTypeInstance):
(Rewriter.prototype.visitTypeRef):
* WebGPUShadingLanguageRI/Visitor.js:
(Visitor.prototype.visitNativeTypeInstance):

Modified Paths

Diff

Modified: trunk/Tools/ChangeLog (221486 => 221487)


--- trunk/Tools/ChangeLog	2017-09-01 18:47:41 UTC (rev 221486)
+++ trunk/Tools/ChangeLog	2017-09-01 18:48:27 UTC (rev 221487)
@@ -1,3 +1,37 @@
+2017-09-01  Filip Pizlo  <fpi...@apple.com>
+
+        WSL Rewriter should be an identity on things that aren't inside function bodies
+        https://bugs.webkit.org/show_bug.cgi?id=176208
+
+        Reviewed by Myles Maxfield.
+        
+        Previously, if the Rewriter encountered a FunctionDef, StructType, NativeType, etc., then it
+        would either crash or try to rewrite them. That's unfortunate because we use the Rewriter to
+        rewrite struct and function bodies. If a function calls another function, then rewriting the
+        caller should not mean also rewriting the callee. Previously we "fixed" this by religiously
+        wrapping references to types with TypeDef and doing other such hacks. But that's subtly wrong.
+        It only worked because Rewriter wasn't rewriting TypeRef.type. I think that Rewriter has to
+        rewrite that in the long run because it may refer to another TypeRef, and it may be an
+        instantiation that is using types that themselves need to be rewritten.
+
+        * WebGPUShadingLanguageRI/Checker.js:
+        (Checker.prototype.visitProtocolDecl.set throw):
+        * WebGPUShadingLanguageRI/NullType.js:
+        (NullType):
+        * WebGPUShadingLanguageRI/Rewriter.js:
+        (Rewriter.prototype.visitFuncDef):
+        (Rewriter.prototype.visitNativeFunc):
+        (Rewriter.prototype.visitNativeFuncInstance):
+        (Rewriter.prototype.visitNativeType):
+        (Rewriter.prototype.visitTypeDef):
+        (Rewriter.prototype.visitStructType):
+        (Rewriter.prototype.visitTypeVariable):
+        (Rewriter.prototype.visitConstexprTypeParameter):
+        (Rewriter.prototype.visitNativeTypeInstance):
+        (Rewriter.prototype.visitTypeRef):
+        * WebGPUShadingLanguageRI/Visitor.js:
+        (Visitor.prototype.visitNativeTypeInstance):
+
 2017-09-01  Alex Christensen  <achristen...@webkit.org>
 
         Disable ObjC WebGL policy SPI on iOS

Modified: trunk/Tools/WebGPUShadingLanguageRI/Checker.js (221486 => 221487)


--- trunk/Tools/WebGPUShadingLanguageRI/Checker.js	2017-09-01 18:47:41 UTC (rev 221486)
+++ trunk/Tools/WebGPUShadingLanguageRI/Checker.js	2017-09-01 18:48:27 UTC (rev 221487)
@@ -129,7 +129,7 @@
         let rhsType = node.rhs.visit(this);
         if (!lhsType.equals(rhsType))
             throw new WTypeError(node.origin.originString, "Type mismatch in assignment: " + lhsType + " versus " + rhsType);
-        node.type = TypeRef.wrap(lhsType);
+        node.type = lhsType;
         return lhsType;
     }
     
@@ -138,7 +138,7 @@
         let type = node.ptr.visit(this).unifyNode;
         if (!type.isPtr)
             throw new WTypeError(node.origin.originString, "Type passed to dereference is not a pointer: " + type);
-        node.type = TypeRef.wrap(type.elementType);
+        node.type = type.elementType;
         node.addressSpace = type.addressSpace;
         return node.type;
     }
@@ -237,7 +237,7 @@
             }
         }
         node.func = overload.func;
-        node.actualTypeArguments = overload.typeArguments.map(TypeRef.wrap);
+        node.actualTypeArguments = overload.typeArguments;
         let result = overload.func.returnType.substituteToUnification(
             overload.func.typeParameters, overload.unificationContext);
         if (!result)

Modified: trunk/Tools/WebGPUShadingLanguageRI/NullType.js (221486 => 221487)


--- trunk/Tools/WebGPUShadingLanguageRI/NullType.js	2017-09-01 18:47:41 UTC (rev 221486)
+++ trunk/Tools/WebGPUShadingLanguageRI/NullType.js	2017-09-01 18:48:27 UTC (rev 221487)
@@ -28,5 +28,6 @@
     // FIXME: This will have to behave like a type variable for the purposes of unification, so that
     // it can be unified with any pointer type. Then, we can do a verification at the end to see if
     // we got unified with a pointer type.
+    // https://bugs.webkit.org/show_bug.cgi?id=176235
 }
 

Modified: trunk/Tools/WebGPUShadingLanguageRI/Rewriter.js (221486 => 221487)


--- trunk/Tools/WebGPUShadingLanguageRI/Rewriter.js	2017-09-01 18:47:41 UTC (rev 221486)
+++ trunk/Tools/WebGPUShadingLanguageRI/Rewriter.js	2017-09-01 18:48:27 UTC (rev 221487)
@@ -47,6 +47,26 @@
         return oldItem;
     }
     
+    // We return identity for anything that is not inside a function/struct body. When processing
+    // function bodies, we only recurse into them and never out of them - for example if there is a
+    // function call to another function then we don't rewrite the other function. This is how we stop
+    // that.
+    visitFuncDef(node) { return node; }
+    visitNativeFunc(node) { return node; }
+    visitNativeFuncInstance(node) { return node; }
+    visitNativeType(node) { return node; }
+    visitTypeDef(node) { return node; }
+    visitStructType(node) { return node; }
+    visitTypeVariable(node) { return node; }
+    visitConstexprTypeParameter(node) { return node; }
+    
+    visitNativeTypeInstance(node)
+    {
+        return new NativeTypeInstance(
+            node.type.visit(this),
+            node.typeArguments.map(argument => argument.visit(this)));
+    }
+    
     visitFuncParameter(node)
     {
         let result = new FuncParameter(node.origin, node.name, node.type.visit(this));
@@ -92,9 +112,7 @@
     visitTypeRef(node)
     {
         let result = new TypeRef(node.origin, node.name, node.typeArguments.map(typeArgument => typeArgument.visit(this)));
-        // We should probably visit this type.
-        // https://bugs.webkit.org/show_bug.cgi?id=176208
-        result.type = node.type;
+        result.type = node.type ? node.type.visit(this) : null;
         return result;
     }
     

Modified: trunk/Tools/WebGPUShadingLanguageRI/Visitor.js (221486 => 221487)


--- trunk/Tools/WebGPUShadingLanguageRI/Visitor.js	2017-09-01 18:47:41 UTC (rev 221486)
+++ trunk/Tools/WebGPUShadingLanguageRI/Visitor.js	2017-09-01 18:48:27 UTC (rev 221487)
@@ -96,6 +96,13 @@
             typeParameter.visit(this);
     }
     
+    visitNativeTypeInstance(node)
+    {
+        node.type.visit(node);
+        for (let typeArgument of node.typeArguments)
+            typeArgument.visit(this);
+    }
+    
     visitTypeDef(node)
     {
         for (let typeParameter of node.typeParameters)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to