Skip to content

Add Symbol.isSuperAccessor to reflection API #13388

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

Merged
merged 1 commit into from
Apr 12, 2024

Conversation

pikinier20
Copy link
Contributor

No description provided.

@@ -249,7 +249,7 @@ trait ClassLikeSupport:
extension (c: ClassDef)
def extractMembers: Seq[Member] = {
val inherited = c.getNonTrivialInheritedMemberTrees.collect {
case dd: DefDef if !dd.symbol.isClassConstructor && !(dd.symbol.isSuperBridgeMethod || dd.symbol.isDefaultHelperMethod) => dd
case dd: DefDef if !dd.symbol.isClassConstructor && !(dd.symbol.isSuperAccessor || dd.symbol.isDefaultHelperMethod) => dd
Copy link
Member

@smarter smarter Sep 10, 2021

Choose a reason for hiding this comment

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

Alternatively, we could make sure to set the Artifact flag (which doesn't do anything at runtime but is used to signal to javac that this method should not be called from user code) on super accessors, default getters, etc.

@nicolasstucki
Copy link
Contributor

Needs tests

@nicolasstucki
Copy link
Contributor

Part of #13371

@nicolasstucki
Copy link
Contributor

Rebased on master

@nicolasstucki
Copy link
Contributor

Rebased on master

@smarter
Copy link
Member

smarter commented Nov 24, 2021

I think my suggestion from #13388 (comment) is still the way to go, unless there's a good reason to specifically care about super accessors from reflection?

@nicolasstucki nicolasstucki assigned pikinier20 and unassigned smarter Jan 13, 2022
Copy link
Contributor

@nicolasstucki nicolasstucki left a comment

Choose a reason for hiding this comment

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

@pikinier20 could you try the alternative solution that does not require this addition?

@pikinier20
Copy link
Contributor Author

pikinier20 commented Apr 25, 2022

@nicolasstucki Do you think it's worth to add such method to API?

@nicolasstucki
Copy link
Contributor

I think it is worth it even if there is another if later we manage to also do this via flags. We will still have code generated before the addition of the flag that will need to fall back to this check.

@pikinier20 you can remove the MiMa filters as they are not needed anymore. Instead you have to run locally scala3-bootstrapped/testCompilation tests/run-custom-args/tasty-inspector/stdlibExperimentalDefinitions.scala and update the list there.

@ckipp01
Copy link
Member

ckipp01 commented May 10, 2023

What's the current status with this. Are you still waiting on a review @pikinier20? @nicolasstucki is this something you can return back to to review?

@nicolasstucki
Copy link
Contributor

I think we should rebase this and merge it. It looks good to me.

@nicolasstucki nicolasstucki merged commit 794ef5a into scala:main Apr 12, 2024
@nicolasstucki nicolasstucki deleted the add-isSuperAccessor branch April 12, 2024 10:05
@Kordyjan Kordyjan added this to the 3.5.0 milestone May 10, 2024
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.

5 participants