Skip to content

Conversation

@psteiger
Copy link

@psteiger psteiger commented Feb 14, 2024

Description:

When we have self types (implemented by recursive generics), Motif fails with StackOverflowError:

	at com.uber.xprocessing.ext.XTypeKt.hasCollectionType(XType.kt:316)
	at com.uber.xprocessing.ext.XTypeKt.hasCollectionType(XType.kt:316)
	at com.uber.xprocessing.ext.XTypeKt.hasCollectionType(XType.kt:316)
        ...

The following code seems to trigger that condition:

open class BaseRouter<out R : BaseRouter<R, *>, I : BaseInteractor<*, *, *>>(
    interactor: I,
) : Router<I>(interactor)

abstract class BaseInteractor<P : Any, I : BaseInteractor<*, I, *>, R : BaseRouter<R, I>>(
    presenter: P,
) : Interactor<P, R>(presenter) 
public class ConcreteInteractor extends 
    BaseInteractor<EmptyPresenter, ConcreteInteractor, ConcreteRouter>

public class ConcreteRouter extends
    BaseRouter<ConcreteRouter, ConcreteInteractor>
   ...

This PR changes XType.hasCollectionType() implementation to:

  1. Not re-process types that were already processed
  2. Use a queue instead of recursion (to avoid StackOverflowError for big enough type trees).

Test

Created a local build, built and ran one of our apps with the local build.

@davissuber
Copy link
Contributor

Code looks great; could we add a test?

@jbarr21 jbarr21 self-requested a review February 15, 2024 00:42
@davissuber
Copy link
Contributor

davissuber commented Feb 15, 2024

Some extra investigation notes regarding the room-compiler-processing library:

  • the version are on 2.6.0-alpha02 is closer to the latest version 2.6.1 than I originally thought
  • there aren't that much difference in the XType neighborhood between the two versions (based on 5-min of my untrained eyeballing), so bumping the library alone may not solve the stack overflow problem
  • however, based on this comment, it seems that hasCollectionType is introduced in part to handle some KSP type equality checks, which even the current version of room-compiler-processing has some special override
    • in other words, maybe we can circumvent the stack overflow problem by changing how we use the room-compiler-processing library (hand-wavy comments made without much context, and possibly a rather involved task)

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

Successfully merging this pull request may close these issues.

4 participants