Title: [222210] trunk/Tools
- Revision
- 222210
- Author
- [email protected]
- Date
- 2017-09-19 09:38:37 -0700 (Tue, 19 Sep 2017)
Log Message
Cannot put typedefs of templated structs inside structs
https://bugs.webkit.org/show_bug.cgi?id=177123
Reviewed by Michael Saboff.
Ever since I wrote the generic type instantiation code in WSL, it had a bizarre stink to it. I
did not know what it was, until I met this test case:
struct Foo { int2 x; }
The problem was that Type has a getter called instantiatedType, which invoked
InstantiateImmediates. That's great. Where we went wrong was that TypeRef overrode
instantiatedType, and then InstantiateImmediates called TypeRef.instantiatedType from its
visitTypeRef. What a mess. None of that was necessary for anything, and it was just wrong. This
patch fixes it so that:
- Type has an instantiatedType getter, which does InstantiateImmediates. I didn't change this.
This was the only part of the status quo that really made sense.
- TypeRef no longer overrides instantiatedType.
- InstantianteImmediates.prototype.visitTypeRef now does what TypeRef's instantiatedType would
have done.
And voila! This test case passes and so do all of the other test cases.
The reason why that specific test case did not work was that the use of a TypeDef (int2) for
a generic type instantiation (vec2<int>) caused there to be two TypeRef's in a row, sorta like
this, if we could use wrap() to denote a TypeRef that just wrapped another type:
wrap(vec2<int>)
We would call instantiatedType on this monstrosity. Since TypeRef overrode instantiatedType,
we would stop there because that TypeRef has no type arguments - it's just a wrapper. Well,
actually, we'd do something a bit stranger than stopping there - there's more to the story -
but the effect was the same. Now we do InstantiateImmediates, which recursively builds a new
type. This chews through the wrapper and then instantiates the wrapped type, as we want it to.
Also, this approach no longer has the stink. instantiatedType isn't overriden by anyone
anymore!
* WebGPUShadingLanguageRI/Checker.js:
* WebGPUShadingLanguageRI/InstantiateImmediates.js:
(InstantiateImmediates.prototype.visitTypeRef):
(InstantiateImmediates.prototype.visitReferenceType): Deleted.
* WebGPUShadingLanguageRI/Test.js:
(TEST_instantiateStructInStructWithInt2):
* WebGPUShadingLanguageRI/TypeRef.js:
(TypeRef.wrap):
(TypeRef.prototype.setTypeAndArguments):
(TypeRef.prototype.toString):
(TypeRef):
(TypeRef.prototype.get instantiatedType): Deleted.
Modified Paths
Diff
Modified: trunk/Tools/ChangeLog (222209 => 222210)
--- trunk/Tools/ChangeLog 2017-09-19 16:03:09 UTC (rev 222209)
+++ trunk/Tools/ChangeLog 2017-09-19 16:38:37 UTC (rev 222210)
@@ -1,3 +1,58 @@
+2017-09-18 Filip Pizlo <[email protected]>
+
+ Cannot put typedefs of templated structs inside structs
+ https://bugs.webkit.org/show_bug.cgi?id=177123
+
+ Reviewed by Michael Saboff.
+
+ Ever since I wrote the generic type instantiation code in WSL, it had a bizarre stink to it. I
+ did not know what it was, until I met this test case:
+
+ struct Foo { int2 x; }
+
+ The problem was that Type has a getter called instantiatedType, which invoked
+ InstantiateImmediates. That's great. Where we went wrong was that TypeRef overrode
+ instantiatedType, and then InstantiateImmediates called TypeRef.instantiatedType from its
+ visitTypeRef. What a mess. None of that was necessary for anything, and it was just wrong. This
+ patch fixes it so that:
+
+ - Type has an instantiatedType getter, which does InstantiateImmediates. I didn't change this.
+ This was the only part of the status quo that really made sense.
+
+ - TypeRef no longer overrides instantiatedType.
+
+ - InstantianteImmediates.prototype.visitTypeRef now does what TypeRef's instantiatedType would
+ have done.
+
+ And voila! This test case passes and so do all of the other test cases.
+
+ The reason why that specific test case did not work was that the use of a TypeDef (int2) for
+ a generic type instantiation (vec2<int>) caused there to be two TypeRef's in a row, sorta like
+ this, if we could use wrap() to denote a TypeRef that just wrapped another type:
+
+ wrap(vec2<int>)
+
+ We would call instantiatedType on this monstrosity. Since TypeRef overrode instantiatedType,
+ we would stop there because that TypeRef has no type arguments - it's just a wrapper. Well,
+ actually, we'd do something a bit stranger than stopping there - there's more to the story -
+ but the effect was the same. Now we do InstantiateImmediates, which recursively builds a new
+ type. This chews through the wrapper and then instantiates the wrapped type, as we want it to.
+ Also, this approach no longer has the stink. instantiatedType isn't overriden by anyone
+ anymore!
+
+ * WebGPUShadingLanguageRI/Checker.js:
+ * WebGPUShadingLanguageRI/InstantiateImmediates.js:
+ (InstantiateImmediates.prototype.visitTypeRef):
+ (InstantiateImmediates.prototype.visitReferenceType): Deleted.
+ * WebGPUShadingLanguageRI/Test.js:
+ (TEST_instantiateStructInStructWithInt2):
+ * WebGPUShadingLanguageRI/TypeRef.js:
+ (TypeRef.wrap):
+ (TypeRef.prototype.setTypeAndArguments):
+ (TypeRef.prototype.toString):
+ (TypeRef):
+ (TypeRef.prototype.get instantiatedType): Deleted.
+
2017-09-19 Per Arne Vollan <[email protected]>
[Win] WebKit fails to build with 64-bit Perl.
Modified: trunk/Tools/WebGPUShadingLanguageRI/Checker.js (222209 => 222210)
--- trunk/Tools/WebGPUShadingLanguageRI/Checker.js 2017-09-19 16:03:09 UTC (rev 222209)
+++ trunk/Tools/WebGPUShadingLanguageRI/Checker.js 2017-09-19 16:38:37 UTC (rev 222210)
@@ -53,7 +53,7 @@
doStatement(func);
}
}
-
+
visitFuncDef(node)
{
node.body.visit(this);
Modified: trunk/Tools/WebGPUShadingLanguageRI/InstantiateImmediates.js (222209 => 222210)
--- trunk/Tools/WebGPUShadingLanguageRI/InstantiateImmediates.js 2017-09-19 16:03:09 UTC (rev 222209)
+++ trunk/Tools/WebGPUShadingLanguageRI/InstantiateImmediates.js 2017-09-19 16:38:37 UTC (rev 222210)
@@ -28,8 +28,12 @@
visitTypeRef(node)
{
node = super.visitTypeRef(node);
- let result = node.instantiatedType.visit(new AutoWrapper());
- return result;
+ if (!node.type.instantiate) {
+ if (node.typeArguments.length)
+ throw new Error("type does not support instantiation: " + type + " (" + type.constructor.name + ")");
+ return node;
+ }
+ return node.type.instantiate(node.typeArguments).visit(new AutoWrapper());
}
visitReferenceType(node)
Modified: trunk/Tools/WebGPUShadingLanguageRI/Test.js (222209 => 222210)
--- trunk/Tools/WebGPUShadingLanguageRI/Test.js 2017-09-19 16:03:09 UTC (rev 222209)
+++ trunk/Tools/WebGPUShadingLanguageRI/Test.js 2017-09-19 16:38:37 UTC (rev 222210)
@@ -3922,6 +3922,23 @@
checkInt(program, callFunction(program, "foo", [], []), 43);
}
+function TEST_instantiateStructInStructWithInt2()
+{
+ let program = doPrep(`
+ struct Foo {
+ int2 x;
+ }
+ int foo()
+ {
+ Foo x;
+ x.x.x = 42;
+ x.x.x++;
+ return x.x.x;
+ }
+ `);
+ checkInt(program, callFunction(program, "foo", [], []), 43);
+}
+
function TEST_simpleEnum()
{
let program = doPrep(`
Modified: trunk/Tools/WebGPUShadingLanguageRI/TypeRef.js (222209 => 222210)
--- trunk/Tools/WebGPUShadingLanguageRI/TypeRef.js 2017-09-19 16:03:09 UTC (rev 222209)
+++ trunk/Tools/WebGPUShadingLanguageRI/TypeRef.js 2017-09-19 16:38:37 UTC (rev 222210)
@@ -38,7 +38,7 @@
{
if (type instanceof TypeRef && !type.typeArguments)
return type;
- let result = new TypeRef(type.origin, type.name, []);
+ let result = new TypeRef(type.origin, null, []);
result.type = type;
return result;
}
@@ -54,17 +54,6 @@
get name() { return this._name; }
get typeArguments() { return this._typeArguments; }
- get instantiatedType()
- {
- let type = this.type.unifyNode;
- if (!type.instantiate) {
- if (this.typeArguments.length)
- throw new Error("type does not support instantiation: " + type + " (" + type.constructor.name + ")");
- return this;
- }
- return type.instantiate(this.typeArguments);
- }
-
get unifyNode()
{
if (!this.typeArguments.length)
@@ -95,7 +84,7 @@
setTypeAndArguments(type, typeArguments)
{
- this._name = type.name;
+ this._name = null;
this.type = type;
this._typeArguments = typeArguments;
}
@@ -118,7 +107,7 @@
toString()
{
if (!this.name)
- return "ref:" + this.type.toString();
+ return this.type.toString();
if (!this.typeArguments.length)
return this.name;
return this.name + "<" + this.typeArguments + ">";
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
