Remove most uses of scala-reflect jar

[smarter]

The remaining uses of scala-reflect (scala.reflect.io.*, scala.reflect.internal.util.WeakHashSet) cannot be removed until we integrate the backend in the dotty repo.

[smarter]

Interestingly this revealed a bug in how we set PureInterface which is now fixed by the latest commit.

[smarter]

I think there might be more issues in defKind but this is off-topic, I'll open an issue after this PR is merged.

[odersky]

Otherwise LGTM

[odersky]

I did not understand why the change. I assume it has to do with default arguments? But, first, if a parameter has a default value the enclosing trait it's still a NoInits, and second, won't we then see the default method which gives us the right DefKind anyway?

[smarter]

Yes, it's for default arguments, the corresponding commit message gives more information: 618fe96

if a parameter has a default value the enclosing trait is still a NoInits

Ah indeed, I missed that.

won't we then see the default method which gives us the right DefKind anyway?

No, because defKind is called on each element of the body of the class before they are desugared.

It would be much simpler if defKind was called after desugaring (and would also avoid other issues with default parameters of class constructors for example) but I don't know what that would involve.

[odersky]

Right. I missed the context before. defKind is called before the trees are desugared. Yes, then we need to do it as you describe except for the NoInits issue. Also since we traverse all methods of a trait here I'd avoid creating too much garbage via flatten. I'd just do a double forall:

if (tree.unforcedRhs == EmptyTree) && tree.vparamss.forall(_.forall(_.rhs.isEmpty)) NoInitsInterface else NoInits

[odersky]

Does this file not duplicate standard collection functionality? Why keep it?

[smarter]

I'm fine either way, apparently the original motivation for these methods is that they're faster than doing the equivalent using chains of methods from the standard library

[odersky]

I doubt it. If we do want to keep these methods, we should also move them to Decorators.

[smarter]

What would map2 be decorated on? Anyway, I'll just remove those methods in favor of the standard library, it shouldn't make much of a difference.

[odersky]

I think this should go in StringDecorator in file Decorators.scala

[smarter]

Sure.

[smarter]

@odersky Comments addressed, thanks! The existing StringDecorators was an instance of PreName so it felt wrong to add random methods to it, instead I renamed it to PreNamedString and added a new StringDecorators implicit class. I updated individual commits in this PR to have a cleaner history but that makes it harder to review the changes, you can see the diff from the previous set of commits at dotty-staging@fa6f988

[smarter]

Interesting, adding the new StringDecorator broke the bootstrap in a weird way:

[error] -- [E008] Member Not Found Error: /home/smarter/opt/dotty/compiler/src/dotty/tools/dotc/core/Comments.scala 
[error] 405 |      defs(sym) ++= defines(raw).map {
[error]     |      ^^^^^^^^^^^^^
[error]     |value `++=` is not a member of scala.collection.immutable.Map[String, String] - did you mean `scala$collection$MapLike$$B.+`?
[error] -- [E008] Member Not Found Error: /home/smarter/opt/dotty/compiler/src/dotty/tools/dotc/typer/Docstrings.scala 
[error] 41 |          tplExp.defineVariables(sym)
[error]    |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^
[error]    |value `++=` is not a member of scala.collection.immutable.Map[String, String] - did you mean `scala$collection$MapLike$$B.+`?

The errors go away if I unimport StringDecorator, I'll investigate.

[smarter]

The issue was due to dotty trying to use the private splitAt in StringDecorator, I've fixed that by hiding splitAt better but I'll open an issue for that.