Bug 189290 – [WHLSL] It shouldn’t be possible to use ternary expressions as l-values

• Status: RESOLVED
• Resolution: FIXED
• Component: WebGPU

[Thomas Denney]

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.

[Thomas Denney]

(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.

[Thomas Denney]

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.

[Thomas Denney]

We will therefore disallow:

[Thomas Denney]

Therefore the following needs to be disallowed:

    (someCondition ? a : b) = something
    &(someCondition ? a : b)

[Myles C. Maxfield]

(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.

[Thomas Denney]

Created attachment 350425 [details]
Patch

[Myles C. Maxfield]

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.

[Thomas Denney]

Created attachment 350713 [details]
Patch

[Thomas Denney]

Created attachment 350714 [details]
Patch

[EWS]

Comment on attachment 350714 [details]
Patch

Rejecting attachment 350714 [details] from commit-queue.

[e-mail] 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.

[WebKit Commit Bot]

Comment on attachment 350714 [details]
Patch

Clearing flags on attachment: 350714

Committed r236449: <https://trac.webkit.org/changeset/236449>

[Radar WebKit Bug Importer]

<rdar://problem/44748793>

[Myles C. Maxfield]

Migrated to https://github.com/gpuweb/WHLSL/issues/43