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

Name-based pattern match CCE on Tuple1 #7897

Closed
scabug opened this issue Oct 6, 2013 · 7 comments
Closed

Name-based pattern match CCE on Tuple1 #7897

scabug opened this issue Oct 6, 2013 · 7 comments
Assignees
Labels
Milestone

Comments

@scabug
Copy link

@scabug scabug commented Oct 6, 2013

The name-based pattern matcher doesn't like a Product1 extractor.

The extracted value is taken as v instead of v._1.

Here is the test added to test/files/run/name-based-patmat.scala:

package p0 { 
  class Single(val x: Any) extends AnyRef with Product1[String] {
    private def s = "" + x
    override def canEqual(x: Any) = this eq x.asInstanceOf[AnyRef]
    def isEmpty = false
    def get = this
    def _1 = s + " only"

    override def toString = s"Single(${_1})"
  }

  object Single {
    def unapply(x: Any): Single = new Single(x)
  }
}

This throws CCE:

    "catdog" match {
      case p0.Single(x) => println(s"`$x` has ${x.length} chars")
      case x            => println("fail: " + x)
    }

FWIW, I was working from this spec (i.e. commit comment):
"If it does not contain _1, then it is a single value
extractor analogous like Option[T]."

@scabug
Copy link
Author

@scabug scabug commented Oct 6, 2013

Imported From: https://issues.scala-lang.org/browse/SI-7897?orig=1
Reporter: @som-snytt
Affected Versions: 2.11.0-M6
See #7850

@scabug
Copy link
Author

@scabug scabug commented Oct 6, 2013

@paulp said:
This does the right thing in a hopefully imminent wip.

@scabug
Copy link
Author

@scabug scabug commented Oct 6, 2013

@som-snytt said:
"Wip it good." Is that an old one?

I have a spec question. Boolean match is achieved by unapply returning boolean. What about get returning boolean? It's the same use case. Right now, you must supply a pattern arg. isEmpty means you don't match.

The additional use case is that in a macro, it's more symmetrical or regular to generate the return of get in one place. I already worked around #7897 in the obvious way, get: String, so this was the next target.

  class Nada(s: String) extends AnyRef {
    def isEmpty = s != "null"
    def get: Boolean = !isEmpty

    override def toString = "Nada"
  }

  object Nada {
    def unapply(x: Any): Nada = new Nada(String.valueOf(x))
  }

Trying to eliminate the bad:

    null match {
      case p0.Nada(/*bad*/) => println(s"matched Nada")
      case _         => println("no matchada")
    }

@scabug
Copy link
Author

@scabug scabug commented Oct 9, 2013

@retronym said:
Possibly the same issue as #7850

@scabug
Copy link
Author

@scabug scabug commented Nov 22, 2013

@paulp said:
If it's a boolean extractor, get should actually return Unit. That doesn't work, but it should. Having get return Boolean seems undesirable to me because there's no way to be sure which of isEmpty and get will be honored if they are in disagreement.

@scabug
Copy link
Author

@scabug scabug commented Dec 15, 2013

@paulp said:
From some experience with it I can say the way it should be specified that isEmpty is always checked first and if it's false it will never match. I forget what makes me say that or what alternative I'm positioning it against, but I know there was something.

Also, it's important that get returning boolean is not the same use case. If get returns boolean the unapply encodes three possible outcomes. If the unapply returns boolean it encodes two possible outcomes.

If I were designing my own pattern matcher there would be a first class Product and all extractors would return some efficiently encoded variation of Option[Product[Arity]], but as it is there's not going to be a way to abstract uniformly over unapply return types.

@scabug
Copy link
Author

@scabug scabug commented Dec 16, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants