Bug 176316

Summary: WSL should correctly handle the interaction between casting operators and complex types
Product: WebKit Reporter: Myles C. Maxfield <mmaxfield>
Component: WebGPUAssignee: Myles C. Maxfield <mmaxfield>
Status: RESOLVED FIXED    
Severity: Normal CC: fpizlo, jfbastien, keith_miller, saam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 176199    
Attachments:
Description Flags
WIP
none
WIP
none
Patch
none
Patch
none
Patch fpizlo: review+

Myles C. Maxfield
Reported 2017-09-03 11:48:44 PDT
For example, we need to figure out what to do if: typedef Foo = Bar; operator Foo(Baz); operator Bar(Baz); The following should also probably work (maybe?) typedef Foo = Bar[] operator Foo(Baz);
Attachments
WIP (18.55 KB, patch)
2017-09-05 18:24 PDT, Myles C. Maxfield 		no flags
DetailsFormatted DiffDiff
WIP (18.83 KB, patch)
2017-09-05 20:43 PDT, Myles C. Maxfield 		no flags
DetailsFormatted DiffDiff
Patch (22.77 KB, patch)
2017-09-05 21:24 PDT, Myles C. Maxfield 		no flags
DetailsFormatted DiffDiff
Patch (23.45 KB, patch)
2017-09-05 22:18 PDT, Myles C. Maxfield 		no flags
DetailsFormatted DiffDiff
Patch (24.69 KB, patch)
2017-09-05 22:28 PDT, Myles C. Maxfield 		fpizlo: review+
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Myles C. Maxfield
Comment 1 2017-09-03 11:50:40 PDT
And of course, what about operator Foo[](Bar);
Myles C. Maxfield
Comment 2 2017-09-03 13:21:15 PDT
operator T<T>(T);
Myles C. Maxfield
Comment 3 2017-09-05 18:24:51 PDT
Created attachment 319966 [details] WIP
Myles C. Maxfield
Comment 4 2017-09-05 20:43:56 PDT
Created attachment 319976 [details] WIP
Myles C. Maxfield
Comment 5 2017-09-05 21:24:41 PDT
Created attachment 319980 [details] Patch
Myles C. Maxfield
Comment 6 2017-09-05 21:33:00 PDT
I guess I could test this more if I add operator bool<><T>(T x) { T defaultValue; return x != defaultValue; } Then I could test that !7 is false and !0 is true.
Myles C. Maxfield
Comment 7 2017-09-05 22:04:35 PDT
(In reply to Myles C. Maxfield from comment #6) > I guess I could test this more if I add > > operator bool<><T>(T x) { > T defaultValue; > return x != defaultValue; > } > > Then I could test that !7 is false and !0 is true. Actually, it looks like I can't do this unless I add some sort of Comparable protocol. It would have to be: protocol Equatable { bool operator==(Equatable, Equatable); } operator bool<><T:Comparable>(T x) { T defaultValue; return x != defaultValue; }
Myles C. Maxfield
Comment 8 2017-09-05 22:05:10 PDT
(In reply to Myles C. Maxfield from comment #7) > (In reply to Myles C. Maxfield from comment #6) > > I guess I could test this more if I add > > > > operator bool<><T>(T x) { > > T defaultValue; > > return x != defaultValue; > > } > > > > Then I could test that !7 is false and !0 is true. > > Actually, it looks like I can't do this unless I add some sort of Comparable > protocol. It would have to be: > > protocol Equatable { > bool operator==(Equatable, Equatable); > } > > operator bool<><T:Comparable>(T x) { > T defaultValue; > return x != defaultValue; > } Whoops, got this wrong: protocol Equatable { bool operator==(Equatable, Equatable); } operator bool<><T:Equatable>(T x) { T defaultValue; return x != defaultValue; }
Myles C. Maxfield
Comment 9 2017-09-05 22:18:35 PDT
Created attachment 319982 [details] Patch
Myles C. Maxfield
Comment 10 2017-09-05 22:28:32 PDT
Created attachment 319985 [details] Patch
Filip Pizlo
Comment 11 2017-09-06 10:28:49 PDT
Comment on attachment 319985 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=319985&action=review r=me with comments > Tools/WebGPUShadingLanguageRI/Checker.js:237 > + checkCastOrCallExpression(node, cast) Let's make "cast" not be a boolean. It seems that the only use of cast is to deal with the returnType. What if you replaced "cast" with "returnType", and visitCastExpression would pass node.returnType while visitCallExpression passes null. > Tools/WebGPUShadingLanguageRI/ResolveOverloadImpl.js:68 > + if (returnType) { > + if (!returnType.unify(unificationContext, func.returnType)) { > + failures.push(new OverloadResolutionFailure(func, "Return type " + func.returnType + " does not match " + returnType)); > + continue; > + } > + } Nice > Tools/WebGPUShadingLanguageRI/StandardLibrary.js:75 > +const standardLibraryEpilogue = ` > +operator bool<><T:Equatable>(T x) { > + T defaultValue; > + return x != defaultValue; > +} > +`; Maybe move this to StandardLibraryEpilogue, so that the start line number of it doesn't depend on the length of the prologue?
Myles C. Maxfield
Comment 12 2017-09-06 11:26:29 PDT
Committed r221686: <http://trac.webkit.org/changeset/221686>
Radar WebKit Bug Importer
Comment 13 2017-09-27 12:38:21 PDT
<rdar://problem/34693661>
Myles C. Maxfield
Comment 14 2018-10-13 17:03:27 PDT
Migrated to https://github.com/gpuweb/WHLSL/issues/155
Note You need to log in before you can comment on or make changes to this bug.