fix:better error msg for cyclic error for constructors

[doofin]

This provides better error msg for #17076 , doesn't solve the cyclic error yet

[dwijnand]

@doofin can we have a test case for this, at least?

[doofin]

@doofin can we have a test case for this, at least?

ok,I'll try to look at tests and find a suitable place to add them

[doofin]

@dwijnand added one test case

[doofin]

Hi @dwijnand @ckipp01 got some time to check this ? Just saw the new scala blog post "Give your feedback to improve error reporting" and it reminds me of this, thanks!

[nicolasstucki]

This comment is not necessary.

[doofin]

Is it expected that a symbol can both be isMethod and isConstructor ?

[nicolasstucki]

Yes. It is a method because it is represented with a def.

[doofin]

if I remember correctly, isMethod is true(which shouldn't be) for case class as in #17076 so I add this warning, maybe I should indicate in a different way? (or maybe open a different issue for isMethod)

[nicolasstucki]

If we print in PostTyper

        case tree: DefDef =>
          println("---")
          println("DefDef: " + tree.show)
          println("flags: " + tree.symbol.flagsString)
          println("isConstructor: " + tree.symbol.isConstructor)

we can see that this class

class F(a: Int):
  def this() = this(0)
  def f = 1

has the the Method flag and isConstructor to true for both the primary and secondary constructors.

---
DefDef: def <init>(a: Int): Unit
flags: <method> <stable> <touched>
isConstructor: true
---
DefDef: def <init>(): Unit =
  {
    this(0)
    ()
  }
flags: <method> <touched> <no-default-params>
isConstructor: true
---
DefDef: def f: Int = 1
flags: <method> <touched>
isConstructor: false

[nicolasstucki]

What might happen is that the Method was not in flagsUNSAFE because it would be computed and added to flags while typing. But the failure may have happened before that.

[doofin]

thanks i got it, removed the comment

[nicolasstucki]

The error message of the failing code is

-- Error: t/Test.scala:7:30 ----------------------------------------------------
7 |  case class Foo[T <: Int](x: Any)
  |                              ^^^
  |                  Something's wrong: missing original symbol for type tree
-- [E044] Cyclic Error: t/Test.scala:7:2 ---------------------------------------
7 |  case class Foo[T <: Int](x: Any)
  |  ^
  |  Overloaded or recursive constructor Foo needs return type
  |
  |   Run with -explain-cyclic for more details.
  |
  | longer explanation available when compiling with `-explain`

We should fix the Something's wrong: missing original symbol for type tree. Patching the second error is not going to help. It might even hide useful information such as the info of -explain-cyclic that tells us how this happened.

[doofin]

The error message of the failing code is

-- Error: t/Test.scala:7:30 ----------------------------------------------------
7 |  case class Foo[T <: Int](x: Any)
  |                              ^^^
  |                  Something's wrong: missing original symbol for type tree
-- [E044] Cyclic Error: t/Test.scala:7:2 ---------------------------------------
7 |  case class Foo[T <: Int](x: Any)
  |  ^
  |  Overloaded or recursive constructor Foo needs return type
  |
  |   Run with -explain-cyclic for more details.
  |
  | longer explanation available when compiling with `-explain`

We should fix the Something's wrong: missing original symbol for type tree. Patching the second error is not going to help. It might even hide useful information such as the info of -explain-cyclic that tells us how this happened.

@nicolasstucki could you point out which file/LOC that I should change?

[nicolasstucki]

@nicolasstucki could you point out which file/LOC that I should change?

Not sure where the bug that ended up with Something's wrong: missing original symbol for type tree originates. This is caught in

https://github.com/lampepfl/dotty/blob/6efcdbaa3df8a6b1e48dfcb208efe682bf5382e6/compiler/src/dotty/tools/dotc/typer/Typer.scala#L2159-L2168

But it does not seem like something simple to figure out.

The message was added in 9c834fc. Which comes from a println in a4516ea.

The issue might be located in tree.ensureCompletions.