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+

Description Myles C. Maxfield 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);
Comment 1 Myles C. Maxfield 2017-09-03 11:50:40 PDT
And of course, what about
operator Foo[](Bar);
Comment 2 Myles C. Maxfield 2017-09-03 13:21:15 PDT
operator T<T>(T);
Comment 3 Myles C. Maxfield 2017-09-05 18:24:51 PDT
Created attachment 319966 [details]
WIP
Comment 4 Myles C. Maxfield 2017-09-05 20:43:56 PDT
Created attachment 319976 [details]
WIP
Comment 5 Myles C. Maxfield 2017-09-05 21:24:41 PDT
Created attachment 319980 [details]
Patch
Comment 6 Myles C. Maxfield 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.
Comment 7 Myles C. Maxfield 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;
}
Comment 8 Myles C. Maxfield 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;
}
Comment 9 Myles C. Maxfield 2017-09-05 22:18:35 PDT
Created attachment 319982 [details]
Patch
Comment 10 Myles C. Maxfield 2017-09-05 22:28:32 PDT
Created attachment 319985 [details]
Patch
Comment 11 Filip Pizlo 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?
Comment 12 Myles C. Maxfield 2017-09-06 11:26:29 PDT
Committed r221686: <http://trac.webkit.org/changeset/221686>
Comment 13 Radar WebKit Bug Importer 2017-09-27 12:38:21 PDT
<rdar://problem/34693661>
Comment 14 Myles C. Maxfield 2018-10-13 17:03:27 PDT
Migrated to https://github.com/gpuweb/WHLSL/issues/155