Fix regression: do not approximate prefixes when using memberType in reflect API

[jchyb]

Fixes #22424

[dwijnand]

No, I don't think we should do it this way. Perhaps we should do something like member.info.asSeenFrom(self.<thisType>, member.owner) .

[jchyb]

@dwijnand After spending some time on this, I don’t think I’m able to use thisType without it causing further regressions after all. Replacing symbols with thisType can cause certain type applications from the prefix to get ignored.
At first, as the memberType implementation, I tried a simple:

          val classSymbol = self.classSymbol
          member.info
            .asSeenFrom(classSymbol.thisType, member.owner)
            .substThis(classSymbol.asClass, self) // we remove the added This(_) for compatibility

But this caused issues for nested classes, like:

trait A2[T]:
  case class C(t: T)

TypeRepr.of[A2[Int]#C].memberType(tFieldSymbol) would then return TypeRef(ThisType(TypeRef(ThisType(TypeRef(NoPrefix,module class <empty>)),trait A2)),type T), instead of an Int - the substThis cannot help here like it does in other cases, as it looks for the C class which is not part of the TypeRef.

Then, to make sure any previously generated thisType class gets replaced with correct part of the original self I tried walking through self and trying to replace this-type of every class I find there:

          val qualTypeRepr = if self.typeSymbol.isClassDef then self.typeSymbol.thisType else self
          val memberTypeWithThis = member.info
            .asSeenFrom(qualTypeRepr, member.owner)
          new TypeAccumulator[Types.Type] {
            def apply(x: Types.Type, tp: Types.Type): Types.Type = tp match
              case at @ AppliedType(prefix, args) => apply(
                if at.typeSymbol.isClassDef then x.substThis(at.typeSymbol.asClass, at) else at,
                prefix.asInstanceOf[Types.Type]
              )
              case other => foldOver(x, other)
          }.apply(
            if self.typeSymbol.isClassDef then memberTypeWithThis.substThis(self.typeSymbol.asClass, self) else memberTypeWithThis,
            self
          )

This fixes the simple nested types like the example above, however it does not work for the following:

trait B2T:
  type T; case class C(t: T)
class B2C[U] extends B2T:
  final override type T = U

Where for (TypeRepr.of[B2C[Int]#C].memberType(tFieldSymbol) we obtain TypeRef(ThisType(TypeRef(ThisType(TypeRef(NoPrefix,module class <empty>)),trait B2T)),type T) instead of an Int.
I suppose I could, for any class that is a part of AppliedType, also go through its parents, but at that point I am just reinventing asSeenFrom.
WIth that in mind, I’d like to resubmit the previous fix with the new asSeenFrom parameter.

[dwijnand]

Where for (TypeRepr.of[B2C[Int]#C].memberType(tFieldSymbol) we obtain TypeRef(ThisType(TypeRef(ThisType(TypeRef(NoPrefix,module class <empty>)),trait B2T)),type T) instead of an Int.

From your example it seems like you create a (B2C[Int]#C).this value (for qualTypeRepr), where I would think you need a specific instance (i.e. stable) of B2C[Int], and then a specific instance of C, in order to select the precise type of the t field.

I really don't like just pipelining a boolean into the algorithm, to crudely toggle it off. I feel like the problem is in the input data.

[jchyb]

I don't think I'm able to fix it then - I tried once again to concoct something like:

          val tempReplacements = scala.collection.mutable.ListBuffer[(dotc.core.Symbols.ClassSymbol, Types.Type)]();
          val newSelf = new Types.TypeMap {
            def apply(tp: Types.Type) = tp match
              case t @ TermRef(prefix, _) if !t.isStable && t.typeSymbol.isClassDef =>
                tempReplacements.addOne((t.typeSymbol.asClass, t))
                t.typeSymbol.thisType
              case other => mapOver(other)
          }.apply(self)
          val res = member.info
            .asSeenFrom(newSelf, member.owner)
          tempReplacements.reverse.foldLeft(res) {
            case (memberTypeResult, (clsSymbol, replacingType)) =>
              memberTypeResult.substThis(clsSymbol, replacingType)
          }

but artificially adding those thisTypes that still breaks shapeless-3 community build, no matter how much I try to minimize the this-type additions, and how much I try to strip them out later after the asSeenFrom. asSeenFrom usually is used for the typechecking algorithm, but from the perspective of the reflect API, memberType is the only way we can actually get the type of a symbol. This means that, for that particular use case we want this type to be as precise as possible, and we never want anything replaced with Nothing. It's very possible that I just don't understand the concept of stability here and there is an obvious answer to all of this - I'm sorry.

[jchyb]

@dwijnand I finally found a solution that works well without changing asSeenFrom - it looks like it's easier to adjust member.info to always fit self, than doing the opposite and then stripping the unnecessary This trees later. It passed the new test and the tests with memberType in the closed community build (I did have to rerun just now, as I realized I incorrectly named a val).

[dwijnand]

LGTM, nicely done!!!

[jchyb]

Thank you!