Skip to content

[Feature Request] Add a flag to disable implicit conversions for inout, in and out #6858

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

Closed
devshgraphicsprogramming opened this issue Aug 13, 2024 · 1 comment
Labels
enhancement Feature suggestion needs-triage Awaiting triage

Comments

@devshgraphicsprogramming

User defined conversion operations don't work as expected (operator T() is actually never called), only implicit conversions happen.

While this is okay when lets say assigning variables, its a nightmare when calling functions.

Also all this sits on DXC's rather flaky overload resolution.

Is your feature request related to a problem? Please describe.

This sort code compiles without any warning
https://godbolt.org/z/z4c74TEjW
https://godbolt.org/z/84KsEbPvT

Yes in and out aren't supposed to be references, however I'd appreciate not letting such hidden conversions happen in a language thats supposed to resemble C or C++ (with hopefully some strong typing) and implement on the Clang stack.

This is the sort of stuff that I'd have to care about to not get caught out in Python or JavaScript.

Describe the solution you'd like

-fno-implicit-argument-conversions which disables conversions happenning for in, out and inout function arguments.

It would still be useful to allow implicit copies for arguments not decorated with anything.

Also for that to kick in without breaking SFINAE/overload-resolution (so one can use to detect presence / pick the correct overload to call).

The very least I'd want is a warning whenever such a conversion happens behind my back.

Describe alternatives you've considered

Nothing really cuts it for this usecase.

Additional context

This has come up before when I was trying to write "method presence" detectors with SFINAE and due to there being no constexpr function reference type, the only way we could test for a method presence was with checking things like sizeof(declval<S>().someMethod(declval<Arg0>())).

However the problem was that due to implicit conversions for argument inputs, nearly anything matches float someMethod(in uint32_t)

@llvm-beanz
Copy link
Collaborator

TL;DR: We're not going to add flags to DXC to magically change language behavior. That dramatically increases complexity for testing and other language features.

Below I've provided a detailed breakdown of where the language problems are here, the path forward for HLSL, and lots of details why we won't add this flag.

User defined conversion operations don't work as expected (operator T() is actually never called), only implicit conversions happen.

This is due to how overload resolution works in HLSL. We should actually just disallow writing cast operators since we never resolve them (#6860).

Yes in and out aren't supposed to be references

Just to note: in is the same as no annotation. inout and out are copied temporaries.

however I'd appreciate not letting such hidden conversions happen in a language thats supposed to resemble C or C++ (with hopefully some strong typing) and implement on the Clang stack.

HLSL's conversions aren't all that different from what C & C++ allow for by-value parameter initialization. C's usual arithmetic conversions are even more extensive and cause all sorts of fun. My point being that HLSL actually aligns a lot more with C & C++ on implicit conversions than you may think. The only real oddity of HLSL is that we have a by-value copy to initialize a parameter from an argument and a by-value copy to write back the result of a modified parameter to the argument lvalue. The initialization and copy sequences follow C/C++ rules pretty closely.

The very least I'd want is a warning whenever such a conversion happens behind my back.

In Clang we will do a better job representing these cast sequences and warning on the implicit conversions, but these are fundamental to the language. The PR for the Clang implementation, already has full support for warning on all implicit conversions for inout and out argument expressions. Unfortunately back-porting that into DXC is non-trivial so it isn't going to happen. I had a draft attempting to bring this back to DXC, but it is way too breaking and cascades to being an unmanageable change due to all the associated things that need fixing (#5249).

Overload resolution is a different story. DXC's implementation of HLSL overload resolution is extremely limited and deeply flawed. We're not going to fix that in DXC mostly due to resourcing constraints, but Clang's overload resolution is more comprehensive and can handle cases that DXC's can't (like conversion operators).

We do have a language proposal to support non-member operator overloading (0008-Non-member Operator Overloading). Supporting that completely requires extending the contexts in which overloads are resolved. I've already incorporated language to support that case (and conversion operators) into the language specification draft:

Overload resolution selects the function to call in the following contexts:

  • invocation of a function named in a function call expression;
  • invocation of a function call operator on a class object named in function call syntax;
  • invocation of the operator referenced in an expression;
  • invocation of a user-defined conversion for copy-initialization of a class object;
  • invocation of a conversion function for initialization of an object of a non-class type from an expression of class
    type.

Source: [Overload.Res]

DXC supports overload resolution in the first two contexts and only partially in the third context (only when the operator is a member of the first argument's type).

-fno-implicit-argument-conversions which disables conversions happenning for in, out and inout function arguments.

There is no good way in DXC to disable implicit conversions, and doing so would almost certainly break a lot of things. The language rules for initializing argument expressions follows standard copy-initialization rules, and special casing that under a compiler option would likely cause all sorts of language constructs to just not work.

Worth noting, without copy-initialization of argument expressions, you wouldn't be able to call any function with an inout or out argument that is a groupshared object, or the result returned from a UAV subscript operator. The by-value passing is required for address space normalization for parameters, and we need to be able to correctly initialize those expressions.

@llvm-beanz llvm-beanz closed this as not planned Won't fix, can't repro, duplicate, stale Aug 14, 2024
@github-project-automation github-project-automation bot moved this to Triaged in HLSL Triage Aug 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature suggestion needs-triage Awaiting triage
Projects
Archived in project
Development

No branches or pull requests

2 participants