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

Figure out whether we are mis-using instancetype in the Darwin APIs #23384

Closed
bzbarsky-apple opened this issue Oct 28, 2022 · 5 comments · Fixed by #25169
Closed

Figure out whether we are mis-using instancetype in the Darwin APIs #23384

bzbarsky-apple opened this issue Oct 28, 2022 · 5 comments · Fixed by #25169

Comments

@bzbarsky-apple
Copy link
Contributor

See #23364 (comment) -- what does instancetype promise, exactly? When is it appropriate to use it?

@jtung-apple @ksperling-apple

@bzbarsky-apple
Copy link
Contributor Author

Also see #23364 (comment)

@jtung-apple
Copy link
Contributor

Quoting @ksperling-apple - first comment:

Not changed in this PR, but it looks to me like the return type should be MTRDevice * instead of instancetype? If you were to call this method on a sub-class of MTRDevice, would you be guaranteed to get an instance of that sub-class?

A subclass of MTRDevice would return an object of the subclass in "reasonable code", as in, using the class names explicitly.

If you play with a Class variable in a way that limits the compiler's ability to infer the actual class, then you may get an id which is assignable to anything (the runtime type will still be correct, if you ask the object for its class). But doing this is not good practice in ordinary code.

@bzbarsky-apple
Copy link
Contributor Author

I think the issue is this: for a factory method as here, what happens? It can do one of two things. The impl could [[MTRDevice alloc] init] as I think we do, or it could [[self.class alloc] init], right? And it seems like we should return instancetype in the latter case and MTRDevice * in the former?

@jtung-apple
Copy link
Contributor

Okay you're right about this. Given the implementation of the factory method asks MTRDeviceController for a MTRDevice object, the return type really can't be anything but MTRDevice *. And subclassing MTRDevice really doesn't make sense because of that.

@jtung-apple
Copy link
Contributor

We should change the factory method to return MTRDevice *.

bzbarsky-apple added a commit to bzbarsky-apple/connectedhomeip that referenced this issue Feb 17, 2023
We should not be claiming to return instancetype if we actually always return an
instance of our class.

Fixes project-chip#23384
bzbarsky-apple added a commit to bzbarsky-apple/connectedhomeip that referenced this issue Feb 17, 2023
We should not be claiming to return instancetype if we actually always return an
instance of our class.

Fixes project-chip#23384
bzbarsky-apple added a commit that referenced this issue Feb 17, 2023
…5169)

We should not be claiming to return instancetype if we actually always return an
instance of our class.

Fixes #23384
kkasperczyk-no pushed a commit to kkasperczyk-no/sdk-connectedhomeip that referenced this issue Mar 15, 2023
…5169)

We should not be claiming to return instancetype if we actually always return an
instance of our class.

Fixes project-chip/connectedhomeip#23384
kkasperczyk-no pushed a commit to kkasperczyk-no/sdk-connectedhomeip that referenced this issue Mar 15, 2023
…5169)

We should not be claiming to return instancetype if we actually always return an
instance of our class.

Fixes project-chip/connectedhomeip#23384
lecndav pushed a commit to lecndav/connectedhomeip that referenced this issue Mar 22, 2023
…oject-chip#25169)

We should not be claiming to return instancetype if we actually always return an
instance of our class.

Fixes project-chip#23384
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants