Skip to content
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

Allow implicit casting from T** to void** and higher void pointer arities #3453

Open
ThadHouse opened this issue May 13, 2020 · 14 comments
Open

Comments

@ThadHouse
Copy link

ThadHouse commented May 13, 2020

Some native API's need to be able to take a void** or a void*** Currently this requires an explicit cast to cast a T** to a void**. However, to pointers of the same arity, implicit conversions would be allowed, and actually would likely make some bugs harder.

Currently, if I have a T* val, and want to pass a pointer to it to an API taking void**, the explicit cast would have to be (void**)&val. However, (void**)val would compile as well, being a bug, which I've actually seen happen before. If an implicit conversion was allowed, passing val to a function taking void** would fail to compile, since the pointer arities are different, and the compiler could even suggest taking &val.

Note the important part of this is the pointer arities must match. Implicit casting of a T** to a void*** shouldn't be allowed.

This wouldn't cause any breaking code, as the implicit cast was disallowed before, and it wouldn't change overload resolution to any of the existing casts.

@ThadHouse ThadHouse changed the title Allow implicit casting from T* to void* and matching pointer arities Allow implicit casting from T** to void** and higher void pointer arities May 13, 2020
@john-h-k
Copy link

+1. It's incredibly frustrating and a source of bugs when this happens. Implicit conversions here are both logical and will lower the number of bugs present

@333fred
Copy link
Member

333fred commented May 13, 2020

and it wouldn't change overload resolution to any of the existing casts.

public unsafe void M(void* ptr) {}
public unsafe void M(void** ptr) {}
public unsafe void MyOtherFunc(T** ptr)
{
    M(ptr);
}

This would now be ambiguous.

@john-h-k
Copy link

@333fred i woul;d just tiebreak to the first for backcompat personally
void* implicit more precedent than void{*...} implicit
void{*...} implicit equally precedent to any other void{*...} implicit

@HaloFour
Copy link
Contributor

Sounds like a good use case for an analyzer. Allowing the compiler to convert between types of pointers implicitly sounds like a good way to introduce even more subtle bugs.

@john-h-k
Copy link

@HaloFour I'd argue not really. It already occurs, and between the same arity of pointer it's logical and only erases type

@HaloFour
Copy link
Contributor

Sounds like a good reason to have more guardrails, not fewer. An analyzer would be able to detect an explicit cast between pointers of different arities and to warn or error appropriately, regardless of whether or not it was legal in the language. That would prevent the issue without creating the potential for more issues due to laxing the rules around pointer conversion.

@john-h-k
Copy link

Casting between arities can be a desired behaviour, which is why it should be allowed but explicit. Casting at the same arity from T to void is always a safe, logical, and legal defined operation so can be implicit

@PathogenDavid
Copy link

I agree with HaloFour, if this is a common issue in your codebase you're better off writing an analyzer for (void**)val scenarios.

i woul;d just tiebreak to the first for backcompat personally

If you get used to expecting pointers to implicitly to void pointers of the same level of indirection, this pit of failure just becomes easier to fall into.

Casting between arities can be a desired behaviour, which is why it should be allowed but explicit.

Writing an analyzer doesn't mean it's suddenly forbidden. You can suppress the analyzer in the rare case you need to.

@john-h-k
Copy link

You can suppress the analyzer in the rare case you need to.
I wouldn't describe it as that rare

Sounds like a good reason to have more guardrails
Why does this perfectly legal, safe, and logical operation need more guards?

@PathogenDavid
Copy link

I wouldn't describe it as that rare

What code base are you working in where it is common to stuff a a pointer into storage intended for a double pointer?

@ThadHouse
Copy link
Author

Tons of C APIs use a void** to return an output pointer. In fact its extremely common, since its the standard COM uses. And its really not that hard to miss an &, and because the explicit cast is required, theres no hint you forgot it until it breaks at runtime.

@PathogenDavid
Copy link

That's not what I'm talking about, @john-h-k is arguing that using an analyzer to detect a missed & isn't acceptable because it makes it more cumbersome to do it intentionally.

I agree that the original problem you presented exists. I just think an analyzer is a better solution over a language change.

@vladd
Copy link

vladd commented May 21, 2020

Wouldn't it be enough just to prohibit the cast to the higher pointer arity? E. g. cast from T* to void** shouldn't be correct, as T is not an array of somethings.

@333fred
Copy link
Member

333fred commented May 21, 2020

Wouldn't it be enough just to prohibit the cast to the higher pointer arity? E. g. cast from T* to void** shouldn't be correct, as T is not an array of somethings.

I would love to, but that's a breaking change best suited to an analyzer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants