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

Consider static and non-static methods as non-double def #19400

Merged
merged 3 commits into from
Jan 12, 2024
Merged

Consider static and non-static methods as non-double def #19400

merged 3 commits into from
Jan 12, 2024

Conversation

dwijnand
Copy link
Member

@dwijnand dwijnand commented Jan 8, 2024 •
edited
Loading

Fixes #19394
Fixes #19396
Fixes #17332

@dwijnand dwijnand force-pushed the static-and-targetName branch from 275495b to e873376 Compare January 8, 2024 18:24
@armanbilge
Copy link
Contributor

Do the JS backend changes fix #19396?

@dwijnand
Copy link
Member Author

dwijnand commented Jan 9, 2024

Yeah.

@dwijnand dwijnand requested a review from sjrd January 9, 2024 22:49
@dwijnand
Copy link
Member Author

dwijnand commented Jan 9, 2024

@sjrd Could you let me know how we should fix this? This works, but perhaps it should be fixed different or even in a phase before the backend.

@dwijnand dwijnand self-assigned this Jan 10, 2024
@dwijnand dwijnand marked this pull request as ready for review January 12, 2024 10:10
@dwijnand dwijnand removed their assignment Jan 12, 2024
@dwijnand dwijnand requested a review from sjrd January 12, 2024 10:10
@dwijnand dwijnand assigned dwijnand and unassigned sjrd Jan 12, 2024
@dwijnand
Copy link
Member Author

dwijnand commented Jan 12, 2024 •
edited
Loading

Fixes 2 other issues. Just seeing if I can get it to also fix #14710 by fixing whatever it's wrongly doing for @static val.

(Nvm, can't figure out how to fix the final "Cannot find this in scope" linking error)

@dwijnand dwijnand enabled auto-merge January 12, 2024 15:12
@sjrd
Copy link
Member

sjrd commented Jan 12, 2024

You might want to look at the tests that show up in

$ git grep '// scalajs: --skip --pending'

to see if there are any that can be enabled now.

@dwijnand dwijnand disabled auto-merge January 12, 2024 15:50
@dwijnand dwijnand force-pushed the static-and-targetName branch from 529c95a to abffedc Compare January 12, 2024 16:32
@dwijnand dwijnand enabled auto-merge January 12, 2024 16:32
@dwijnand dwijnand merged commit c50f2ff into scala:main Jan 12, 2024
16 checks passed
@armanbilge
Copy link
Contributor

Thanks, this fixed compilation in Cats Effect. Is this a potential LTS backport candidate?

Unfortunately, the compiled code is still not working at runtime in Scala.js.

@dwijnand dwijnand deleted the static-and-targetName branch January 15, 2024 11:21
@Kordyjan Kordyjan added this to the 3.4.1 milestone Feb 14, 2024
@WojciechMazur WojciechMazur mentioned this pull request Jun 27, 2024
Merged
WojciechMazur added a commit that referenced this pull request Jun 28, 2024
@WojciechMazur
Backport "Consider static and non-static methods as non-double def" t…
fdb2ced 
…o LTS (#20826)

Backports #19400 to the LTS branch.

PR submitted by the release tooling.
[skip ci]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sjrd sjrd sjrd approved these changes

Assignees

@dwijnand dwijnand

Labels
None yet
Projects
None yet
Milestone
3.4.1
4 participants
@dwijnand @armanbilge @sjrd @Kordyjan