8000 Fix make macro by pablf · Pull Request #9147 · zio/zio · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix make macro #9147

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

Closed
wants to merge 1 commit into from
Closed

Fix make macro #9147

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion core/shared/src/main/scala-2/zio/internal/macros/LayerMacroUtils.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ private[zio] trait LayerMacroUtils {

intersectionTypes
.map(_.dealias)
.filterNot(t => typeOf[Any] <:< t)
.filterNot(t => typeOf[Object] <:< t || typeOf[Any] <:< t)
.distinct
}

Expand Down
56 changes: 22 additions & 34 deletions core/shared/src/main/scala/zio/internal/macros/Graph.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,20 @@ final case class Graph[Key, A](
envKeys: List[Key]
) {

// Map assigning to each type the times that it must be built
// -1 designs a `Key` from the environment
private var neededKeys: Map[Key, Int] = Map.empty
// List with the types that must be built from given nodes, contained the number of times needed
private var neededKeys: List[Key] = List.empty
// Dependencies to pass to next iteration of buildComplete
private var dependencies: List[Key] = Nil
private var envDependencies: List[Key] = Nil

private def get(k: Key, m: List[Key]): Int =
m.count(key => areEquals(key, k))
Comment on lines +18 to +19
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps we should add an exists method as well here just to avoid traversing the whole list in L84 below?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes and also create another custom method because we only need to differentiate between 0, 1 and >1 appearances in the other cases


private def areEquals(k1: Key, k2: Key): Boolean =
keyEquals(k1, k2)
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't this needs to be keyEquals(k1, k2) || keyEquals(k2, k1)?

Copy link
Member Author

Choose a reason for hiding this comment

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

It shouldn't be needed but the order is important, maybe that was an issue if you tried this version also

Copy link
Member

Choose a reason for hiding this comment

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

Should probably have a test here to catch the "wrong" implementation.


private var usedEnvKeys: Set[Key] = Set.empty
def usedRemainders(): Set[A] = usedEnvKeys.map(environment(_)).map(_.value)
private var topLevel = true

def buildNodes(
outputs: List[Key],
Expand All @@ -42,13 +46,13 @@ final case class Graph[Key, A](

private def constructDeps(): List[Key] =
if (dependencies.isEmpty) dependencies
else distinctKeys(dependencies) ++ distinctKeys(envDependencies)
else distinctKeys(dependencies ++ envDependencies)

/**
* Restarts variables for next iteration of buildComplete
*/
private def restartKeys(): Unit = {
neededKeys = Map.empty
neededKeys = List.empty
dependencies = Nil
envDependencies = Nil
}
Expand Down Expand Up @@ -76,8 +80,8 @@ final case class Graph[Key, A](
envOutputs.map(addEnv(_))

forEach(normalOutputs) { output =>
if (created.exists(k => keyEquals(k, output))) {
if (neededKeys.get(output).isEmpty) throw new Throwable("This can't happen.")
if (created.exists(k => areEquals(k, output))) {
if (get(output, neededKeys) == 0) throw new Throwable("This can't happen.")
Right(())
} else {
for {
Expand All @@ -87,7 +91,7 @@ final case class Graph[Key, A](
if (topLevel || parent.isEmpty) Some(GraphError.MissingTopLevelDependency(output))
else Some(GraphError.missingTransitiveDependency(parent.get, output))
)
nodeOutputs = node.outputs.map(out => findKey(out, outputs))
nodeOutputs = node.outputs
_ <- Right(nodeOutputs.map(addKey(_)))
_ <- Right { created = nodeOutputs ++ created }
_ <- parent match {
Expand All @@ -100,38 +104,22 @@ final case class Graph[Key, A](
}.map(_ => ())
}

private def findKey(key: Key, keys: List[Key]): Key =
keys.find(k => keyEquals(key, k)).getOrElse(key)

private def addEnv(key: Key): Unit = {
val keyFromEnv = envKeys
.find(env => keyEquals(key, env) || keyEquals(env, key))
.find(env => areEquals(env, key))
.getOrElse(throw new Throwable("This shouldn't happen"))
usedEnvKeys = usedEnvKeys + keyFromEnv
neededKeys.get(key) match {
case Some(_) => ()
case None => neededKeys = neededKeys + (key -> -1)
}
}

private def addKey(key: Key): Unit =
neededKeys.get(key) match {
case Some(-1) => ()
case Some(n) => neededKeys = neededKeys + (key -> (n + 1))
case None => neededKeys = neededKeys + (key -> 1)
}
if (!isEnv(key)) neededKeys = key :: neededKeys

private def buildNode(node: Node[Key, A]): Either[::[GraphError[Key, A]], LayerTree[A]] =
build(node.inputs).map { case (deps, allEnv) =>
if (allEnv) LayerTree.succeed(node.value)
else deps >>> LayerTree.succeed(node.value)
}

private def tapKey(key: Key): Option[Int] =
neededKeys.get(key) match {
case Some(n) => Some(n)
case None => neededKeys.find(pair => keyEquals(pair._1, key) || keyEquals(key, pair._1)).map(_._2)
}

/**
* Builds a layer containing only types that appears once. Types appearing
* more than once are replaced with ZLayer.environment[_] and left for the
Expand All @@ -143,11 +131,11 @@ final case class Graph[Key, A](
envDependencies = output :: envDependencies
Right((LayerTree.succeed(environment(output).value), true))
} else
tapKey(output) match {
case None => throw new Throwable(s"This can't happen")
case Some(1) =>
get(output, neededKeys) match {
case 0 => throw new Throwable(s"This can't happen")
case 1 =>
getNodeWithOutput[GraphError[Key, A]](output).flatMap(node => buildNode(node).map(tree => (tree, false)))
case Some(n) => {
case _ => {
dependencies = output :: dependencies
Right((LayerTree.succeed(environment(output).value), true))
}
Expand All @@ -167,10 +155,10 @@ final case class Graph[Key, A](
.toRight(error.map(e => ::(e, Nil)).getOrElse(throw new Throwable("This can't happen")))

private def findNodeWithOutput(output: Key): Option[Node[Key, A]] =
nodes.find(_.outputs.exists(keyEquals(_, output)))
nodes.find(_.outputs.exists(areEquals(_, output)))

private def isEnv(key: Key): Boolean =
envKeys.exists(env => keyEquals(key, env) || keyEquals(env, key))
envKeys.exists(env => areEquals(key, env))

private def assertNonCircularDependency(
node: Node[Key, A],
Expand Down
4 changes: 2 additions & 2 deletions core/shared/src/main/scala/zio/internal/macros/RenderedGraph.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ private[macros] object RenderedGraph {
override def >>>(that: RenderedGraph): RenderedGraph =
that match {
case Value(string, children) => Value(string, self +: children)
case Row(_) => throw new Error("NOT LIKE THIS")
case Row(values) => Row(self +: values)
}

override def render(depth: Int): String = {
Expand Down Expand Up @@ -72,7 +72,7 @@ private[macros] object RenderedGraph {
override def >>>(that: RenderedGraph): RenderedGraph =
that match {
case Value(string, children) => Value(string, self.values ++ children)
case Row(_) => throw new Error("NOT LIKE THIS")
case Row(values2) => Row(values ++ values2)
}

override def render(depth: Int): String =
Expand Down
Loading
0