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

ordinal: drop unchecked with sealed abstract skips #13414

Closed
wants to merge 4 commits into from

Conversation

dwijnand
Copy link
Member

@dwijnand dwijnand commented Aug 29, 2021

In the ordinal method, drop the use of @unchecked by skipping children
that are sealed traits or sealed abstract classes and don't have any
anonymous subclasses.

The @unchecked I added in ee5a82f. In a
PR to do specifically with reachability I started seeing similar warnings in
ordinal (this was before adding @unchecked was merged) so I'm hoping
with this patch to avoid the false positives properly.

@dwijnand
Copy link
Member Author

dwijnand commented Sep 9, 2021

This is failing because the "tricksier" variation in the second half of tests/patmat/t6146.scala is now emitting an unreachable warning.

But first: I found that the definition order of the T1/T2 traits, the O1/O2 objects and the external child Shorty changed whether or not the unreachable warning was emitted by ordinal. That made me believe that the definition order affected which were the sealed children or possibly just affected the ordinal match emission. Turns out it's the logic of suppressing reachability warnings until we hit a case that matches that was affecting it: Shorty never matches, but if case _: Shorty => comes first (which happens when it's declared before the traits) then it's ignored. So I'm addressing that first in #13485.

In that "emit deferred reachability warnings" PR, in turn, I discovered that my fix for unboxed/boxed primitives (#13290) wasn't right, the unreachable ones were just not getting reported.

So now I need to fix Shorty not being considered a valid case for O1.O2.Format and then the match won't have any unreachable cases.

In the `ordinal` method, drop the use of `@unchecked` by skipping
children that are sealed traits or sealed abstract classes and don't
have any anonymous subclasses.

The `@unchecked` I added in ee5a82f.
In a PR to specifically with reachability I started similar warnings in
`ordinal` (this was before adding `@unchecked` was merged) so I'm hoping
with this patch to avoid the false positives properly.
@dwijnand
Copy link
Member Author

Fix that, and something else breaks. Taking a break from this.

@dwijnand dwijnand closed this Sep 22, 2021
@dwijnand dwijnand deleted the check-ordinal-match branch September 22, 2021 17:41
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.

None yet

1 participant