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

experiment with removing transparent inline from op definitions #264

Closed
wants to merge 1 commit into from

Conversation

erikerlandson
Copy link
Owner

An experiment in removing transparent inline on operators.

This branch removes transparent inline from the pow method.

@erikerlandson
Copy link
Owner Author

The results are interesting. The operator itself, in isolation, works, but it does lose important type information.

Welcome to Scala 3.1.1 (11.0.14, Java OpenJDK 64-Bit Server VM).
Type in expressions for evaluation. Or try :help.
                                                                                                                                            
scala> import coulomb.*, algebra.instances.all.given, coulomb.ops.standard.given, coulomb.policy.standard.given
                                                                                                                                            
scala> import coulomb.units.us.{*,given}, coulomb.units.mks.{*,given}, coulomb.units.accepted.{*,given}
                                                                                                                                            
scala> 2d.withUnit[Meter].pow[3]
val res0: Any = 8.0
                                                                                                                                            
scala> res0.asInstanceOf[Quantity[Double, Meter ^ 3]]
val res1: coulomb.Quantity[Double, coulomb.units.us.Meter ^ 3] = 8.0
                                                                                                                                                                                                                                                                                        
scala> def f(q: Quantity[Double, Meter ^ 3]): Double = q.value
def f(q: coulomb.Quantity[Double, coulomb.units.us.Meter ^ 3]): Double
                                                                                                                                            
scala> f(res0)
-- [E007] Type Mismatch Error: -------------------------------------------------
1 |f(res0)
  |  ^^^^
  |  Found:    (res0 : Any)
  |  Required: coulomb.Quantity[Double, coulomb.units.us.Meter ^ (3 : Int)]

longer explanation available when compiling with `-explain`
1 error found
                                                                                                                                            
scala> 

From the above we can see that applying the pow operator succeeds, but without transparent inline the type of the operator is Any and so the coulomb unit type system isn't propagated in a useful way in operation outputs.

This is also reflected in the failures of the unit tests.

@erikerlandson
Copy link
Owner Author

Overall this does not seem viable. It might be possible to recover using the old Aux trick but that feels like backsliding to Scala 2

Comment on lines -29 to 35
transparent inline given ctx_pow_FractionalPower[V, U, E](using
given ctx_pow_FractionalPower[V, U, E](using
alg: FractionalPower[V],
dbv: typeexpr.DoubleValue[E],
su: SimplifiedUnit[U ^ E]
): Pow[V, U, E] =
val e = typeexpr.double[E]
val e = dbv.value
new Pow[V, U, E]:
Copy link
Contributor

@armanbilge armanbilge Mar 31, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: instead of making this an anonymous class within the inline function, can we make it a named class outside that is then instantiated within the inline function? The problem with anonymous classes in inline functions is that means everywhere the function is inlined, you will get a completely new (anonymous) class. This causes code bloat, esp. on JS.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll play around with it 👍

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #265. If I'm understanding what you are thinking, it looks promising

@erikerlandson erikerlandson deleted the remove-transparent-inline branch April 9, 2022 00:34
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.

2 participants