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 #18484: Query every legal child of sealed class on children call #18561

Closed
wants to merge 1 commit into from

Conversation

jchyb
Copy link
Contributor

@jchyb jchyb commented Sep 15, 2023

Sometimes, a macro or Mirror can be called/generated for a type that may have not had its children registered by the natural typechecking procedures of the compiler. This meant that, in those cases, the Mirror could not have been generated correctly, and macro could not have obtained the children of the class.

This was partially fixed in a previous PR by introducing a separate procedure for querying children, outside of the regular typechecking procedure. However, the children were only queried for in the owner symbol or in the companion class, whereas non-enum sealed classes specifically, can appear anywhere in the file. This commit aims to rectify this by searching for any class that appears in the same file as the sealed class.

Fixes #18484
I made the test case here a bit more robust than in the issue to better test the changes.

if !is(Enum) then // non-enum sealed class
// Make sure all children are completed, so that
// they show up in Child annotations.
completeChildrenIn(defn.RootClass, recursively = true)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I spent some time trying to find something smarter than just starting at the <root> package - but neither Symbol.topLevelClass nor Symbol.enclosingPackageClass really helped. If some kind of method that returns packages/classes for a source file there already exists in the compiler, it is probably more performant than this (but I could not find anything like that).

…call

Sometimes, a macro or Mirror can be called/generated for a type that
may have not had its children registered by natural typechecking
procedures of the compiler. This meant that, in those cases, the Mirror
could not have been generated correctly, and macro could not obtain the
children of the class.

This was partially fixed in a previous PR by introducing a separate
procedure for querying children, outside of the regular typechecking
procedure. However, they was only queried in the owner symbol or in the
companion class, when non-enum sealed classes specifically, can
appear anywhere in the file. This commit aims to rectify this by
searching for any class that appears in the same file as the sealed
type.
@jchyb
Copy link
Contributor Author

jchyb commented Oct 30, 2023

Apparently, the previous behavior is (or should be made) part of the spec. Even barring the inefficiency of the presented fix, apparently completing children in various corner case situations was a constant problem in Scala 2, thus the more limiting, clear and transparent rule presented in the previous error message (as in, the fact that it asks you to keep all of the children on the same nesting level) might make things more consistent.

@jchyb jchyb closed this Oct 30, 2023
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.

Incorrect children queried before class was discovered error when typeCheckErrors is used
1 participant