Skip to content
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

Import Implied #5868

Merged
merged 10 commits into from Feb 11, 2019
Merged

Import Implied #5868

merged 10 commits into from Feb 11, 2019

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Feb 7, 2019

Introduce a new form import implied to import implied instances. Regular imports don't touch them anymore. See: 16cb974 for the docs.

@odersky odersky changed the title Implicit Flags Refactorings Import Implied Feb 8, 2019
@odersky
Copy link
Contributor Author

odersky commented Feb 8, 2019

I am counting on the bootstrap to test this in anger, both for functionality and for usability. Adding it now, since it is always better to start more restrictive instead of tightening the rules later.

We need that for pretty printing, and generally, to distinguish
the two styles for implicits.
Align with usage elsewhere where a flag name normally applies to both
terms and types, and is qualified with Term or Type if we want only one half.
I get it that `toMessage` will give a more precise error message. But that's no
reason to hide the message obtained from `getMessage` completely.
Interpretation of what `implied` means is still missing.
Implied imports can only import implied definitions.
Regular imports can only import non-implied definitions.
The change of Implicit to be a common flag now caused
some breakage, which this commit fixes.
Add implied import entry to sidebar
Copy link
Contributor

@liufengyun liufengyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be a first-step of a paradigm shift from definition-site implicits to use-site implicits. I like this transition.

However, it seems there is a little issue with programmer's intuitive understanding of lexical scoping and import A._. The discussion is embedded inline.

object B {
import A._
foo // error: no implicit argument was found
foo given tc // error: not found: tc
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forbidding explicit usage of tc here is a little counter-intuitive to lexical scoping, given the literal reading of import A._.

What about the following:

  • import A._ to import all members for explicit usage
  • implied A._ to enable implied instances defined in A for implicit resolution
  • implied A.c make A.c available for implicit resolution, no matter A.c is implicit or not.

The last one is up to debate, the idea is to encourage use-site implicits over definition-site implicits.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to use this feature to make any member a candidate for implicit resolution in scope, i.e. import implied A.foo?

@liufengyun liufengyun assigned odersky and unassigned liufengyun Feb 11, 2019
@biboudis biboudis added this to the 0.13 Tech Preview milestone Feb 11, 2019
@odersky
Copy link
Contributor Author

odersky commented Feb 11, 2019

@liufengyun What you propose is definitely worth considering. There's a complication however, in that the "implicitness" of a definition now depends not only on its definition, but also on the way it is imported. This is likely to have profound effects on spec and implementation.

The current PR avoids this problem, and results in the narrowest possible import semantics. So I propose to get this in now since we can always generalize later.

Copy link
Contributor

@liufengyun liufengyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise, LGTM

@@ -18,7 +18,7 @@ import config.Printers.cyclicErrors
class TypeError(msg: String) extends Exception(msg) {
def this() = this("")
def toMessage(implicit ctx: Context): Message = super.getMessage
override def getMessage: String = throw new Exception("Use toMessage instead for TypeError")
override def getMessage: String = super.getMessage
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The call super.getMessage will get empty string, which may hide useful error messages.

@@ -324,7 +325,8 @@ object TastyFormat {
final val OPAQUE = 35
final val EXTENSION = 36
final val GIVEN = 37
final val PARAMsetter = 38
final val IMPLIED = 38
final val PARAMsetter = 39
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Advance tasty version, as the format changes?

@odersky odersky merged commit a198e48 into scala:master Feb 11, 2019
@liufengyun liufengyun deleted the add-import-implied branch February 14, 2019 13:46
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.

None yet

4 participants