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.

Sign up for GitHub

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

Jump to bottom

Wunused: Check if symbol exists before isValidMemberDef check #17316

Merged
merged 1 commit into from
Apr 25, 2023
Merged

Wunused: Check if symbol exists before isValidMemberDef check #17316

merged 1 commit into from
Apr 25, 2023

Conversation

KacperFKorban
Copy link
Member

closes lampepfl#17309

@KacperFKorban
Copy link
Member Author

The fix is pretty obvious based on the stack trace.
I don't have a minimization yet though. The problem seems to be with this line (which is a macro expansion from a different library):
https://github.com/lichess-org/lila-ws/blob/764b174d4e041daea8043b34229ee175ccdcbc09/src/main/scala/evalCache/EvalCacheBsonHandlers.scala#L91

@KacperFKorban KacperFKorban marked this pull request as ready for review April 20, 2023 12:01
@jchyb
Copy link
Contributor

jchyb commented Apr 20, 2023 •
edited
Loading

Minimizing that is going to be tough (I tried), I'll try quickly releasing an testing locally, hopefully that is enough (the fix looks good in my opinion)

jchyb
jchyb approved these changes Apr 20, 2023
View reviewed changes
Copy link
Contributor

@jchyb jchyb left a comment

Choose a reason for hiding this comment

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

After manually testing, lila-ws does compile successfully now

@jchyb jchyb added the backport:nominated If we agree to backport this PR, replace this tag with "backport:accepted", otherwise delete it. label Apr 20, 2023
@Kordyjan
Copy link
Contributor

While I really want this fix to be out in RC5 as soon as possible, I think we should wait for a clear reproduction, to see where exactly we have made a mistake.

This was referenced Apr 25, 2023
Closed
Closed
@Kordyjan Kordyjan merged commit c9ca7ce into scala:main Apr 25, 2023
@Kordyjan Kordyjan deleted the fix-i17309 branch April 25, 2023 16:27
@Kordyjan
Copy link
Contributor

This is a symptom of another bug, that will be investigated in #17346. For now, I think we should backport this PR and do not wait with RC5.

@Kordyjan Kordyjan added backport:accepted This PR needs to be backported, once it's been backported replace this tag by "backport:done" and removed backport:nominated If we agree to backport this PR, replace this tag with "backport:accepted", otherwise delete it. labels Apr 26, 2023
@Kordyjan Kordyjan mentioned this pull request Apr 26, 2023
Merged
Kordyjan added a commit that referenced this pull request Apr 26, 2023
@Kordyjan
Backport "Wunused: Check if symbol exists before isValidMemberDef che…
14237d3 
…ck" (#17354)

Backports #17316
@Kordyjan Kordyjan added backport:done This PR was successfully backported. and removed backport:accepted This PR needs to be backported, once it's been backported replace this tag by "backport:done" labels May 15, 2023
@Kordyjan Kordyjan added this to the 3.3.0 milestone Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dwijnand dwijnand dwijnand left review comments

@jchyb jchyb jchyb approved these changes

Assignees
No one assigned
Labels
backport:done This PR was successfully backported.
Projects
None yet
Milestone
3.3.0
Development

Successfully merging this pull request may close these issues.

3.3.0-RC4 -Wunused:all compiler exception
4 participants
@KacperFKorban @jchyb @Kordyjan @dwijnand