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

Introducing: the fields phase [ci: last-only] #5141

Merged
merged 17 commits into from Aug 11, 2016
Commits on Aug 10, 2016
  1. Do not add `@TraitSetter` -- not sure what it's for

    adriaanm committed Jun 3, 2016
    Also deprecate the TraitSetter annotation.
Commits on Aug 11, 2016
  1. Fields phase

    adriaanm committed Apr 29, 2016
    One step towards teasing apart the mixin phase, making
    each phase that adds members to traits responsible for
    mixing in those members into subclasses of said traits.
    
    Another design tenet is to not emit symbols or trees
    only to later remove them. Therefore, we model a
    val in a trait as its accessor. The underlying field
    is an implementation detail. It must be mixed into
    subclasses, but has no business in a trait (an interface).
    
    Also trying to reduce tree creation by changing less in subtrees
    during tree transforms.
    
    A lot of nice fixes fall out from this rework:
    
      - Correct bridges and more precise generic signatures for
        mixed in accessors, since they are now created before erasure.
    
      - Correct enclosing method attribute for classes nested in trait fields.
    
        Trait fields are now created as MethodSymbol (no longer TermSymbol).
        This symbol shows up in the `originalOwner` chain of a class declared
        within the field initializer. This promoted the field getter to
        being the enclosing method of the nested class, which it is not
        (the EnclosingMethod attribute is a source-level property).
    
      - Signature inference is now more similar between vals and defs
    
      - No more field for constant-typed vals, or mixed in accessors
        for subclasses. A constant val can be fully implemented in a trait.
    
    TODO:
      - give same treatment to trait lazy vals (only accessors, no fields)
      - remove support for presuper vals in traits
        (they don't have the right init semantics in traits anyway)
      - lambdalift should emit accessors for captured vals in traits,
        not a field
    
    Assorted notes from the full git history before squashing below.
    
    Unit-typed vals: don't suppress field
    
    it affects the memory model -- even a write of unit to a field is relevant...
    unit-typed lazy vals should never receive a field
    this need was unmasked by test/files/run/t7843-jsr223-service.scala,
    which no longer printed the output expected from the `0 to 10 foreach`
    
    Use getter.referenced to track traitsetter
    
    reify's toolbox compiler changes the name of the trait
    that owns the accessor between fields and constructors (`$` suffix),
    so that the trait setter cannot be found when doing mkAssign in constructors
    this could be solved by creating the mkAssign tree immediately during fields
    
    anyway, first experiment: use `referenced` now that fields runs closer
    to the constructors phase (I tried this before and something broke)
    
    Infer result type for `val`s, like we do for `def`s
    
    The lack of result type inference caused pos/t6780 to fail
    in the new field encoding for traits, as there is no separate accessor,
    and method synthesis computes the type signature based on the ValDef tree.
    This caused a cyclic error in implicit search, because now the
    implicit val's result type was not inferred from the super member,
    and inferring it from the RHS would cause implicit search to consider
    the member in question, so that a cycle is detected and type checking fails...
    
    Regardless of the new encoding, we should consistently infer result types
    for `def`s and `val`s.
    
    Removed test/files/run/t4287inferredMethodTypes.scala and test/files/presentation/t4287c,
    since they were relying on inferring argument types from "overridden" constructors
    in a test for range positions of default arguments. Constructors don't override,
    so that was a mis-feature of -Yinfer-argument-types.
    
    Had to slightly refactor test/files/presentation/doc, as it was relying
    on scalac inferring a big intersection type to approximate the anonymous
    class that's instantiated for `override lazy val analyzer`.
    Now that we infer `Global` as the expected type based on the overridden val,
    we make `getComment` private in navigating between good old Skylla and Charybdis.
    I'm not sure why we need this restriction for anonymous classes though;
    only structural calls are restricted in the way that we're trying to avoid.
    
    The old behavior is maintained nder -Xsource:2.11.
    
    Tests:
      - test/files/{pos,neg}/val_infer.scala
      - test/files/neg/val_sig_infer_match.scala
      - test/files/neg/val_sig_infer_struct.scala
    
    need NMT when inferring sig for accessor
    Q: why are we calling valDefSig and not methodSig?
    A: traits use defs for vals, but still use valDefSig...
    keep accessor and field info in synch
  2. Fields phase synthesizes modules

    adriaanm committed May 27, 2016
    For now, keep the info transform in refchecks.
    Ultimately, refchecks should only check, not transform trees/infos.
    
    Fixes scala/scala-dev#126:
    the accessor for a module in a trait is correctly marked non-final
    (it's deferred).
  3. Test EnclosingMethod attribute for classes in lazy vals

    lrytz authored and adriaanm committed May 30, 2016
    Local and anonymous classes need to have an EnclosingMethod attribute
    denoting the enclosing class and method. In fact, the enclosing class
    must always be defined for local and anonymous classes, but the
    enclosing method may be null (for local / anonymous classes defined
    in field initializers or local blocks within a class body).
    
    The new test here ensures that classes declared within a lazy val
    initializer block indeed have the enclosing method set to null.
  4. Reduce flag fiddling

    adriaanm committed May 5, 2016
    There isn't much point to the late* flags in a world where
    we're mutating flags left and right in tree and info transformers...
    
    So, lets get rid of the indirection until we can include flags
    in a symbol's type history, like we do for its info.
    
    This retires lateDEFERRED (redundant with SYNTHESIZE_IMPL_IN_SUBCLASS).
    Since it's introduced so late, it makes little sense to have these
    synthetic members go back to DEFERRED. Instead, just set DEFERRED directly.
    
    Also remove unused late* and not* flags.
    notPRIVATE subsumes lateFINAL for effective finality (scala/scala-dev#126)
  5. Uncurry's info transform: non-static module --> method

    adriaanm committed May 27, 2016
    We do this during uncurry so we can insert the necessary
    applications to the empty argument list. Fields is too late.
    
    Refchecks is no longer an info transform.
  6. Don't cache `MethodSymbol`'s `memberType`.

    adriaanm committed May 27, 2016
    Correct caching is impossible because `sym.tpeHK.asSeenFrom(pre, sym.owner)`
    may have different results even for reference-identical `sym.tpeHK` and `pre`
    (even in the same period). For example, `pre` could be a `ThisType`.
    For such a type, `tpThen eq tpNow` does not imply `tpThen` and `tpNow` mean
    the same thing, because `tpThen.typeSymbol.info` could have been different
    from what it is now, and the cache won't know simply by looking at `pre`.
    
    Somehow this distinction never caused trouble, but when starting to desugar
    module definitions during the fields phase, it causes several test failures.
    I tried keying the cache on the current period to no avail.
  7. Address lrytz's review feedback

    adriaanm committed Jun 3, 2016
    Remove obsolete hack for BeanSetter's RHS
    
    Use currentOwner.isClass instead of exprOwner.isLocalDummy
    
    Refactor: shortest branches first in if/else
    
    Fix comments from when the prototype ran before refchecks
    
    Also, store `isScala212` as a `val` in `Namer`
    since the `def` on `settings` parses the version each time...
  8. Simplify erasure + mixin

    adriaanm committed May 30, 2016
    Remove some old, obsolete & untested hacks from ExplicitOuter.
    Added a test for one of them to show this is now fine.
    
    There are a lot of `makeNotPrivate` invocations sprinkled around
    the codebase. Lets see if we can centralize the ones dealing
    with trait methods that need implementations in the phase that emits them.
    
    For example Fields (accessors for fields/modules) or SuperAccessors.
  9. LambdaLift emits paramaccessor syms and defdefs

    adriaanm committed May 31, 2016
    ... instead of emitting ValDefs and field symbols, which are then
    promptly unlinked and transformed by the "late trait methods"
    logic in mixins...
    
    Mixins still synthesizes implementations for these accessors
    in subclasses.
    
    A paramaccessor in a trait is a method without an underlying field.
  10. Allow 'overriding' deferred var

    adriaanm committed Jul 6, 2016
    Discovered by scala-js's test suite.
  11. Align double definition check with spec

    adriaanm committed Jul 7, 2016
    Remove weird special cases for private-local fields
    and parameter accessor (fields).
    
    One change with the new trait val encoding:
    
    ```
    scala>  trait T { private[this] var x: String = "1" ; def x(): Int = 1 }
    <console>:11: error: method x is defined twice;
      the conflicting variable x was defined at line 11:37
            trait T { private[this] var x: String = "1" ; def x(): Int = 1 }
                                                              ^
    ```
    
    Whereas:
    
    ```
    scala>  class T { private[this] var x: String = "1" ; def x(): Int = 1 }
    defined class T
    ```
    
    Before, both the `class` and `trait` definition were accepted.
    (Because there is no accessor for a private[this] val/var,
    and a MethodType does not match the type of a value.)
    
    (Dotty accepts neither the class or the trait definition.)
  12. Drive accessor synthesis from info transformer

    adriaanm committed Jul 9, 2016
    Derive/filter/propagate annotations in info transformer,
    don't rely on having type checked the derived trees in order
    to see the annotations.
    
    Use synthetics mechanism for bean accessors -- the others
    will soon follow.
    
    Propagate inferred tpt from valdef to accessors
    by setting type in right spot of synthetic tree
    during the info completer.
    
    No need to add trees in derivedTrees, and get rid of
    some overfactoring in method synthesis, now that we have
    joined symbol and tree creation.
    
    Preserve symbol order because tests are sensitive to it.
    
    Drop warning on potentially discarded annotations,
    I don't think this warrants a warning.
    
    Motivated by breaking the scala-js compiler, which relied
    on annotations appearing when trees are type checked.
    Now that ordering constraint is gone in the new encoding,
    we may as well finally fix annotation assignment.
  13. Admit @volatile on accessor in trait

    adriaanm committed Jul 18, 2016
    There's no other place to squirrel away the annotation
    until we create a field in a subclass.
    
    The test documents the idea, but does not capture the
    regression seen in the wild, as explained in a comment.
  14. Mixed in getter needs NullaryMethodType too

    adriaanm committed Jul 20, 2016
    Clone at uncurry to preserve it in its info history.
    Discovered by the scala-js test suite.
  15. Make fewer trait methods not-{private, protected}

    adriaanm committed Jun 4, 2016
    No longer making trait methods not-protected.
    (The backend only does public/private because of the poor
    mapping between visibility from Scala to the JVM).
    
    Note that protected trait members will not receive static forwarders
    in module classes (when mixed into objects).
    
    Historic note: we used to `makeNotPrivate` during explicitouter,
    now we do it later, which means more private methods must be excluded
    (e.g., lambdaLIFTED ones).
  16. Review feedback from Jason & Lukas

    adriaanm committed Aug 11, 2016