Skip to content

Conversation

@som-snytt
Copy link
Contributor

If a pattern definition binds no variables, it is
probably a mistake if it is marked implicit,
because it introduces no implicit values,
or if it is a template statement, because
it accidentally introduces a template member.

Fixes scala/bug#11618

@scala-jenkins scala-jenkins added this to the 2.13.3 milestone Feb 7, 2020
@som-snytt
Copy link
Contributor Author

som-snytt commented Feb 7, 2020

The other possible feature is to elide the temporary, which is unused. That also sounds like a good first issue.

The flags tests in Typers deserve more scrutiny; it may not matter, since finding the attachment is definitive.

Github doesn't suggest a reviewer, knowing that no one will want a piece of this.

@som-snytt som-snytt force-pushed the issue/11618 branch 2 times, most recently from 831338b to ea0c2d3 Compare February 7, 2020 05:26
Copy link
Member

@lrytz lrytz left a comment

Choose a reason for hiding this comment

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

This is very helpful! I agree that mentioning locally in the warning is a good idea.

@lrytz lrytz modified the milestones: 2.13.3, 2.13.2 Feb 12, 2020
If a pattern definition binds no variables, it is
probably a mistake if it is marked implicit,
because it introduces no implicit values,
or if it is a template statement, because
it accidentally introduces a template member.

Also since the Scouts are selling cookies this
week, I did some Girl Scouting and tweaked doc
for partest and improved some tests, including
a test that fails under Java 9 because it
uses underscore.
@som-snytt
Copy link
Contributor Author

The latest includes locally advice, and avoids the message in REPL. I wish the current run or compiler knew it was working on REPL's behalf. Also test improvements.

@lrytz
Copy link
Member

lrytz commented Feb 14, 2020

/rebuild (to check if jenkins is back)

@lrytz
Copy link
Member

lrytz commented Feb 14, 2020

Bonus follow-up could be to warn about spurious tuple fields resulting from pattern deconstruction

$> cat A.scala
case class C(x: Int, y: Int)

class A {
  val C(a, b) = C(1, 2)
  val (c, d) = (1, 2)
}
$> cfr-decompiler A.class
public class A {
    private final /* synthetic */ Tuple2 x$1;
    private final int a;
    private final int b;
    private final /* synthetic */ Tuple2 x$2;
    private final int c;
    private final int d;

    public int a() {
        return this.a;
    }

    public int b() {
        return this.b;
    }

    public int c() {
        return this.c;
    }

    public int d() {
        return this.d;
    }

    public A() {
        C c = new C(1, 2);
        if (c == null) {
            throw new MatchError((Object)c);
        }
        int a = c.x();
        int b = c.y();
        Tuple2.mcII.sp sp2 = new Tuple2.mcII.sp(a, b);
        this.x$1 = sp2;
        this.a = this.x$1._1$mcI$sp();
        this.b = this.x$1._2$mcI$sp();
        Tuple2.mcII.sp sp3 = new Tuple2.mcII.sp(1, 2);
        if (sp3 == null) {
            throw new MatchError((Object)sp3);
        }
        int c2 = sp3._1$mcI$sp();
        int d = sp3._2$mcI$sp();
        Tuple2.mcII.sp sp4 = new Tuple2.mcII.sp(c2, d);
        this.x$2 = sp4;
        this.c = this.x$2._1$mcI$sp();
        this.d = this.x$2._2$mcI$sp();
    }
}

@lrytz lrytz merged commit ca2573b into scala:2.13.x Feb 14, 2020
@som-snytt som-snytt deleted the issue/11618 branch February 14, 2020 09:31
@som-snytt
Copy link
Contributor Author

Thanks. Scala progresses because of brave individuals who merge stuff.

@som-snytt
Copy link
Contributor Author

Doti will have a warning about unindented underscores.

@SethTisue
Copy link
Member

SethTisue commented Apr 28, 2020

@som-snytt how come I don't get the warning in the first case?

scala 2.13.2> object X { val _ = 42 }
object X

scala 2.13.2> :paste -raw
// Entering paste mode (ctrl-D to finish)

object X { val _ = 42 }

// Exiting paste mode, now interpreting.

object X { val _ = 42 }
               ^
<pastie>:1: warning: Pattern definition introduces Unit-valued member of X; consider wrapping it in `locally { ... }`.

@som-snytt
Copy link
Contributor Author

I can't believe no one laughed at "unindented underscores." I was sick that week, answering the question what was I doing on Valentine's, a couple of days later I was two solid days and nights laid out in bed, for some reason not permanently.

@SethTisue guessing scala/bug#11957

@cb372
Copy link
Contributor

cb372 commented May 4, 2020

This change just helped me find and delete a line of dead code. Thanks @som-snytt!

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.

Warn if implicit underscore is no-op

6 participants