Skip to content

WIP - name-based extractors for unapply to drop allocation of Some #47

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

Open
lbialy wants to merge 3 commits into
base: master
Choose a base branch
from
Open

WIP - name-based extractors for unapply to drop allocation of Some #47

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2b92984
this compiles but introduces usage limitation :(
lbialy Mar 10, 2019
79dd88d
Trying to get name-based extractors to work using shared, generic Any…
Apr 9, 2019
b4d5940
It werks!
Apr 9, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions shared/src/main/scala/io/estatico/newtype/NewType.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ trait NewType extends BaseNewType { self =>
}

object NewType {
class NewTypeUnapplyOps[A](val a: A) extends AnyVal {
def isEmpty: Boolean = false
def get: A = a
}
Copy link
Copy Markdown
Member

@carymrobbins carymrobbins May 31, 2019

Choose a reason for hiding this comment

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

Should probably be final class

Copy link
Copy Markdown
Member

@carymrobbins carymrobbins May 31, 2019

Choose a reason for hiding this comment

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

I also wonder if you could get away with using val get in the constructor, e.g.

final class NewTypeUnapplyOps[A](val get: A) extends AnyVal {
  def isEmpty: Boolean = false
}

trait Of[R] extends BaseNewType.Of[R] with NewType
trait Default[R] extends Of[R] with NewTypeExtras
}
Expand Down
10 changes: 5 additions & 5 deletions shared/src/main/scala/io/estatico/newtype/macros/NewTypeMacros.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,14 +185,14 @@ private[macros] class NewTypeMacros(val c: blackbox.Context)
clsDef: ClassDef, valDef: ValDef, tparamsNoVar: List[TypeDef], tparamNames: List[TypeName]
): List[Tree] = {
if (!unapply) Nil else {
// Note that our unapply method should Some since its isEmpty/get is constant.
// We are using a generic value class name-based extractor not to allocate Some
List(
if (tparamsNoVar.isEmpty) {
q"""def unapply(x: ${clsDef.name}): Some[${valDef.tpt}] =
Some(x.asInstanceOf[${valDef.tpt}])"""
q"""def unapply(x: ${clsDef.name}): io.estatico.newtype.NewType.NewTypeUnapplyOps[${valDef.tpt}] =
new io.estatico.newtype.NewType.NewTypeUnapplyOps[${valDef.tpt}](x.coerce)"""
} else {
q"""def unapply[..$tparamsNoVar](x: ${clsDef.name}[..$tparamNames]): Some[${valDef.tpt}] =
Some(x.asInstanceOf[${valDef.tpt}])"""
q"""def unapply[..$tparamsNoVar](x: ${clsDef.name}[..$tparamNames]): io.estatico.newtype.NewType.NewTypeUnapplyOps[${valDef.tpt}] =
new io.estatico.newtype.NewType.NewTypeUnapplyOps[${valDef.tpt}](x.coerce[${valDef.tpt}])"""
Copy link
Copy Markdown
Member

@carymrobbins carymrobbins May 31, 2019

Choose a reason for hiding this comment

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

Should probably avoid the use of the .coerce extension method as it's unreliable from this context (requires the user to have the Coercible ops imported). Instead, should be able to use asInstanceOf safely.

}
)
}
Expand Down
1 change: 1 addition & 0 deletions shared/src/test/scala/io/estatico/newtype/macros/NewTypeMacrosTest.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package io.estatico.newtype.macros

import io.estatico.newtype.Coercible
Copy link
Copy Markdown
Member

@carymrobbins carymrobbins May 31, 2019

Choose a reason for hiding this comment

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

I assume this import is unnecessary?

import org.scalatest.{FlatSpec, Matchers}
import io.estatico.newtype.ops._
import org.scalacheck.Arbitrary
Expand Down