Skip to content

SI-3439 Fix use of implicit constructor params in super call#4043

Merged
retronym merged 2 commits intoscala:2.11.xfrom
retronym:ticket/3439-2
Nov 2, 2014
Merged

SI-3439 Fix use of implicit constructor params in super call#4043
retronym merged 2 commits intoscala:2.11.xfrom
retronym:ticket/3439-2

Conversation

@retronym
Copy link
Member

When typechecking the primary constructor body, the symbols of
constructor parameters of a class are owned by the class's owner.
This is done make scoping work; you shouldn't be able to refer to
class members in that position.

However, other parts of the compiler weren't so happy about
this arrangement. The enclosed test case shows that our
checks for invalid, top-level implicits was spuriously triggered,
and implicit search itself would fail.

Furthermore, we had to hack Run#compiles to special case
top-level early-initialized symbols. See SI-7264 / 86e6e92.

This commit:

  • introduces an intermediate local dummy term symbol which
    will act as the owner for constructor parameters and early
    initialized members
  • adds this to the Run#symSource map if it is top level
  • simplifies Run#compiles accordingly
  • tests this all in a top-level class, and one nested in
    another class.

Review by @lrytz.

Up for discussion: this might need to target -Xsource:2.12
as it could introduce a new candidate implicit in the super call.

When typechecking the primary constructor body, the symbols of
constructor parameters of a class are owned by the class's owner.
This is done make scoping work; you shouldn't be able to refer to
class members in that position.

However, other parts of the compiler weren't so happy about
this arrangement. The enclosed test case shows that our
checks for invalid, top-level implicits was spuriously triggered,
and implicit search itself would fail.

Furthermore, we had to hack `Run#compiles` to special case
top-level early-initialized symbols. See SI-7264 / 86e6e92.

This commit:

  - introduces an intermediate local dummy term symbol which
    will act as the owner for constructor parameters and early
    initialized members
  - adds this to the `Run#symSource` map if it is top level
  - simplifies `Run#compiles` accordingly
  - tests this all in a top-level class, and one nested in
    another class.
@lrytz lrytz modified the milestones: 2.11.4, 2.11.5 Oct 14, 2014
@adriaanm
Copy link
Contributor

PLS REBUILD/pr-scala@bfa7997

@scala-jenkins
Copy link

(kitty-note-to-self: ignore 60403289)
🐱 Roger! Rebuilding pr-scala for bfa7997. 🚨

@lrytz
Copy link
Member

lrytz commented Oct 28, 2014

LGTM!

retronym added a commit that referenced this pull request Nov 2, 2014
SI-3439 Fix use of implicit constructor params in super call
@retronym retronym merged commit 54c720e into scala:2.11.x Nov 2, 2014
retronym added a commit to retronym/scala that referenced this pull request Jan 15, 2015
Implicit search declines to force the info of candidate implicits
that either a) are defined beyond the position of the implicit search
site, or b) enclose the implicit search site.

The second criterion used to prevent consideration of `O` in
the super constructor call:

    implicit object O extends C( { implicitly[X] })

However, after scala#4043, the
block containing the implicit search is typechecked in a context
owned by a local dummy symbol rather than by `O`. (The dummy and
`O` share an owner.)

This led to `O` being considered as a candidate for this implicit
search. This search is undertaken during completion of the info of
`O`, which leads to it being excluded on account of the LOCKED flag.

Unfortunately, this also excludes it from use in implicit search
sites subsequent to `O`, as `ImplicitInfo` caches
`isCyclicOrErroneous`.

This commit adjusts the position of the local dummy to be identical
to that of the object. This serves to exclude `O` as a candidate
during the super call on account of criterion a).
retronym added a commit to retronym/scala that referenced this pull request Jan 29, 2015
Implicit search declines to force the info of candidate implicits
that either a) are defined beyond the position of the implicit search
site, or b) enclose the implicit search site.

The second criterion used to prevent consideration of `O` in
the super constructor call:

    implicit object O extends C( { implicitly[X] })

However, after scala#4043, the
block containing the implicit search is typechecked in a context
owned by a local dummy symbol rather than by `O`. (The dummy and
`O` share an owner.)

This led to `O` being considered as a candidate for this implicit
search. This search is undertaken during completion of the info of
`O`, which leads to it being excluded on account of the LOCKED flag.

Unfortunately, this also excludes it from use in implicit search
sites subsequent to `O`, as `ImplicitInfo` caches
`isCyclicOrErroneous`.

This commit adjusts the position of the local dummy to be identical
to that of the object. This serves to exclude `O` as a candidate
during the super call on account of criterion a).
albsadowski pushed a commit to albsadowski/scala that referenced this pull request Feb 17, 2015
Implicit search declines to force the info of candidate implicits
that either a) are defined beyond the position of the implicit search
site, or b) enclose the implicit search site.

The second criterion used to prevent consideration of `O` in
the super constructor call:

    implicit object O extends C( { implicitly[X] })

However, after scala#4043, the
block containing the implicit search is typechecked in a context
owned by a local dummy symbol rather than by `O`. (The dummy and
`O` share an owner.)

This led to `O` being considered as a candidate for this implicit
search. This search is undertaken during completion of the info of
`O`, which leads to it being excluded on account of the LOCKED flag.

Unfortunately, this also excludes it from use in implicit search
sites subsequent to `O`, as `ImplicitInfo` caches
`isCyclicOrErroneous`.

This commit adjusts the position of the local dummy to be identical
to that of the object. This serves to exclude `O` as a candidate
during the super call on account of criterion a).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants