-
Notifications
You must be signed in to change notification settings - Fork 33
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
Big update to flesh out const instance methods #34
base: main
Are you sure you want to change the base?
Big update to flesh out const instance methods #34
Conversation
I started digging a bit into this in DXC and identified a lot of cross-cutting issues that we need to consider in this. This change updates the proposal to flesh out the behavior and related issues around HLSL const-correctness. This does identify some source-breaking changes that this would introduce as well as some behavior changes that I think we should introduce in the HLSL built-in types.
After this PR has been inactive for almost a year now: Are the plans to bring const qualifiers to instance methods still valid? At the moment, I have to compile a lot of shared code (C++ & HLSL) by removing the const keyword with the preprocessor ( For the time being, I consider hacking in the If I know that at some point in the future const-methods will be supported, I would be more inclined to accept the hack mentioned above as a temporary measure. |
@ChristianReinbold, thank you for your interest in the HLSL language.
Our team is very small and our focus has been on Shader Model 6.8 rather than HLSL 202x over the last year. Which may seem like there is no progress here, but we are still planning this as a likely feature for the next HLSL language version.
This is going to be a really difficult thing to do with HLSL as the language exists today. There's a lot of differences between HLSL and C++, so this use case isn't something we actively support with HLSL today (although we are working to support it in the future).
I assume this is your justification for why the hack you describe below is "ok", but your specific use case isn't the same as others so it isn't something we can bank on people doing. Most HLSL code that exists today isn't shared source with C++.
It works by parsing the const qualifier, but it doesn't fix the underlying issues with overload resolution. Today our overload resolution ignores const-qualification, and we actually allow calling non-const methods on constant objects in some contexts because of hacks on top of hacks...
Fundamentally it is a change to the language grammar and syntax. All changes to language grammar should be gated on a language version and/or feature check.
Two points here: (1) This isn't the place to talk about an implementation detail. This PR and the underlying proposal are about changing the language, not the compiler implementation. (2) This hack would basically make it so that people writing code would write it in a way that gave them a reasonable expectation that it worked in one way, but it would actually work differently from expectations. That's the kind of behavior we're trying to remove from HLSL because we have too many instances of it. I know it isn't satisfying, but the only answer today for people trying to share code between HLSL and C++ is to keep using preprocessor macros to morph the code between the languages. For this specific feature you'll need to wait until HLSL 202x begins development. |
Thanks for the detailed answer, which - for me - is perfectly satisfying. My intention when writing this comment was about managing expectations, and in this regard it helped a lot. I now have a better idea which rabbit hole we may continue to go down with the current code-base.
I highly appreciate the work you are putting into HLSL, and I am genuinely impressed how well the feature set of HLSL 2021 worked out in practice so far, for a multitude of reasons. I guess we are just testing the limits with our use case. I hope you are fine with me inquiring here and there about features I would be interested in. The current approach of peeking into HLSL specs and dxc sources whenever struggling with unexpected behavior or compiler bugs / limitations, "fixing" them and upstreaming them whenever possible seems to work quite well. As long as you are happy to spend some time with irregular PRs that have the risk of not accounting for the whole picture and thus heading the wrong way sometimes (in the end my compiler background is limited to non-existant), I probably will continue with this approach.
I am aware. I intended to give you a more complete picture about the implications this language feature has for us. I am sorry for having side-tracked the conversation with that.
I agree. I do not consider bringing in this hack as a PR to the dxc repo. |
I started digging a bit into this in DXC and identified a lot of cross-cutting issues that we need to consider in this.
This change updates the proposal to flesh out the behavior and related issues around HLSL const-correctness.
This does identify some source-breaking changes that this would introduce as well as some behavior changes that I think we should introduce in the HLSL built-in types.