SI-9131 Fix use of apply syntactic sugar with by-name param#4286
Merged
adriaanm merged 1 commit intoscala:2.12.xfrom Feb 23, 2015
Merged
SI-9131 Fix use of apply syntactic sugar with by-name param#4286adriaanm merged 1 commit intoscala:2.12.xfrom
adriaanm merged 1 commit intoscala:2.12.xfrom
Conversation
9bc0dda to
c676da6
Compare
Contributor
|
Could you remove the @ in my |
Member
Author
|
Duly scrubbed. |
Contributor
|
ta |
Member
Author
|
/rebuild |
Contributor
|
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.
Member
Author
|
Rebased. |
Contributor
|
LGTM |
Member
Author
|
/synch |
Member
Author
|
/rebuild |
Member
Author
|
/synch |
Contributor
|
@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) |
Member
Author
|
gotcha, thanks. |
Contributor
|
/synch |
Contributor
|
/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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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,
insertApplystabilizes the type ofqual, e.g.replacing
Ident(foo).setType(typeOf[Int])withIdent(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:
When I last touched this code in SI-6206 / 267650c, I noted:
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
insertApplyin favour ofTreeGen.stabilize.We then avoid the ill-formed singleton type, and the spurious
"apply is not a member" type error.