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

LinearSeq#head and LinearSeq#tail aren't abstract #11697

Closed
argv-minus-one opened this issue Aug 20, 2019 · 6 comments · Fixed by scala/scala#10106
Closed

LinearSeq#head and LinearSeq#tail aren't abstract #11697

argv-minus-one opened this issue Aug 20, 2019 · 6 comments · Fixed by scala/scala#10106

Comments

@argv-minus-one
Copy link

The LinearSeq#isEmpty, LinearSeq#head, and LinearSeq#tail methods should be abstract, so that it is obvious (and verified at compile time) that these methods must be implemented in order to have a working LinearSeq. Currently, they are not, and new LinearSeq[SomeType] {} is permitted at compile time but causes a stack overflow at run time.

Also, the LinearSeq documentation no longer says that these methods must be implemented. In 2.12 and earlier, this was in the documentation for LinearSeqOptimized, but it doesn't seem to have been brought over.

@dwijnand
Copy link
Member

dwijnand commented Dec 4, 2019

Also, the LinearSeq documentation no longer says that these methods must be implemented. In 2.12 and earlier, this was in the documentation for LinearSeqOptimized, but it doesn't seem to have been brought over.

This part can be fast-tracked, without issues of binary compatibility.

@SethTisue
Copy link
Member

volunteer to tackle this for 2.13.2?

@SethTisue SethTisue modified the milestones: 2.13.2, 2.13.3 Feb 6, 2020
@SethTisue SethTisue modified the milestones: 2.13.3, 2.13.4 May 12, 2020
@dwijnand dwijnand modified the milestones: 2.13.4, Backlog Oct 20, 2020
@guizmaii
Copy link

guizmaii commented Aug 9, 2022

Is this related to the fact that the optimizer can't inline such code:

implicit final class RichList[A](private val list: List[A]) extends AnyVal {
  def sumBy[B: Numeric](f: A => B): B = list.foldLeft(Numeric[B].zero) { case (acc, a) => Numeric[B].plus(acc, f(a)) }
}

If I compile with the optimizer enabled and the -opt-warnings:any-inline-failed flag, I have the following error:

[error] Failed to check if scala/collection/LinearSeqOps::foldLeft(Ljava/lang/Object;Lscala/Function2;)Ljava/lang/Object; can be safely inlined to LanguageExtensions$RichList$ without causing an IllegalAccessError. Checking instruction INVOKEINTERFACE scala/collection/LinearSeq.isEmpty ()Z (itf) failed:
[error] The method isEmpty()Z could not be found in the class scala/collection/LinearSeq or any of its parents.
[error]     def sumBy[B: Numeric](f: A => B): B = list.foldLeft(Numeric[B].zero) { case (acc, a) => Numeric[B].plus(acc, f(a)) }
[error]                                                                          ^
[error] LanguageExtensions.scala:114:93: scala/collection/LinearSeqOps::foldLeft(Ljava/lang/Object;Lscala/Function2;)Ljava/lang/Object; could not be inlined:

??? 🤔

@som-snytt
Copy link

@guizmaii it won't inline methods that can be overridden. Here, I think it's looking for candidates to inline and notices that there is an abstract isEmpty in a subinterface (LinearSeqOps <: IterableOnceOps or whatever).

It's just looking at bytecode, so it doesn't have enough information about foldLeft to know that isEmpty is called on List there.

@SethTisue
Copy link
Member

Not sure whether to leave this open — the linked PR initially aimed to change the code, but ended up changing the documentation only.

@som-snytt
Copy link

I created an issue on library-next for later revisitation.

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.

6 participants