opalj / opal

https://www.opal-project.de
Other
51 stars 27 forks source link

Fix default interface methods #215

Closed errt closed 2 weeks ago

errt commented 2 months ago

Fixes two problems where methods were not computed correctly for some inheritance hierarchies w.r.t. interface default methods.

The additional checks are rather costly, but I didn't evaluate the actual performance impact. Feedback on the impact and suggestions for improving the performance of this code are welcome.

johannesduesing commented 2 months ago

@errt Can you provide the example for which this issue first surfaced during analysis? That would also be helpful for me designing a larger-scale look into the issue 😄

errt commented 2 months ago

The example for the change necessary in Project was class scala.collection.mutable.IndexedSeqView$AbstractTransformed, method public scala.collection.IterableFactory iterableFactory() (the class is found in OPAL/bi/src/test/resources/classfiles/scala-2.12.4/scala-library-2.12.4.jar. This is defined in two interfaces: scala.collection.mutable.Iterable (implemented via interfaces scala.collection.mutable.IndexedSeqView$Transformed -> scala.collection.mutable.IndexedSeqView -> scala.collection.mutable.IndexedSeq -> scala.collection.mutable.Seq) and scala.collection.View (implemented via superclass scala.collection.SeqViewLike$AbstractTransformed and its interfaces scala.collection.SeqViewLike$Transformed -> scala.collection.SeqView).

The example for the changes to DeclaredMethodsKey was class org.opalj.br.instructions.UNRESOLVED_INVOKEDYNAMIC, method public boolean isInstanceMethod() (the class is found in OPAL/bi/src/test/resources/classfiles/OPAL-MultiJar-SNAPSHOT-08-14-2014.jar). This is defined with implementation in trait org.opalj.br.instructions.INVOKEDYNAMIC (which the above class extends directly) and as an abstract method in the class' direct supertype org.opalj.br.instructions.InvocationInstruction.

To reproduce the issues and debug the above information, I ran org.opalj.support.info.FieldAssignabilityRunner with none of the fixes applied with a breakpoint set in scala.Predef.assert. This produced the error for the first example. After applying the changes to the Project class, running the same produced the error for the second example.

johannesduesing commented 2 months ago

I'm having a few problems reproducing the first example for the changes in Project .. i'm not sure if i'm overlooking something very obvious here, maybe you can help me: I can't find scala.collection.View in scala-library-2.12.4, and also no mention of a method iterableFactory() (in Scala 2.12.4). I do however see both View, scala.collection.mutable.Iterable and their definitions of iterableFactory() in Scala 2.13.14, however in 2.13.14 there is no scala.collection.mutable.IndexedSeqView$AbstractTransformed that inherits both of them (which i did see in 2.12.4) - am i missing something here?

errt commented 2 months ago

True, the scala.collection.View and scala.collection.mutable.Iterable in this case came from Scala 3.5.0