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

Compilation never stops with an upper bounded type param using a union type with 3 types #14870

Closed
Lasering opened this issue Apr 6, 2022 · 5 comments · Fixed by #14880
Closed

Comments

@Lasering
Copy link

Lasering commented Apr 6, 2022

Compiler version

3.1.1

Minimized code

This code compiles (although it takes a little bit, aka more than usual for so little code)

import squants.energy.{Energy, KilowattHours}
import squants.time.{Minutes, Time}
import squants.thermal.{Temperature, Kelvin}
import squants.{Quantity, UnitOfMeasure, Dimensionless, Each}

case class Price[Q <: Quantity[Q] & (Energy | Time)](amount: Double, per: UnitOfMeasure[Q])
Price(5.5, KilowattHours)
Price(5.5, Minutes)
//Price(5.5, Kelvin) // This line correctly fails to compile

Changing Price to

case class Price[Q <: Quantity[Q] & (Energy | Time | Dimensionless)](amount: Double, per: UnitOfMeasure[Q])

Causes the compiler to never stop compiling.

Output

Scastie times out after 30 seconds.
Ran locally for 300 seconds then killed it.

Expectation

With a union type of 2 types it works, but with 3 types it fails. This suggests a explosion of combinations to big to handle in a reasonable time. There must be a limit to this explosion of combinations, however I was expecting it to be much higher: in the tens or even in the hundreds not 3.

@Lasering Lasering added itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label labels Apr 6, 2022
@odersky
Copy link
Contributor

odersky commented Apr 7, 2022

Hard to say what is at the root of this without a full analysis of squants. We need a minimization here to be able to act on this.

@odersky odersky added stat:needs minimization Needs a self contained minimization and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels Apr 7, 2022
@Lasering
Copy link
Author

Lasering commented Apr 7, 2022

Minimization:

abstract class Quantity[A <: Quantity[A]]
class Energy extends Quantity[Energy]
class Time extends Quantity[Time]
class Dimensionless extends Quantity[Dimensionless]

class Price[Q <: Quantity[Q] & (Energy | Time | Dimensionless)]

@odersky
Copy link
Contributor

odersky commented Apr 7, 2022

Thanks for the minimization! I looked at this for a while, but could not see a specific problem that we could fix. It seems to be a combinatorial explosion of recursive subtype tests. My advice would be to not use F-bounded types, and certainly not in conjunction with complicated &/| combinations.

Over time we should try to make F-bounded types deprecated. They really interact terribly with almost everything else in the language. We need to keep them for Java compat, but maybe we can restrict them to
just that.

@griggt griggt added area:f-bounds and removed stat:needs minimization Needs a self contained minimization labels Apr 7, 2022
@Lasering
Copy link
Author

Lasering commented Apr 7, 2022

Squants defines Quantity with a f-bound, and since I want to have a UnitOfMeasure I'm forced to also have an f-bound. Otherwise it would be much simpler to do:

class Price[Q <: (Energy | Time | Dimensionless)](amount: Double, per: UnitOfMeasure[Q])

Which in an ideal world would compile since Energy, Time, and Dimensionless are all subclasses of Quantity[Q] however the compiler says: Type argument Q does not conform to upper bound squants.Quantity[Q]

It would be interesting to see how to solve problems the f-bounds solve without using them.

odersky added a commit to dotty-staging/dotty that referenced this issue Apr 7, 2022
Don't recursively apply joins on the left hand side if it contains
an AndType, in order to prevent possible exponential explosions.

Fixes scala#14870
@odersky
Copy link
Contributor

odersky commented Apr 7, 2022

I could find the culprit that caused the exponential explosion, and it looks like this case can be fixed, after all.

michelou pushed a commit to michelou/dotty that referenced this issue Apr 25, 2022
Don't recursively apply joins on the left hand side if it contains
an AndType, in order to prevent possible exponential explosions.

Fixes scala#14870
@Kordyjan Kordyjan added this to the 3.2.0 milestone Aug 1, 2023
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.

4 participants