Drop requirement that implicit functions must be non-empty #4549
Conversation
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
It's the kind of trivial requirement we should not do. In the case of implicit function types, it turns out that it even blocks something that would be useful! Also, rename NonEmptyFunction to FunctionWithMods.
On the other hand, empty erased function types are again disabled, since they will be compatible with no closures at all. Note: I did not re-instantiate the tests for disallowing empty erased functions. As is so often the case, the most trivial properties get the most tests, precisely because they are easiest to test. But in the end, that just contributes to noise and friction if one wants to change things. neg/i2642.scala does test the relevant bits, that should be enough.
odersky
changed the title
nicolasstucki
previously requested changes
May 22, 2018
| @@ -32,9 +34,12 @@ class ExpandSAMs extends MiniPhase { | |||
| ctx.platform.isSam(cls) | |||
|
|
|||
| override def transformBlock(tree: Block)(implicit ctx: Context): Tree = tree match { | |||
| case Block(stats @ (fn: DefDef) :: Nil, Closure(_, fnRef, tpt)) if fnRef.symbol == fn.symbol => | |||
| case Block(stats @ (fn: DefDef) :: Nil, cl @ Closure(_, fnRef, tpt)) if fnRef.symbol == fn.symbol => | |||
| case _ => (false, false) | ||
| } | ||
| if (isErased && args.isEmpty) ctx.error(em"empty function cannot not be erased", tree.pos) |
There was a problem hiding this comment.
This looks inconsistent. We do provide names for ErasedFunction0 and ErasedImplicitFunction0 and add them in scope, but we do not allow them to be used in function types.
- We could remove the start
ErasedFuctionNandErasedImplicitFuctionNatN = 1 - Or allow them, as a function type that is equivalent to
Funcion0orImplicitFunction0. We could even consider making them type aliases and warn the theerasedkeyword is useless.
There was a problem hiding this comment.
@nicolasstucki Agree we should start ErasedFunctionN at 1. Can you do the change and push it on top of this PR? Thanks!
nicolasstucki
force-pushed
the
change-inline-untpd
branch
from
0068147 to
c85cee0
Compare
|
@odersky I added the requested changes. Appart from my part, the code LGTM. Can you have a look at the last commits. |
odersky
commented
Jun 6, 2018
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.
This removes an arbitrary restriction and is a first step towards unifying by-name types and implicit function types.
It's been trickier than anticipated since we needed a new mechanism to mark nullary implicit closures. Without parameters we cannot look at the parameter modifiers. Instead we produce an implicit function type as closure target type.