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 java typer problems with inner class references and raw types #19747

Merged
merged 6 commits into from
Feb 22, 2024

Conversation

bishabosha
Copy link
Member

Tests changes against source dependency, Tasty dependency, and class dependency.

fixes #19619

@bishabosha bishabosha requested a review from sjrd February 20, 2024 17:31
@bishabosha bishabosha marked this pull request as ready for review February 20, 2024 17:32
@sjrd
Copy link
Member

sjrd commented Feb 21, 2024

dotty.tools.dotc.CompilationTests.posTwice fails, according to CI.

@@ -34,7 +34,7 @@ object ContextOps:
if (elem.name == name) return elem.sym.denot // return self
}
val pre = ctx.owner.thisType
if ctx.isJava then javaFindMember(name, pre, required, excluded)
if ctx.isJava then javaFindMember(name, pre, lookInCompanion = true, required, excluded)
Copy link
Member

Choose a reason for hiding this comment

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

Why are we allowed to look in the companion here? This probably deserves a comment more than the false case.


package lib;

public class RawTypesGen<W> {
Copy link
Member

Choose a reason for hiding this comment

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

This file is already not used in this commit, and it gets removed later. Consider removing it from this commit in the first place.

potential TODO: tree's of Raw types from Java are still incorrect
once they are serialized to TASTy, however it is debateable if we
care to support them.
@bishabosha
Copy link
Member Author

bishabosha commented Feb 21, 2024

this was the error in CI:

-- [E083] Type Error: tests/pos-java-interop-separate/inner-fbounds/Configurer_1.java:6:40 -----------------------------
6 |    public final class Registry extends AbstractRegistry<Registry> {}
  |                                        ^^^^^^^^^^^^^^^^^^^^^^^^^^
  |                                 AbstractConfigurer is not a valid class prefix, since it is not an immutable path
  |
  | longer explanation available when compiling with `-explain`

this basically means that we can not widen the prefix of Java types, or you have to turn off the stability checker in Typer for Java types, or you only widen at Pickler. So I'll have to give up on preventing illegal types in Scala sources for java types like foo.Inner where foo is a local variable (or allow this for Java sources)

@bishabosha bishabosha assigned bishabosha and unassigned sjrd Feb 21, 2024
@bishabosha bishabosha force-pushed the tasty/fix-java-parse-issues branch 2 times, most recently from 858b609 to 114f168 Compare February 21, 2024 17:25
(If we widen Foo.this.Inner in the parents of a class to AbstractFoo#Inner, i.e.
  an inherited class Inner prefixed by its owner abstract class AbstractFoo,
  this is not considered a stable prefix for a class parent type.
  But in Java this is the correct type so allow it.)
@bishabosha bishabosha merged commit 51ea65a into scala:main Feb 22, 2024
17 checks passed
@bishabosha bishabosha deleted the tasty/fix-java-parse-issues branch February 22, 2024 14:54
@Kordyjan Kordyjan added this to the 3.4.2 milestone Mar 28, 2024
WojciechMazur added a commit that referenced this pull request Jul 2, 2024
… types" to LTS (#20934)

Backports #19747 to the LTS branch.

PR submitted by the release tooling.
[skip ci]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Java source dependency with inner class type is incorrectly typed as static selection
3 participants