RESOLVED FIXED Bug 176285
WSL should support the bool type
https://bugs.webkit.org/show_bug.cgi?id=176285
Summary WSL should support the bool type
Filip Pizlo
Reported 2017-09-02 17:24:48 PDT
It should be declared as a native type. Then there should be a BoolLiteral that works more-or-less like the current UintLiteral. This unlocks the ability to add control flow constructs.
Attachments
WIP (18.48 KB, patch)
2017-09-03 12:20 PDT, Myles C. Maxfield
no flags
WIP (21.46 KB, patch)
2017-09-03 13:00 PDT, Myles C. Maxfield
no flags
WIP (27.18 KB, patch)
2017-09-03 23:16 PDT, Myles C. Maxfield
no flags
Patch (39.09 KB, patch)
2017-09-04 12:17 PDT, Myles C. Maxfield
fpizlo: review+
Filip Pizlo
Comment 1 2017-09-02 18:01:25 PDT
Also, it seems like it would be best to remove the notion that operator! is a customizable operator. Instead, go straight to explicit operator bool. In this world, operator! is built-in. operator! and all of the conditional contexts ("if (_)", "while (_)", and "for (;_;)") are parsed so that they first emit a call to "operator bool(T)". Essentially we can turn this: !e into this: !(operator bool(e)) Where you can declare an operator bool for whatever types you like. We could declare a default one that looks like this: operator bool<T>(T value) { T defaultValue; return value != defaultValue; } Note that a!=b is parsed as !(a==b). Currently override resolution is order-based: we parse the signatures in order and we pick the first one that matches. We could say that the standard library has a suffix portion that is parsed after the program, which contains the following: operator bool<T>(T value) { T defaultValue; return value != defaultValue; } native bool operator==<T>(T a, T b); // Does bitwise equality By making this a suffix, it means that the programmer can always rely on these as defaults but can easily override them for their type. So, in this world, this totally works. struct MyPoint { int x; int y; } void foo() { MyPoint p; ... if (p) do stuff if p is not the origin } Does this seem useful, or overkill? Do you want to try to do this?
Myles C. Maxfield
Comment 2 2017-09-03 12:20:06 PDT
Myles C. Maxfield
Comment 3 2017-09-03 12:22:53 PDT
(In reply to Filip Pizlo from comment #1) > > operator bool<T>(T value) > { > T defaultValue; > return value != defaultValue; > } You aren't placing any type restrictions on T. Does this mean that we will require every type in the whole language to be default-constructible?
Filip Pizlo
Comment 4 2017-09-03 12:25:37 PDT
(In reply to Myles C. Maxfield from comment #3) > (In reply to Filip Pizlo from comment #1) > > > > operator bool<T>(T value) > > { > > T defaultValue; > > return value != defaultValue; > > } > > You aren't placing any type restrictions on T. Does this mean that we will > require every type in the whole language to be default-constructible? That's already true, since we don't have constructors or destructors or any member functions. Structs have a default value (just give all fields the default value). Arrays have a default value (just give all array elements the default value).
Myles C. Maxfield
Comment 5 2017-09-03 13:00:57 PDT
Myles C. Maxfield
Comment 6 2017-09-03 13:06:56 PDT
(In reply to Filip Pizlo from comment #4) > (In reply to Myles C. Maxfield from comment #3) > > (In reply to Filip Pizlo from comment #1) > > > > > > operator bool<T>(T value) > > > { > > > T defaultValue; > > > return value != defaultValue; > > > } > > > > You aren't placing any type restrictions on T. Does this mean that we will > > require every type in the whole language to be default-constructible? > > That's already true, since we don't have constructors or destructors or any > member functions. Structs have a default value (just give all fields the > default value). Arrays have a default value (just give all array elements > the default value). ArrayRefs will be null?
Filip Pizlo
Comment 7 2017-09-03 13:27:55 PDT
(In reply to Myles C. Maxfield from comment #6) > (In reply to Filip Pizlo from comment #4) > > (In reply to Myles C. Maxfield from comment #3) > > > (In reply to Filip Pizlo from comment #1) > > > > > > > > operator bool<T>(T value) > > > > { > > > > T defaultValue; > > > > return value != defaultValue; > > > > } > > > > > > You aren't placing any type restrictions on T. Does this mean that we will > > > require every type in the whole language to be default-constructible? > > > > That's already true, since we don't have constructors or destructors or any > > member functions. Structs have a default value (just give all fields the > > default value). Arrays have a default value (just give all array elements > > the default value). > > ArrayRefs will be null? Word. Zero length and zero base pointer.
Myles C. Maxfield
Comment 8 2017-09-03 23:16:52 PDT
Myles C. Maxfield
Comment 9 2017-09-04 12:17:01 PDT
Filip Pizlo
Comment 10 2017-09-04 13:01:26 PDT
Comment on attachment 319858 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=319858&action=review r=me. Would be good to clean up the way the parser and leader interact. It’s easier in the long run if the lexer doesn’t know too much about keywords. Please also fix the WTypeError issue and the origin issue. The rest (like naming) is optional. > Tools/WebGPUShadingLanguageRI/Checker.js:229 > + throw new Error("Trying to negate something that isn't a bool: " + node.value); This should be a WTypeError since this case can be reached with bad input. (The Error above is fine because that will only happen with a bug in our code.) > Tools/WebGPUShadingLanguageRI/Intrinsics.js:209 > + throw new WTypeError(thing.toString(), "Unrecognized intrinsic: " + thing); The first arg should be the origin string since that’s what we do everywhere else. Can we do that here? > Tools/WebGPUShadingLanguageRI/Lexer.js:106 > + if (isAddressSpace(RegExp.lastMatch)) Why can’t this get handled as a keyword? If it’s not a keyword, why are address spaces still in the keyword regezp? > Tools/WebGPUShadingLanguageRI/Lexer.js:108 > + if (["true", "false"].includes(RegExp.lastMatch)) Ditto. Compilers usually recognize all such things as keywords. It’s up to the parser to decide how to categorize keywords. > Tools/WebGPUShadingLanguageRI/LogicalNegationExpression.js:27 > +class LogicalNegationExpression extends Expression { This is usually called LogicalNot > Tools/WebGPUShadingLanguageRI/Parse.js:188 > + if (token = tryConsumeKind("boolLiteral")) You could have said tryConsume(“true”, “false”)
Filip Pizlo
Comment 11 2017-09-04 13:44:10 PDT
Comment on attachment 319858 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=319858&action=review > Tools/WebGPUShadingLanguageRI/Parse.js:629 > + result.name = "operator " + castType; Oh now I get why TypeDef resolution doesn't work! Ha! Yeah, the way to get around this (which you can do in a separate patch) is: - Make these guys always be called "operator cast". That is always the name. - Thread returnType through resolveOverloadImpl and everyone who calls it. - This must be null if calling anything other than "operator cast". - When calling "operator cast", Checker knows the desired return type, and so it will pass that as the returnType. - resolveOverloadImpl will attempt unification on the returnType if it is given one. It will do this the same way it does for argument types. You may want a subclass of CallExpression for these casts, since those call expressions know the desired return type syntactically. Visitors and the Rewriter will have to visit and rewrite that desired return type. If you do that, then TypeDefs will magically resolve themselves because the TypeDef resolver will simply find the TypeDef in the returnType and replace it will something else. > Tools/WebGPUShadingLanguageRI/StandardLibrary.js:56 > +native operator int(int); > +native operator uint(uint); > +native operator bool(bool); Why are there self casts? Shouldn't they be provided by the language?
Myles C. Maxfield
Comment 12 2017-09-05 14:12:22 PDT
Myles C. Maxfield
Comment 13 2017-09-06 12:34:45 PDT
Radar WebKit Bug Importer
Comment 14 2017-09-27 12:39:14 PDT
Myles C. Maxfield
Comment 15 2018-10-13 17:07:45 PDT
Note You need to log in before you can comment on or make changes to this bug.