Skip to content

Regression for inline if when condition is defined in trait #16641

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

Closed
WojciechMazur opened this issue Jan 9, 2023 · 3 comments · Fixed by #16652
Closed

Regression for inline if when condition is defined in trait #16641

WojciechMazur opened this issue Jan 9, 2023 · 3 comments · Fixed by #16652
Assignees
Labels
area:inline itype:bug regression This worked in a previous version but doesn't anymore
Milestone

Comments

@WojciechMazur
Copy link
Contributor

Based on Open CB failure for zio/izumi-reflect Open CB #37007

Bisect points to 342ebf3

Compiler version

3.3.0-RC1-bin-20230105-b65b0f2-NIGHTLY

Minimized code

trait Logger {
  inline def debug: debug = valueOf[debug]
  final type debug = false

  // fails
  inline final def log(inline s: String): Unit = {
    inline if (debug) println(" -> " + s)
  }
}

trait BaseLogger extends Logger {
  // fails
  def bar() = log("case1")
}

object Logger {
  // works
  inline def log(inline shift: Int, s: String): Unit = {
    inline if (valueOf[Logger#debug]) println(" " * shift + " -> " + s)
  }
}

@main def Test = {
  val x: BaseLogger = ???
  // fails
  x.log("case2")
  Logger.log(2, "bar")
}

Output

-- Error: /workspace/bisect/test.scala:13:17 -----------------------------------
13 |  def bar() = log("case1")
   |              ^^^^^^^^^^^^
   |Cannot reduce `inline if` because its condition is not a constant value: {
   |  val Logger_this: (Logger_this² : (BaseLogger.this : BaseLogger)) =
   |    Logger_this²
   |  {
   |    given val ev: ValueOf[(false : Boolean)] =
   |      new ValueOf[(false : Boolean)](false)
   |    false
   |  }:Logger_this.debug:Logger_this.debug
   |}
   |
   |where:    Logger_this  is a value in method bar
   |          Logger_this² is a value in method bar
   |----------------------------------------------------------------------------
   |Inline stack trace
   |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
   |This location contains code that was inlined from test.scala:7
 7 |    inline if (debug) println(" -> " + s)
   |    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    ----------------------------------------------------------------------------

Expectation

Should compile

@WojciechMazur WojciechMazur added itype:bug regression This worked in a previous version but doesn't anymore stat:needs triage Every issue needs to have an "area" and "itype" label labels Jan 9, 2023
@nicolasstucki nicolasstucki added area:inline and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels Jan 9, 2023
@dwijnand dwijnand self-assigned this Jan 9, 2023
@SethTisue SethTisue changed the title Regression for inline if when condidition is defined in trait Regression for inline if when condition is defined in trait Jan 10, 2023
@SethTisue
Copy link
Member

SethTisue commented Jan 10, 2023

The longer reproducer is also good to have, but here's one that's absolutely minimal afaict:

trait Logger:
  inline def debug: False = false
  type False = false
  inline def log(): Unit =
    inline if debug then ()

class Test:
  def test(l: Logger) = l.log()
[error] -- Error: /Users/tisue/tmp/20230110/S.scala:8:29 -------------------------------
[error] 8 |  def test(l: Logger) = l.log()
[error]   |                        ^^^^^^^
[error]   |Cannot reduce `inline if` because its condition is not a constant value: {
[error]   |  val Logger_this: (Logger_this² : (l : Logger)) = Logger_this²
[error]   |  false:Logger_this.False
[error]   |}
[error]   |
[error]   |where:    Logger_this  is a value in method test
[error]   |          Logger_this² is a value in method test
[error]   |-----------------------------------------------------------------------------
[error]   |Inline stack trace
[error]   |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
[error]   |This location contains code that was inlined from S.scala:5
[error] 5 |    inline if debug then ()
[error]   |    ^^^^^^^^^^^^^^^^^^^^^^^
[error]    -----------------------------------------------------------------------------
[error] one error found

@SethTisue SethTisue self-assigned this Jan 10, 2023
@SethTisue
Copy link
Member

SethTisue commented Jan 10, 2023

Dale noticed that the inliner is looking for a ConstantValue tree, but instead it sees the following block, which has been inserted as part of this handling:

{
  val Logger_this: (Logger_this² : (l : Logger)) = Logger_this²
  false:Logger_this.False
}

it has the right type but not the expected tree shape. Perhaps the inliner shouldn't be so picky about the tree shape? Or would that open the barn door too wide?

@SethTisue
Copy link
Member

SethTisue commented Jan 10, 2023

Dale found #10107 (because one of our experiments made it fail) which is relevant  — note the exchange between Jamie and Nico about whether looking inside true: Boolean is fair game or not, in the absence of transparent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:inline itype:bug regression This worked in a previous version but doesn't anymore
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants