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

Cannot find method IndexedSeqView.updated when running a program compiled with CC stdlib #19819

Closed
Linyxus opened this issue Feb 28, 2024 · 8 comments · Fixed by #19873
Closed
Assignees
Labels
area:experimental:cc Capture checking related itype:bug
Milestone

Comments

@Linyxus
Copy link
Contributor

Linyxus commented Feb 28, 2024

Compiler version

main

Minimized code

object Hello extends App {
    println(Vector(1, 2, 3).view.updated(1, 8))
}

Steps to reproduce (assuming that the code is saved as Hello.scala):

sbt> set ThisBuild/Build.scala2Library := Build.Scala2LibraryCCTasty
sbt> scala3-bootstrapped/scalac Hello.scala
sbt> scala3-bootstrapped/scala Hello

Output

Exception in thread "main" java.lang.NoSuchMethodError: 'scala.collection.View scala.collection.IndexedSeqView.updated(int, java.lang.Object)'
	at Hello$.<clinit>(Hello.scala:2)
	at Hello.main(Hello.scala)

Expectation

It should run.

@Linyxus Linyxus added itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label area:experimental:cc Capture checking related and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels Feb 28, 2024
@odersky
Copy link
Contributor

odersky commented Feb 28, 2024

I think it's because at runtime we use the normal stdlib, where there is no updated in IndexedSeqView. Can you try to make the updated that we added an inline method?

@nicolasstucki nicolasstucki self-assigned this Feb 29, 2024
@nicolasstucki
Copy link
Contributor

This is the difference in the bytecode

  44: invokestatic  #54                 // Method scala/runtime/BoxesRunTime.boxToInteger:(I)Ljava/lang/Integer;
- 47: invokeinterface #60,  3           // InterfaceMethod scala/collection/IndexedSeqView.updated:(ILjava/lang/Object;)Ljava/lang/Object;
+ 47: invokeinterface #60,  3           // InterfaceMethod scala/collection/IndexedSeqView.updated:(ILjava/lang/Object;)Lscala/collection/View;
  52: invokevirtual #64                 // Method scala/Predef$.println:(Ljava/lang/Object;)V

nicolasstucki added a commit to dotty-staging/dotty that referenced this issue Feb 29, 2024
This added a an override for the unapply method to fix the signature,
but this method does not exist in the original library. We inline all
calls to it to avoid any runtime calls to this new override.

Fixes scala#19819
@odersky
Copy link
Contributor

odersky commented Feb 29, 2024

Which is which?

@nicolasstucki
Copy link
Contributor

The one that returns a View is using the CC library. That signature corresponds to the addition new override added in https://github.com/lampepfl/dotty/pull/19798/files#diff-1557066ce6e57518a7fa5d6138de6a2a431ecc6baf30fcf1e423c73cf99b340eR51

Inlining it seems to work (#19824). It is not an ideal solution as it will duplicate the implementation of updated at each call site.

@odersky
Copy link
Contributor

odersky commented Feb 29, 2024

Or maybe we can massage the return type by going through some abstract type. E.g.

type C[T] <: T
def updated(...): C[View]

Would that work?

@Linyxus
Copy link
Contributor Author

Linyxus commented Feb 29, 2024

Or maybe we can massage the return type by going through some abstract type. E.g.

type C[T] <: T
def updated(...): C[View]

Would that work?

We have to say type C[T] >: T <: T to make it typecheck. I tried and it didn't work.

I attempted another fix here. Do you think that is a good idea?

@odersky
Copy link
Contributor

odersky commented Feb 29, 2024

Not ure it will work, but it's worth a try!

@Linyxus
Copy link
Contributor Author

Linyxus commented Feb 29, 2024

I confirm that it works. So maybe we could use this scheme for other methods that need to be added as well?

nicolasstucki added a commit to dotty-staging/dotty that referenced this issue Mar 6, 2024
This added a an override for the unapply method to fix the signature,
but this method does not exist in the original library. We inline all
calls to it to avoid any runtime calls to this new override.

Fixes scala#19819
Linyxus added a commit that referenced this issue Mar 6, 2024
@Kordyjan Kordyjan added this to the 3.4.2 milestone Mar 28, 2024
@Kordyjan Kordyjan modified the milestones: 3.4.2, 3.5.0 May 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:experimental:cc Capture checking related itype:bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants