Skip to content

Conversation

@hamzaremmal
Copy link
Member

This specific feature was removed when #1212 was solved. The issue was always the unpickler crashing.
In this PR, we actually fix the unpickler and recover the old behaviour. The main reason for this PR is #23770.

@hamzaremmal hamzaremmal self-assigned this Nov 16, 2025
@hamzaremmal hamzaremmal added the needs-minor-release This PR cannot be merged until the next minor release label Nov 16, 2025
else
newSymbol(ctx.owner, name, flags, completer, privateWithin, coord)
}
registerSym(start, sym)
Copy link
Member Author

@hamzaremmal hamzaremmal Nov 16, 2025

Choose a reason for hiding this comment

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

We need to register the symbol before we resolve the annotations (2 lines below)

case name: Name => name == sym.name && tp.symbol == sym
case _ => tp.symbol == sym
case _ => this.symbol == sym
readLater(end, reader =>
Copy link
Member Author

Choose a reason for hiding this comment

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

We delay it because reading the type of the annotation tp will fail; we prepare this call before registering the symbol and we cannot do it after.

@hamzaremmal hamzaremmal added release-notes Should be mentioned in the release notes backport:nominated If we agree to backport this PR, replace this tag with "backport:accepted", otherwise delete it. stat:needs decision Some aspects of this issue need a decision from the maintainance team. labels Nov 16, 2025
@hamzaremmal hamzaremmal marked this pull request as ready for review November 19, 2025 15:27
@hamzaremmal hamzaremmal requested a review from odersky November 19, 2025 15:27
@hamzaremmal hamzaremmal assigned odersky and unassigned hamzaremmal Nov 19, 2025
Copy link
Contributor

@odersky odersky left a comment

Choose a reason for hiding this comment

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

The changes to the unpickler LGTM

@odersky odersky assigned hamzaremmal and unassigned odersky Nov 26, 2025
@hamzaremmal hamzaremmal removed the stat:needs decision Some aspects of this issue need a decision from the maintainance team. label Nov 26, 2025
@hamzaremmal hamzaremmal merged commit f78354a into scala:main Nov 26, 2025
57 checks passed
@hamzaremmal hamzaremmal deleted the self-annot branch November 26, 2025 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:nominated If we agree to backport this PR, replace this tag with "backport:accepted", otherwise delete it. needs-minor-release This PR cannot be merged until the next minor release release-notes Should be mentioned in the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants