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

Inlining in opaque type #14653

Closed
Swoorup opened this issue Mar 9, 2022 · 4 comments · Fixed by #15074
Closed

Inlining in opaque type #14653

Swoorup opened this issue Mar 9, 2022 · 4 comments · Fixed by #15074

Comments

@Swoorup
Copy link

Swoorup commented Mar 9, 2022

Compiler version

3.1.1

Minimized code

https://scastie.scala-lang.org/Baodp0IPRQWdkjwE2SgUYg

type Amount = Amount.Type
object Amount:
  opaque type Type = BigDecimal
  inline def apply(inline dec: BigDecimal): Type = dec

  extension (self: Type)
    inline def value: BigDecimal = self
    inline def +(y: Type): Type = self + y

@main def r(): Unit = 
  val aa: Amount = Amount(1)
  val ab: Amount = Amount(2)
  val ac: Amount = Amount(2)
  val as1: Amount = aa + ab
  val as2: Amount = aa + ab + ac

  println(s"aa + ab = ${as1}")
  println(s"aa + ab = ${as2}")

Output

undefined: aa.+ # -1: TermRef(TermRef(NoPrefix,val aa),+) at inlining
9
Found:    (ac : Playground.Amount)
Required: BigDecimal

Expectation

Either successfully compile or warn about recursive definition.
I am not sure the correct way to write

    inline def +(y: Type): Type = self + y

is instead

    inline def +(y: Type): Type = self.value + y.value
@Swoorup Swoorup added itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label labels Mar 9, 2022
@Swoorup Swoorup changed the title Odd compiler message using opaque type and inline recursive-like definition undefined: aa.+ # -1: TermRef(TermRef(NoPrefix,val aa),+) at inlining Mar 10, 2022
@prolativ prolativ changed the title undefined: aa.+ # -1: TermRef(TermRef(NoPrefix,val aa),+) at inlining Inlining in opaque type Mar 10, 2022
@prolativ prolativ added area:inline area:opaque-types and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels Mar 10, 2022
@prolativ
Copy link
Contributor

I guess this is somehow related to the ordering of compilation phases. Some phases related to inlining are before ElimOpaque and some are after it.

@nicolasstucki
Copy link
Contributor

nicolasstucki commented Apr 29, 2022

Minimized

type Amount = Amount.Type
object Amount:
  opaque type Type = Int
  inline def twice(x: Type): Type = x + x

def double(a: Amount): Amount.Type = Amount.twice(a)

Workaround

- def double(a: Amount): Amount.Type = Amount.twice(a)
+ def double(a: Amount.Type): Amount.Type = Amount.twice(a)

@nicolasstucki
Copy link
Contributor

It seems that by having the alias type we do not detect that we need to generate a refined proxy for x

    def double(a: Amount): Amount.Type = 
      {
        val $proxy1: Amount.type{Type = Int} = 
          Amount.$asInstanceOf[Amount.type{Type = Int}]
        val Amount$_this: ($proxy1 : Amount.type{Type = Int}) = $proxy1
        (a.+(a):Amount$_this.Type)
      }.$asInstanceOf[Amount.Type]
    def double(a: Amount.Type): Amount.Type = 
      {
        val $proxy1: Amount.type{Type = Int} = 
          Amount.$asInstanceOf[Amount.type{Type = Int}]
        val Amount$_this: ($proxy1 : Amount.type{Type = Int}) = $proxy1
        val x$proxy1: ((a : Amount.Type) & $proxy1.Type) = 
          a.$asInstanceOf[(a : Amount.Type) & $proxy1.Type]
        (x$proxy1.+(x$proxy1):Amount$_this.Type)
      }.$asInstanceOf[Amount.Type]

@nicolasstucki
Copy link
Contributor

def a: Amount = ???
def double: Amount.Type = Amount.twice(a)

is inlined with an unrefined proxy

    def a: Amount = ???
    def double: Amount.Type = 
      {
        val $proxy1: Amount.type{Type = Int} = 
          Amount.$asInstanceOf[Amount.type{Type = Int}]
        val Amount$_this: ($proxy1 : Amount.type{Type = Int}) = $proxy1
        val x$proxy1: Amount = a
        (x$proxy1.+(x$proxy1):Amount$_this.Type)
      }.$asInstanceOf[Amount.Type]

nicolasstucki added a commit to dotty-staging/dotty that referenced this issue Apr 29, 2022
bishabosha pushed a commit to dotty-staging/dotty that referenced this issue Oct 18, 2022
@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