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

Val is not stable unless path requested by use site #9276

Closed
bishabosha opened this issue Jul 3, 2020 · 7 comments · Fixed by #11858
Closed

Val is not stable unless path requested by use site #9276

bishabosha opened this issue Jul 3, 2020 · 7 comments · Fixed by #11858
Assignees
Labels
area:tasty-format issues relating to TASTy as a portable standard itype:bug
Milestone

Comments

@bishabosha
Copy link
Member

it seems that a val is not stable in TASTy unless a stable path is requested for it:

Minimized code

// before
object Consts {
  val msg = "Hello World" // not stable
}
object TestConsts {
  def foo = Consts.msg
}
// after
object Consts {
  val msg = "Hello World" // now it is stable because of a use site
}
object TestConsts {
  def foo: Consts.msg.type = Consts.msg
}

Output

before:

80:       VALDEF(7) 26 [msg]
83:         TYPEREF 27 [String]
85:           SHAREDtype 35
87:         STRINGconst 28 [Hello World]

after:

80:       VALDEF(8) 26 [msg]
83:         TYPEREF 27 [String]
85:           SHAREDtype 35
87:         STRINGconst 28 [Hello World]
89:         STABLE

Expectation

@smarter smarter removed their assignment Jul 3, 2020
@smarter
Copy link
Member

smarter commented Jul 3, 2020

@anatoliykmetyuk I don't have time to take on more issues currently.

@odersky
Copy link
Contributor

odersky commented Jan 3, 2021

STABLE can be either user-declared, or structural, in which case it is a hint only. Absence of STABLE when it is a hint is no problem, since it will be re-generated when tested. But maybe the declared and computed versions should be different flags.

@smarter smarter added this to the 3.0.0-RC1 milestone Jan 5, 2021
@smarter smarter added the area:tasty-format issues relating to TASTy as a portable standard label Jan 5, 2021
@anatoliykmetyuk anatoliykmetyuk modified the milestones: 3.0.0-RC1, 3.0.0-RC2 Feb 11, 2021
@odersky
Copy link
Contributor

odersky commented Mar 22, 2021

I had a look. It does not seem possible to fix this, unless we spend a flag, and we have very few left. So, what is the problem here exactly? StableRealizable is an assertion that a value is stable and relializable. It is sometimes an assertion, and at other times a cache, meaning that it simply avoids a recomputation of the property. Can't we leave that as it is?

@odersky odersky assigned bishabosha and unassigned odersky Mar 22, 2021
@smarter
Copy link
Member

smarter commented Mar 22, 2021

So, what is the problem here exactly?

One problem I can think of is it breaks reproducible builds: depending on whether two files are compiled together or one after another, the STABLE flag will be present in tasty or not. Quite a lot of work went into achieve deterministic compilation in Scala 2 to avoid this sort of things, in particular because it's important for build systems that rely heavily on caching compiler artifacts: scala/scala-dev#405

@smarter
Copy link
Member

smarter commented Mar 22, 2021

Could we explicitly compute if something is StableRealizable at Pickler to emit STABLE even if the cache hasn't been set yet?

@odersky
Copy link
Contributor

odersky commented Mar 22, 2021

Could we explicitly compute if something is StableRealizable at Pickler to emit STABLE even if the cache hasn't been set yet?

Not really, since the check is expensive and risks creating cycles. The check is performed only in specific cases right now, which are relatively rare (essentially when we have to ensure that something is a path).

@smarter
Copy link
Member

smarter commented Mar 22, 2021

I assume it can create cycles in Typer, but can it also create cycles in Pickler? The expensiveness might be an issue, though having it always stored in tasty means we wouldn't have to either recompute it on unpickled symbols, which would amortize it somewhat.

odersky added a commit to dotty-staging/dotty that referenced this issue Mar 23, 2021
Fixes scala#9276

That aligns behavior with the TastyFormat doc, where it says:

    STABLE  -- Method that is assumed to be stable, i.e. its applications are legal paths

For other occurrences of StableRealizable, we either reconstitute them if they are implied
(i.e. for module vals and enum values), or we recompute them in realizability checking as needed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:tasty-format issues relating to TASTy as a portable standard itype:bug
Projects
None yet
4 participants