Skip to content

SI-9131 Fix use of apply syntactic sugar with by-name param#4286

Merged
adriaanm merged 1 commit intoscala:2.12.xfrom
retronym:ticket/9131
Feb 23, 2015
Merged

SI-9131 Fix use of apply syntactic sugar with by-name param#4286
adriaanm merged 1 commit intoscala:2.12.xfrom
retronym:ticket/9131

Conversation

@retronym
Copy link
Member

@retronym retronym commented Feb 2, 2015

After typechecking a tree, the typer adapts it to the current
mode and expected type. If we are in FUNmode (typechecking the
qualifier of a value- or type-application), and the tree does not
already have a MethodType or PolyType, it is reinterepreted as
qual.apply.

In doing so, insertApply stabilizes the type of qual, e.g.
replacing Ident(foo).setType(typeOf[Int]) with
Ident(foo).setType(typeOf[foo.type]).

However, this does not check for by-name parameters, which cannot
form the basis for a singleton type, as we can see by trying that
directly:

scala> def foo(a: => String) { type T = a.type }
<console>:7: error: stable identifier required, but a.type found.
       def foo(a: => String) { type T = a.type }
                                         ^

When I last touched this code in SI-6206 / 267650c, I noted:

// TODO reconcile the overlap between Typers#stablize and TreeGen.stabilize

I didn't get around to that, but @adriaanm gave that code a thorough
cleanup in fada1ef.

It appears that on the back of his work, we can now remove the local
stabilization logic in insertApply in favour of TreeGen.stabilize.
We then avoid the ill-formed singleton type, and the spurious
"apply is not a member" type error.

@scala-jenkins scala-jenkins added this to the 2.12.0-M1 milestone Feb 2, 2015
@retronym retronym closed this Feb 2, 2015
@retronym retronym reopened this Feb 2, 2015
@retronym retronym force-pushed the ticket/9131 branch 2 times, most recently from 9bc0dda to c676da6 Compare February 2, 2015 11:20
@adriaanm
Copy link
Contributor

adriaanm commented Feb 2, 2015

Could you remove the @ in my @cameo in your commit message. Otherwise, github will ping me every time this commit passes by in a merge in any repo, and @me's go straight to the top of my inbox.

@retronym
Copy link
Member Author

retronym commented Feb 3, 2015

Duly scrubbed.

@adriaanm
Copy link
Contributor

adriaanm commented Feb 3, 2015

ta

@retronym
Copy link
Member Author

retronym commented Feb 3, 2015

/rebuild

@adriaanm
Copy link
Contributor

adriaanm commented Feb 3, 2015

Ah yes, the IDE build fail: see #4287

After typechecking a tree, the typer adapts it to the current
mode and expected type. If we are in FUNmode (typechecking the
qualifier of a value- or type-application), and the tree does not
already have a MethodType or PolyType, it is reinterepreted as
`qual.apply`.

In doing so, `insertApply` stabilizes the type of `qual`, e.g.
replacing `Ident(foo).setType(typeOf[Int])` with
`Ident(foo).setType(typeOf[foo.type])`.

However, this does not check for by-name parameters, which cannot
form the basis for a singleton type, as we can see by trying that
directly:

```
scala> def foo(a: => String) { type T = a.type }
<console>:7: error: stable identifier required, but a.type found.
       def foo(a: => String) { type T = a.type }
                                         ^
```

When I last touched this code in SI-6206 / 267650c, I noted:

    // TODO reconcile the overlap between Typers#stablize and TreeGen.stabilize

I didn't get around to that, but Adriaan gave that code a thorough
cleanup in fada1ef.

It appears that on the back of his work, we can now remove the local
stabilization logic in `insertApply` in favour of `TreeGen.stabilize`.
We then avoid the ill-formed singleton type, and the spurious
"apply is not a member" type error.

I did have to modify `isStableIdent` to check the symbol's info
in addition to the tree's type for by-name-ness.
@retronym
Copy link
Member Author

retronym commented Feb 3, 2015

Rebased.

@adriaanm
Copy link
Contributor

adriaanm commented Feb 3, 2015

LGTM

@adriaanm adriaanm self-assigned this Feb 3, 2015
@retronym
Copy link
Member Author

retronym commented Feb 5, 2015

/synch

@retronym
Copy link
Member Author

retronym commented Feb 5, 2015

/rebuild

@retronym
Copy link
Member Author

retronym commented Feb 5, 2015

/synch

@adriaanm
Copy link
Contributor

adriaanm commented Feb 5, 2015

@retronym the different jobs are an artifact of a bug i introduced in the bot while trying to harmonize the abbreviation of jenkins job names to context in the commit statuses (see slack)

@retronym
Copy link
Member Author

retronym commented Feb 5, 2015

gotcha, thanks.

@adriaanm
Copy link
Contributor

adriaanm commented Feb 5, 2015

/synch

@adriaanm
Copy link
Contributor

adriaanm commented Feb 5, 2015

/nothingtoseehere

adriaanm added a commit that referenced this pull request Feb 23, 2015
SI-9131 Fix use of apply syntactic sugar with by-name param
@adriaanm adriaanm merged commit f8853a8 into scala:2.12.x Feb 23, 2015
@adriaanm adriaanm added the 2.12 label Oct 29, 2016
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.

3 participants