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

-opt:l:inline breaks incremental compilation #4125

Closed
eparejatobes opened this issue Apr 26, 2018 · 8 comments · Fixed by sbt/zinc#1310 · May be fixed by scala/scala#10633
Closed

-opt:l:inline breaks incremental compilation #4125

eparejatobes opened this issue Apr 26, 2018 · 8 comments · Fixed by sbt/zinc#1310 · May be fixed by scala/scala#10633

Comments

@eparejatobes
Copy link

Given

@inline def x = 2

// somewhere else
def z = x + 1 

changes in x above after a successful compilation will not propagate to z; if we make def x = 3 then we will have z == 3, not 4.

I'm aware of the 2.12 Release Notes stating that

We recommend enabling inlining only in production builds, as sbt’s incremental compilation does not track dependencies introduced by inlining.

but IMHO mentioning it upfront doesn't make it less of a bug: either incremental compilation works correctly when inlining is enabled (which it should), or we are given a way of disabling it (as it was already asked for in #1282).

@dwijnand
Copy link
Member

Duplicate of sbt/zinc#227?

Alternatively could you provide, please, a minimal-but-complete reproduction? For instance sbt version, scala version, scala options, the test, etc. Following our ISSUE_TEMPLATE, ideally 😀.

@eparejatobes
Copy link
Author

Duplicate of sbt/zinc#227?

That one looks a particular side-effect of "does not track dependencies introduced by inlining" related with singleton types and final vals being always inlined by the type checker. This is something wider: you simply cannot trust the incremental compiler at all when using the 2.12 optimizer.

Alternatively could you provide, please, a minimal-but-complete reproduction? (...)

There you go, not just that but a dedicated repo:

@eparejatobes
Copy link
Author

Copying here the relevant portion of the link above; see ohnosequences/sbt4125 for more

steps

Open a shell and

git checkout "x=1"

Now run sbt on a different shell and do ~test. Once it runs for the first time,

git checkout "x=2"

Take a look at the test passing, and the definitions of x and y.

Problem

Incremental compilation when used together with the optimizer generates incorrect bytecode: modifications in methods/vals are not propagated to wherever they are being inlined.

Expectation

The incremental compiler should not ever generate incorrect bytecode.

Version

sbt 1.1.0 most likely previous ones too.

@eed3si9n
Copy link
Member

eed3si9n commented May 2, 2018

@eparejatobes Thanks for the report and reproduction.

@eed3si9n
Copy link
Member

eed3si9n commented May 2, 2018

I wasn't able to follow the instruction exactly:

$ git checkout "x=1"
error: pathspec 'x=1' did not match any file(s) known to git.

but I got the gist of what's going on from manually changing:

  @inline
  def x: Int = 1

to

  @inline
  def x: Int = 2

@eparejatobes
Copy link
Author

I wasn't able to follow the instruction exactly: (...)

My fault, forgot to push the tags; it should work now.

but I got the gist of what's going on (...)

yes, that's it. I can understand that fixing this would involve considerable work in zinc; I think then that there should be a way (however hackish) of disabling incremental compilation. Note that (as this example shows) I could perfectly publish bytecode wildly different from what the source should generate.

@eed3si9n
Copy link
Member

eed3si9n commented May 2, 2018

It can't be too difficult to invalidate everything whenever the flag is present, and show a warning saying "oh snap. you have inline flag on. we can't handle this."

@eed3si9n eed3si9n changed the title Inlining breaks compilation -opt:l:inline breaks incremental compilation May 2, 2018
@eed3si9n
Copy link
Member

eed3si9n commented May 2, 2018

I've reported also to sbt/zinc#537

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants