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

Fix #9161: Add toString to companion modules #9165

Merged
merged 1 commit into from
Aug 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion compiler/src/dotty/tools/dotc/ast/Desugar.scala
Original file line number Diff line number Diff line change
Expand Up @@ -719,7 +719,10 @@ object desugar {
DefDef(methName, derivedTparams, (unapplyParam :: Nil) :: Nil, unapplyResTp, unapplyRHS)
.withMods(synthetic)
}
companionDefs(companionParent, applyMeths ::: unapplyMeth :: companionMembers)
val toStringMeth =
DefDef(nme.toString_, Nil, Nil, TypeTree(), Literal(Constant(className.toString))).withMods(Modifiers(Override | Synthetic))
Copy link
Member

Choose a reason for hiding this comment

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

Scalac is a bit weird:

scala> case class Foo(x: Int); object Foo
class Foo
object Foo

scala> Foo
val res0: Foo.type = Foo$@fe34b86

scala> case class Foo(x: Int)
class Foo

scala> Foo
val res1: Foo.type = Foo

I'm not sure if it's worth replicating this exact behavior, but maybe worth checking the scalac logic which determines when toString is inserted.

Copy link
Member

@smarter smarter Jun 11, 2020

Choose a reason for hiding this comment

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

Seems like scalac only adds the toString in the situations where it also adds Function1 as a parent of the object (to avoid getting "<function1>" as the toString), it's a bit weird but doing the same would make it easier to do bytecode diff between scalac and dotty.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There isn't a way to know if the module will extend Function1 while desugaring. I can't find a place to do it while typing either. Any suggestions?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe check if the moduledef is compiler-generated (span.isZeroExtent) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could not find a tree here with a span that was different for explicit or non explicit modules.

Copy link
Member

Choose a reason for hiding this comment

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

There isn't a way to know if the module will extend Function1 while desugaring

Actually this is implemented in desugaring, see companionParent

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately, both the get a Function in companionParent, the explicit declaration loses it somewhere later (not sure where).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any other idea?

Copy link
Member

Choose a reason for hiding this comment

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

I think it's fine as is.


companionDefs(companionParent, applyMeths ::: unapplyMeth :: toStringMeth :: companionMembers)
}
else if (companionMembers.nonEmpty || companionDerived.nonEmpty || isEnum)
companionDefs(anyRef, companionMembers)
Expand Down
2 changes: 1 addition & 1 deletion tests/run-macros/tasty-extractors-2.check
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ TypeRef(ThisType(TypeRef(NoPrefix(), "scala")), "Unit")
Inlined(None, Nil, Block(List(ClassDef("Foo", DefDef("<init>", Nil, List(Nil), Inferred(), None), List(Apply(Select(New(Inferred()), "<init>"), Nil)), Nil, None, List(DefDef("a", Nil, Nil, Inferred(), Some(Literal(Constant(0))))))), Literal(Constant(()))))
TypeRef(ThisType(TypeRef(NoPrefix(), "scala")), "Unit")

Inlined(None, Nil, Block(List(ClassDef("Foo", DefDef("<init>", Nil, List(Nil), Inferred(), None), List(Apply(Select(New(Inferred()), "<init>"), Nil), TypeSelect(Select(Ident("_root_"), "scala"), "Product"), TypeSelect(Select(Ident("_root_"), "scala"), "Serializable")), Nil, None, List(DefDef("copy", Nil, List(Nil), Inferred(), Some(Apply(Select(New(Inferred()), "<init>"), Nil))))), ValDef("Foo", TypeIdent("Foo$"), Some(Apply(Select(New(TypeIdent("Foo$")), "<init>"), Nil))), ClassDef("Foo$", DefDef("<init>", Nil, List(Nil), Inferred(), None), List(Apply(Select(New(Inferred()), "<init>"), Nil), Applied(Inferred(), List(Inferred()))), Nil, Some(ValDef("_", Singleton(Ident("Foo")), None)), List(DefDef("apply", Nil, List(Nil), Inferred(), Some(Apply(Select(New(Inferred()), "<init>"), Nil))), DefDef("unapply", Nil, List(List(ValDef("x$1", Inferred(), None))), Singleton(Literal(Constant(true))), Some(Literal(Constant(true))))))), Literal(Constant(()))))
Inlined(None, Nil, Block(List(ClassDef("Foo", DefDef("<init>", Nil, List(Nil), Inferred(), None), List(Apply(Select(New(Inferred()), "<init>"), Nil), TypeSelect(Select(Ident("_root_"), "scala"), "Product"), TypeSelect(Select(Ident("_root_"), "scala"), "Serializable")), Nil, None, List(DefDef("copy", Nil, List(Nil), Inferred(), Some(Apply(Select(New(Inferred()), "<init>"), Nil))))), ValDef("Foo", TypeIdent("Foo$"), Some(Apply(Select(New(TypeIdent("Foo$")), "<init>"), Nil))), ClassDef("Foo$", DefDef("<init>", Nil, List(Nil), Inferred(), None), List(Apply(Select(New(Inferred()), "<init>"), Nil), Applied(Inferred(), List(Inferred()))), Nil, Some(ValDef("_", Singleton(Ident("Foo")), None)), List(DefDef("apply", Nil, List(Nil), Inferred(), Some(Apply(Select(New(Inferred()), "<init>"), Nil))), DefDef("unapply", Nil, List(List(ValDef("x$1", Inferred(), None))), Singleton(Literal(Constant(true))), Some(Literal(Constant(true)))), DefDef("toString", Nil, Nil, Inferred(), Some(Literal(Constant("Foo"))))))), Literal(Constant(()))))
TypeRef(ThisType(TypeRef(NoPrefix(), "scala")), "Unit")

Inlined(None, Nil, Block(List(ClassDef("Foo1", DefDef("<init>", Nil, List(List(ValDef("a", TypeIdent("Int"), None))), Inferred(), None), List(Apply(Select(New(Inferred()), "<init>"), Nil)), Nil, None, List(ValDef("a", Inferred(), None)))), Literal(Constant(()))))
Expand Down
4 changes: 4 additions & 0 deletions tests/run/i9161.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@

final case class T(i:Int)

@main def Test = assert(T.toString == "T")
28 changes: 20 additions & 8 deletions tests/semanticdb/metac.expect
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ Schema => SemanticDB v4
Uri => Classes.scala
Text => empty
Language => Scala
Symbols => 106 entries
Symbols => 109 entries
Occurrences => 130 entries

Symbols:
Expand All @@ -363,6 +363,7 @@ classes/C3#x. => val method x
classes/C3. => final object C3
classes/C3.apply(). => method apply
classes/C3.apply().(x) => param x
classes/C3.toString(). => method toString
classes/C3.unapply(). => method unapply
classes/C3.unapply().(x$1) => param x$1
classes/C4# => case class C4
Expand All @@ -376,6 +377,7 @@ classes/C4#x. => val method x
classes/C4. => final object C4
classes/C4.apply(). => method apply
classes/C4.apply().(x) => param x
classes/C4.toString(). => method toString
classes/C4.unapply(). => method unapply
classes/C4.unapply().(x$1) => param x$1
classes/C6# => case class C6
Expand All @@ -389,6 +391,7 @@ classes/C6#x. => val method x
classes/C6. => final object C6
classes/C6.apply(). => method apply
classes/C6.apply().(x) => param x
classes/C6.toString(). => method toString
classes/C6.unapply(). => method unapply
classes/C6.unapply().(x$1) => param x$1
classes/C7# => class C7
Expand Down Expand Up @@ -638,7 +641,7 @@ Schema => SemanticDB v4
Uri => Enums.scala
Text => empty
Language => Scala
Symbols => 167 entries
Symbols => 169 entries
Occurrences => 203 entries

Symbols:
Expand Down Expand Up @@ -706,6 +709,7 @@ _empty_/Enums.Maybe.Just. => final object Just
_empty_/Enums.Maybe.Just.apply(). => method apply
_empty_/Enums.Maybe.Just.apply().(value) => param value
_empty_/Enums.Maybe.Just.apply().[A] => typeparam A
_empty_/Enums.Maybe.Just.toString(). => method toString
_empty_/Enums.Maybe.Just.unapply(). => method unapply
_empty_/Enums.Maybe.Just.unapply().(x$1) => param x$1
_empty_/Enums.Maybe.Just.unapply().[A] => typeparam A
Expand Down Expand Up @@ -797,6 +801,7 @@ _empty_/Enums.`<:<`.Refl#ordinal(). => method ordinal
_empty_/Enums.`<:<`.Refl. => final object Refl
_empty_/Enums.`<:<`.Refl.apply(). => method apply
_empty_/Enums.`<:<`.Refl.apply().[C] => typeparam C
_empty_/Enums.`<:<`.Refl.toString(). => method toString
_empty_/Enums.`<:<`.Refl.unapply(). => method unapply
_empty_/Enums.`<:<`.Refl.unapply().(x$1) => param x$1
_empty_/Enums.`<:<`.Refl.unapply().[C] => typeparam C
Expand Down Expand Up @@ -2075,7 +2080,7 @@ Schema => SemanticDB v4
Uri => NamedApplyBlock.scala
Text => empty
Language => Scala
Symbols => 45 entries
Symbols => 46 entries
Occurrences => 46 entries

Symbols:
Expand Down Expand Up @@ -2104,6 +2109,7 @@ example/NamedApplyBlockCaseClassConstruction.Msg.apply(). => method apply
example/NamedApplyBlockCaseClassConstruction.Msg.apply().(body) => param body
example/NamedApplyBlockCaseClassConstruction.Msg.apply().(head) => param head
example/NamedApplyBlockCaseClassConstruction.Msg.apply().(tail) => param tail
example/NamedApplyBlockCaseClassConstruction.Msg.toString(). => method toString
example/NamedApplyBlockCaseClassConstruction.Msg.unapply(). => method unapply
example/NamedApplyBlockCaseClassConstruction.Msg.unapply().(x$1) => param x$1
example/NamedApplyBlockCaseClassConstruction.bodyText. => val method bodyText
Expand Down Expand Up @@ -2181,7 +2187,7 @@ Schema => SemanticDB v4
Uri => NamedArguments.scala
Text => empty
Language => Scala
Symbols => 15 entries
Symbols => 16 entries
Occurrences => 13 entries

Symbols:
Expand All @@ -2197,6 +2203,7 @@ example/NamedArguments#User#name. => val method name
example/NamedArguments#User. => final object User
example/NamedArguments#User.apply(). => method apply
example/NamedArguments#User.apply().(name) => param name
example/NamedArguments#User.toString(). => method toString
example/NamedArguments#User.unapply(). => method unapply
example/NamedArguments#User.unapply().(x$1) => param x$1
example/NamedArguments#`<init>`(). => primary ctor <init>
Expand Down Expand Up @@ -2434,7 +2441,7 @@ Schema => SemanticDB v4
Uri => Synthetic.scala
Text => empty
Language => Scala
Symbols => 37 entries
Symbols => 38 entries
Occurrences => 162 entries

Symbols:
Expand All @@ -2460,6 +2467,7 @@ example/Synthetic#s.Bar#`<init>`(). => primary ctor <init>
example/Synthetic#s.Bar#copy(). => method copy
example/Synthetic#s.Bar. => final object Bar
example/Synthetic#s.Bar.apply(). => method apply
example/Synthetic#s.Bar.toString(). => method toString
example/Synthetic#s.Bar.unapply(). => method unapply
example/Synthetic#s.Bar.unapply().(x$1) => param x$1
example/Synthetic#s.apply(). => method apply
Expand Down Expand Up @@ -3116,7 +3124,7 @@ Schema => SemanticDB v4
Uri => recursion.scala
Text => empty
Language => Scala
Symbols => 35 entries
Symbols => 36 entries
Occurrences => 56 entries

Symbols:
Expand Down Expand Up @@ -3148,6 +3156,7 @@ recursion/Nats.Succ. => final object Succ
recursion/Nats.Succ.apply(). => method apply
recursion/Nats.Succ.apply().(p) => param p
recursion/Nats.Succ.apply().[N] => typeparam N
recursion/Nats.Succ.toString(). => method toString
recursion/Nats.Succ.unapply(). => method unapply
recursion/Nats.Succ.unapply().(x$1) => param x$1
recursion/Nats.Succ.unapply().[N] => typeparam N
Expand Down Expand Up @@ -3403,7 +3412,7 @@ Schema => SemanticDB v4
Uri => semanticdb-Types.scala
Text => empty
Language => Scala
Symbols => 140 entries
Symbols => 142 entries
Occurrences => 250 entries

Symbols:
Expand Down Expand Up @@ -3431,6 +3440,7 @@ types/Foo#s. => val method s
types/Foo. => final object Foo
types/Foo.apply(). => method apply
types/Foo.apply().(s) => param s
types/Foo.toString(). => method toString
types/Foo.unapply(). => method unapply
types/Foo.unapply().(x$1) => param x$1
types/Foo.x. => val method x
Expand Down Expand Up @@ -3482,6 +3492,7 @@ types/Test.C#RepeatedType#s. => val method s
types/Test.C#RepeatedType. => final object RepeatedType
types/Test.C#RepeatedType.apply(). => method apply
types/Test.C#RepeatedType.apply().(s) => param s
types/Test.C#RepeatedType.toString(). => method toString
types/Test.C#RepeatedType.unapplySeq(). => method unapplySeq
types/Test.C#RepeatedType.unapplySeq().(x$1) => param x$1
types/Test.C#TypeType. => final object TypeType
Expand Down Expand Up @@ -3808,7 +3819,7 @@ Schema => SemanticDB v4
Uri => semanticdb-extract.scala
Text => empty
Language => Scala
Symbols => 17 entries
Symbols => 18 entries
Occurrences => 22 entries

Symbols:
Expand All @@ -3824,6 +3835,7 @@ _empty_/AnObject.Foo#x. => val method x
_empty_/AnObject.Foo. => final object Foo
_empty_/AnObject.Foo.apply(). => method apply
_empty_/AnObject.Foo.apply().(x) => param x
_empty_/AnObject.Foo.toString(). => method toString
_empty_/AnObject.Foo.unapply(). => method unapply
_empty_/AnObject.Foo.unapply().(x$1) => param x$1
_empty_/AnObject.foo(). => method foo
Expand Down