Bug 1735970 Comment 30 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

(In reply to James Teh [:Jamie] from comment #28)
> Maybe a thunk gets used for covariant return types? AccIterable::Next returns an `Accessible*`, but RelatedAccIterator::Next returns a `LocalAccessible*`. That is legal and deliberate, but perhaps why the compiler has to do strange things here.

Yes, you are correct.

It requires not just a covariant return type, but also that the return type is a non-primary base type (the primary base type is the one that shares the pointer value with the derived type).

Specifically, in this new case `LocalAccessible` inherits from both `nsISupports` and `Accessible`. `RelatedAccIterator::Next` returns a pointer to a `LocalAccessible`, which must be cast to `Accessible*` if called through a base class pointer, and that cast needs the thunk to adjust the returned pointer's value.

> If that's the case, though, I wonder why LocalAccessible::EmbeddedChildAt didn't trigger this problem. It also uses a covariant return type and I landed that a while back.

That is a very good point! It does look like it's doing exactly the same thing.

I may try to track that down, but for now, it appears that all I need to do here is remove an assert. (Well, not exactly -- I also need to make it to the same thing as it does in the non-asserting code path. But it's still just removing code.)
(In reply to James Teh [:Jamie] from comment #28)
> Maybe a thunk gets used for covariant return types? AccIterable::Next returns an `Accessible*`, but RelatedAccIterator::Next returns a `LocalAccessible*`. That is legal and deliberate, but perhaps why the compiler has to do strange things here.

Yes, you are correct.

It requires not just a covariant return type, but also that the return type is a non-primary base type (the primary base type is the one that shares the pointer value with the derived type).

Specifically, in this new case `LocalAccessible` inherits from both `nsISupports` and `Accessible`. `RelatedAccIterator::Next` returns a pointer to a `LocalAccessible`, which must be cast to `Accessible*` if called through a base class pointer, and that cast needs the thunk to adjust the returned pointer's value.

> If that's the case, though, I wonder why LocalAccessible::EmbeddedChildAt didn't trigger this problem. It also uses a covariant return type and I landed that a while back.

That is a very good point! It does look like it's doing exactly the same thing.

I may try to track that down, but for now, it appears that all I need to do here is remove an assert. (Well, not exactly -- I also need to make it do the same thing as it does in the non-asserting code path. But it's still just removing code.)

Back to Bug 1735970 Comment 30