-
Notifications
You must be signed in to change notification settings - Fork 50
[NNNN] proposal to disallow explicit conversions for inout params #643
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
||
## Introduction | ||
|
||
DXC allows explicitly casting an array of type T to a vector of type T as an inout parameter. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would swap the contents of the "Proposed Solution" and "Introduction" sections.
A one sentence introduction gives a good summary of the proposal, and your examples here give a more concrete understanding of the proposal and why it matters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I sort of did that, but ended up moving the examples to motivation, but I was still more verbose in the introduction.
Co-authored-by: Damyan Pepper <damyanp@microsoft.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Talked with Sarah offline, had more time to understand the proposal better.
@@ -0,0 +1,112 @@ | |||
--- | |||
title: "NNNN - Disallow all explicit conversions in inout parameters" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider changing the proposal to be more general, like "Disallow all explicit conversions from producing an LValue"
|
||
DXC inconsistently allows some C style explicit casts to produce LValues and be | ||
used as arguments for inout parameters. However, C style explicit casts do not | ||
produce LValues and inout parameters require LValues. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding that HLSL should mimic C++ behavior as closely as possible, so because C++ doesn't allow LValues as a result of these casts, HLSL shouldn't either.
A proposal to disallow explicit conversions for inout parameters.