Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Lint select fromUnit, orInt that incurs widening#10372

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
lrytz merged 3 commits intoscala:2.13.xfromsom-snytt:issue/12728-dubious-selections
Apr 12, 2023

Conversation

@som-snytt
Copy link
Contributor

lrytz and He-Pin reacted with heart emoji
@scala-jenkinsscala-jenkins added this to the2.13.12 milestoneApr 10, 2023
@som-snytt
Copy link
ContributorAuthor

Some current inconsistency.

scala> 42.ceilval res0: Float = 42.0scala> def i = 42def i: Intscala> i.ceil       ^       warning: Widening conversion from Int to Float is deprecated because it loses precision. Write `.toFloat` instead.val res1: Float = 42.0scala> def c = 'c'def c: Charscala> c.ceilval res2: Float = 99.0

wherec.ceil is pronounced "Cécile" and is even less likely to be intended.

With the new warning, the current warning is less correct.

@som-snytt
Copy link
ContributorAuthor

It warns in two spots that specializeArray[Unit].

The other thought is that (as with the current warning), any promotion is dubious, it doesn't matter which member is selected.

Then just needdefinitions.RichFloatClass.

@som-snyttsom-snyttforce-pushed theissue/12728-dubious-selections branch 4 times, most recently fromefd7fd9 tob84df96CompareApril 10, 2023 20:34
@som-snytt
Copy link
ContributorAuthor

The check is only at typer, which gives a pass to

case Literal(Constant(())) =>

Not sure why it was deemed bad to allowcase _: () => as the unit value ought to be a singleton. (It is notUnit with Singleton.) I guess it doesn't matter because the test is always boxed unit equality. There's no advantage tocase _: Unit.

Something became().asInstanceOf[Unit] in fields, a private lazy val after init. (That was thetest execution will be non-parallel inPartestDefaults.)

@som-snyttsom-snytt marked this pull request as ready for reviewApril 10, 2023 22:01
@som-snyttsom-snytt requested a review fromlrytzApril 10, 2023 22:02
Copy link
Member

@lrytzlrytz left a comment

Choose a reason for hiding this comment

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

LGTM, and very useful.


defcheckDubiousAdaptation(sel:Tree):Unit=if (!isPastTyper&& settings.lintNumericMethods) {
valdubious=ScalaIntegralValueClasses(qualTp.typeSymbol)&& (
sel.symbol.owner.eq(BoxedFloatClass)|| sel.symbol.owner.eq(RichFloatClass))
Copy link
Member

Choose a reason for hiding this comment

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

long.isNaN goes throughfloat2Float(long.toFloat), notdouble2Double, so this seems to cover it all.

@som-snyttsom-snyttforce-pushed theissue/12728-dubious-selections branch fromb84df96 to905c92fCompareApril 11, 2023 18:36
@som-snytt
Copy link
ContributorAuthor

Rebased, and enhancedpartest --realeasy to skip tests that run on today's JDK.

-- 1 - run/blank.scala                           [skipped on Java 19, compiling against JDK8 but must run on 11+]

It would be ideal if--realeasy did not skip in case of explicitscalac: --release:8 but that was too subtle for me.

The option also addsrun/t6240-universe-code-gen.scala which is annoying to forget to check. (I had actually fixed it in the wrong commit.)

Selected 1810 tests drawn from 1 named test categories, 2 tests modified on this branch, and other tests you might have forgotten

In case of selection failure,

sbt:root> partest junkDiscarding 1 invalid test pathsSelected 0 tests drawn from the well of despair. I see you're not in a testing mood.[info] Passed: Total 0, Failed 0, Errors 0, Passed 0

Copy link
Member

@lrytzlrytz left a comment
edited
Loading

Choose a reason for hiding this comment

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

Thank you!

@lrytzlrytz merged commitd5247e9 intoscala:2.13.xApr 12, 2023
@SethTisueSethTisue modified the milestones:2.13.12,2.13.11Apr 12, 2023
@som-snyttsom-snytt deleted the issue/12728-dubious-selections branchApril 12, 2023 16:01
@SethTisueSethTisue added the release-notesworth highlighting in next release notes labelMay 3, 2023
@SethTisueSethTisue changed the titleLint select from Unit, or Int that incurs wideningLint select fromUnit, orInt that incurs wideningJun 1, 2023
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@lrytzlrytzlrytz approved these changes

Assignees

No one assigned

Labels

release-notesworth highlighting in next release notes

Projects

None yet

Milestone

2.13.11

Development

Successfully merging this pull request may close these issues.

Lint selections from unit value

4 participants

@som-snytt@lrytz@SethTisue@scala-jenkins

[8]ページ先頭

©2009-2025 Movatter.jp