Bug 189290

Summary: [WHLSL] It shouldn’t be possible to use ternary expressions as l-values
Product: WebKit Reporter: Thomas Denney <tdenney>
Component: WebGPUAssignee: Thomas Denney <tdenney>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, dino, ews-feeder, mmaxfield, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 176199    
Attachments:
Description Flags
Patch
mmaxfield: review+
Patch
none
Patch none

Thomas Denney
Reported 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.
Attachments
Patch (7.40 KB, patch)
2018-09-21 14:13 PDT, Thomas Denney 		mmaxfield: review+
DetailsFormatted DiffDiff
Patch (8.40 KB, patch)
2018-09-24 17:06 PDT, Thomas Denney 		no flags
DetailsFormatted DiffDiff
Patch (8.40 KB, patch)
2018-09-24 17:08 PDT, Thomas Denney 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Thomas Denney
Comment 1 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.
Thomas Denney
Comment 2 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.
Thomas Denney
Comment 3 2018-09-04 17:47:54 PDT
We will therefore disallow:
Thomas Denney
Comment 4 2018-09-04 17:48:37 PDT
Therefore the following needs to be disallowed: (someCondition ? a : b) = something &(someCondition ? a : b)
Myles C. Maxfield
Comment 5 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.
Thomas Denney
Comment 6 2018-09-21 14:13:44 PDT
Created attachment 350425 [details] Patch
Myles C. Maxfield
Comment 7 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.
Thomas Denney
Comment 8 2018-09-24 17:06:18 PDT
Created attachment 350713 [details] Patch
Thomas Denney
Comment 9 2018-09-24 17:08:48 PDT
Created attachment 350714 [details] Patch
EWS
Comment 10 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.
WebKit Commit Bot
Comment 11 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>
Radar WebKit Bug Importer
Comment 12 2018-09-24 18:47:25 PDT
<rdar://problem/44748793>
Myles C. Maxfield
Comment 13 2018-10-13 14:35:55 PDT
Migrated to https://github.com/gpuweb/WHLSL/issues/43
Note You need to log in before you can comment on or make changes to this bug.