Bug 189290 - [WHLSL] It shouldn’t be possible to use ternary expressions as l-values
Summary: [WHLSL] It shouldn’t be possible to use ternary expressions as l-values
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGPU (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Thomas Denney
URL:
Keywords: InRadar
Depends on:
Blocks: 176199
  Show dependency treegraph
 
Reported: 2018-09-04 17:32 PDT by Thomas Denney
Modified: 2018-10-13 14:35 PDT (History)
5 users (show)

See Also:


Attachments
Patch (7.40 KB, patch)
2018-09-21 14:13 PDT, Thomas Denney
mmaxfield: review+
Details | Formatted Diff | Diff
Patch (8.40 KB, patch)
2018-09-24 17:06 PDT, Thomas Denney
no flags Details | Formatted Diff | Diff
Patch (8.40 KB, patch)
2018-09-24 17:08 PDT, Thomas Denney
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Thomas Denney 2018-09-04 17:32:16 PDT
Ternary expressions can be used as l-values:

    (someCondition ? a : b) = 42;

It should be possible to take the address of any l-value, however the following doesn’t work:

    int a = 42;
    int b = 43;
    thread int* c = (someCondition ? a : b);

We should either disallow taking the address of a ternary expression, improve the current error message “Bad address space: undefined”, or support this.
Comment 1 Thomas Denney 2018-09-04 17:38:56 PDT
(In reply to Thomas Denney from comment #0)
> Ternary expressions can be used as l-values:
> 
>     (someCondition ? a : b) = 42;
> 
> It should be possible to take the address of any l-value, however the
> following doesn’t work:
> 
>     int a = 42;
>     int b = 43;
>     thread int* c = (someCondition ? a : b);

Sorry, that should be

      thread int* c = &(someCondition ? a : b);

> 
> We should either disallow taking the address of a ternary expression,
> improve the current error message “Bad address space: undefined”, or support
> this.
Comment 2 Thomas Denney 2018-09-04 17:40:47 PDT
It is worth noting that C doesn’t permit this because it treats the ternary expression as an r-value, and therefore you cannot take its address.
Comment 3 Thomas Denney 2018-09-04 17:47:54 PDT
We will therefore disallow:
Comment 4 Thomas Denney 2018-09-04 17:48:37 PDT
Therefore the following needs to be disallowed:

    (someCondition ? a : b) = something
    &(someCondition ? a : b)
Comment 5 Myles C. Maxfield 2018-09-04 17:50:30 PDT
(In reply to Thomas Denney from comment #4)
> Therefore the following needs to be disallowed:
> 
>     (someCondition ? a : b) = something
>     &(someCondition ? a : b)

So ternary expressions will never be lvalues. Makes it easy.
Comment 6 Thomas Denney 2018-09-21 14:13:44 PDT
Created attachment 350425 [details]
Patch
Comment 7 Myles C. Maxfield 2018-09-24 15:57:55 PDT
Comment on attachment 350425 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=350425&action=review

Should also remove the block from NormalUsePropertyResolver.

> Tools/WebGPUShadingLanguageRI/TernaryExpression.js:-41
> -    get isLValue() { return this._isLValue; }
> -    set isLValue(value) { this._isLValue = value; }

Similar to how CommaExpression.js has a comment about why it isn't an lvalue, a comment here would be appreciated.
Comment 8 Thomas Denney 2018-09-24 17:06:18 PDT
Created attachment 350713 [details]
Patch
Comment 9 Thomas Denney 2018-09-24 17:08:48 PDT
Created attachment 350714 [details]
Patch
Comment 10 EWS 2018-09-24 17:09:51 PDT
Comment on attachment 350714 [details]
Patch

Rejecting attachment 350714 [details] from commit-queue.

tdenney@apple.com does not have committer permissions according to https://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/contributors.json.

- If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.

- If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/contributors.json by adding yourself to the file (no review needed).  The commit-queue restarts itself every 2 hours.  After restart the commit-queue will correctly respect your committer rights.
Comment 11 WebKit Commit Bot 2018-09-24 18:16:23 PDT
Comment on attachment 350714 [details]
Patch

Clearing flags on attachment: 350714

Committed r236449: <https://trac.webkit.org/changeset/236449>
Comment 12 Radar WebKit Bug Importer 2018-09-24 18:47:25 PDT
<rdar://problem/44748793>
Comment 13 Myles C. Maxfield 2018-10-13 14:35:55 PDT
Migrated to https://github.com/gpuweb/WHLSL/issues/43