Fix #8748: Verify symbol is a param by querying the reftree

[DieBauer]

Verify symbol is a param by querying the reftree

In the HoistSuperArgs phase we have established which tree should be
hoisted, and replace all references in the Tree with params that are
accessible in the hoisted tree.

When the Tree contains an anonymous class without parameters,
the paramSyms of that class refer to the created 'fresh' super symbol
(e.g. B$superArg$1), while the RefTree symbol belongs to the superclass B.
This meant that the conditional was never satisfied, and the type was
never replaced by the primary constructor argument of the generated
superArg class.
This is mitigated by replacing the check, to only take params or
accessors into account in the scope of the hoisted tree.

Closes #8748 and closes #8786

I've run dotty-compiler/test and testCompilation and all is green on my machine.

/cc @liufengyun

[dottybot]

Hello, and thank you for opening this PR! 🎉

All contributors have signed the CLA, thank you! ❤️

Have an awesome day! ☀️

[liufengyun]

Good diagnosis 👍

In the commit message, we can add that treeMap is called before typeMap on a given tree, thus previously the code in treeMap is never executed.

[liufengyun]

It's better to strengthen the condition a little:

-                case tree: RefTree if paramSyms.contains(tree.symbol) =>
+                case tree: RefTree if needsRewire(tree.tpe.asInstanceOf[NamedType]) =>

where needsRewire is the condition in the typeMap:

            def needsRewire(tp: NamedType) =
                (tp.symbol.owner == cls || tp.symbol.owner == constr) && tp.symbol.isParamOrAccessor

[DieBauer]

Totally agree, although all tests pass it is better to make it more strict. Thanks for the suggestion.

One question, why should we cast this to NamedType instead of pattern match like:

def needsRewire(t: Type): Boolean = t match {
        case tp: NamedType => (tp.symbol.owner == cls || tp.symbol.owner == constr) && tp.symbol.isParamOrAccessor
        case _             => false
      }

Can the RefTree type (Select, Ident) never be anything else than a TermRef or TypeRef? if that's the case then we can 'safely' cast this. And safe a pattern match :)

[liufengyun]

Yes, it's better to do a pattern match (the cast should be safe, though). And we can remove duplicate code in typeMap to reuse needsRewire.

[DieBauer]

Thanks, I have applied your suggestion, and removed the duplicated code in the typeMap.

[odersky]

Nice! Thanks for the fix.