Try fix #1786: support use package object as value

[liufengyun]

Try fix #1786: support use package object as value.

[sjrd]

Does this still prevent

val m = scala.meta

? Is there a neg test somewhere against that?

[liufengyun]

Thanks @sjrd . We auto convert scala.meta to scala.meta.package when it's used as a value?

[sjrd]

My point is: val m = scala.meta should not be allowed. But if you directly apply it, as in val m = scala.meta { something }, then it's equivalent to calling val m = scala.meta.apply { something }, and that should be allowed.

val m = scala.meta should not be allowed because it would be inconsistent whether the package has a package object or not. For example, if package foo.bar does not have a package object, then val b = foo.bar is technically unimplementable, so must be forbidden. For consistency, so should it be forbidden when foo.bar has a package object.

[DarkDimius]

@liufengyun we already have checks that ensure that java static classes cannot be assigned to variables or passed as arguments. We can use the same infrastructure as restrictions on package are the same.

[liufengyun]

If we allow scala.meta { something } but disallow val m = scala.meta, it seems to break hierarchical typing. It also has consistency issue, as we allow both meta { ... } and val m = meta.

if package foo.bar does not have a package object, then val b = foo.bar is technically unimplementable

What if we only do the auto conversion if the package object indeed exists?

[sjrd]

What if we only do the auto conversion if the package object indeed exists?

That's precisely what I am against, because it's ultra-inconsistent.

as we allow both meta { ... } and val m = meta

That's not normal. val m = meta shouldn't be allowed either if meta is a package or package object.

As @DarkDimius pointed out, the same kind of restrictions apply to Java static objects, so hierarchical typing is already broken (and has been since the dawn of time).

[liufengyun]

Got it, thanks @sjrd @DarkDimius . I'll update the PR soon.

[odersky]

val m = scala.meta should not be allowed because it would be inconsistent whether the package has a package object or not. For example, if package foo.bar does not have a package object, then val b = foo.bar is technically unimplementable, so must be forbidden. For consistency, so should it be forbidden when foo.bar has a package object.

I tend to agree with this point of view. What is the motivation for allowing package objects as values?

[odersky]

Note that I think you can get the package object

foo.bar.package

as a value. That's OK, as this is a real object. By contrast.

foo.bar

is the package, which contains all members of the package object and all members defined in the package. If I write

val m = foo.bar

There's no way that afterwards m.C would select a class defined in package foo.bar bit missing from foo.bar.package.

So I think we should not allow packages which have package objects as values.

[liufengyun]

@odersky The motivation is following code:

package object meta {
  def apply(x: Int): Int = x * x
}

class Test {
  def f = meta { 5 + 4 }
}

Of course, scala.meta could define a method def meta(x: Int): Int, but it doesn't look very nice.

I see the argument for against use package object as value. What about just adapt foo.bar {...} to foo.bar.package.apply {...} in the context of application, while disallow other usages?

[odersky]

On Tue, Dec 13, 2016 at 3:22 PM, liu fengyun ***@***.***> wrote: @odersky <https://github.com/odersky> The motivation is following code: package object meta { def apply(x: Int): Int = x * x } class Test { def f = meta { 5 + 4 } } Of course, scala.meta could define a method def meta(x: Int): Int, but it doesn't look very nice. I see the argument for against use package object as value. What about just adapt foo.bar {...} to foo.bar.package.apply {...} in the context of application, while disallow other usages?

Yes, I think that's the right thing to do. And it should be easy. In checkValue, we omit the check if the prototype is a SelectionProto. We should omit it as well if the prototype is a FunProto or a PolyProto, because in these cases we will insert an `apply` method later on. —

…

You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1787 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAwlVnVKMtQ_rfpel2AhsPXdgVkrkk0aks5rHqoxgaJpZM4LLtcU> . {"api_version":"1.0","publisher":{"api_key":" 05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity": {"external_key":"github/lampepfl/dotty","title":" lampepfl/dotty","subtitle":"GitHub repository","main_image_url":" https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88- 11e6-95fc-7290892c7bb5.png","avatar_image_url":"https:// cloud.githubusercontent.com/assets/143418/15842166/ 7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/lampepfl/dotty"}}," ***@***.*** in #1787: @odersky The motivation is following code:\r\n\r\n```\r\npackage object meta {\r\n def apply(x: Int): Int = x * x\r\n}\r\n\r\nclass Test {\r\n def f = meta { 5 + 4 }\r\n}\r\n```\r\n\r\nOf course, scala.meta could define a method `def meta(x: Int): Int`, but it doesn't look very nice.\r\n\r\nI see the argument for against use package object as value. What about just adapt `foo.bar {...}` to `foo.bar.package.apply {...}` in the context of application, while disallow other usages?\r\n"}],"action":{"name":"View Pull Request","url":"https://github.com/lampepfl/dotty/ pull/1787#issuecomment-266750609"}}}

-- Prof. Martin Odersky LAMP/IC, EPFL

[liufengyun]

Thanks for the tip @odersky , that's an incredibly simple fix! It's done now.

I didn't notice ctx.makePackageObjPrefixExplicit. I thought we have to manually adapt scala.meta {...} to scala.meta.package {...} before the apply adaption.

[odersky]

LGTM