Skip to content

Commit

Permalink
Change chiselCloneType and IO(...) to return parameterized type
Browse files Browse the repository at this point in the history
  • Loading branch information
jackkoenig committed Oct 4, 2017
1 parent 041c962 commit edd11b0
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 7 deletions.
2 changes: 1 addition & 1 deletion chiselFrontend/src/main/scala/chisel3/core/Module.scala
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ abstract class BaseModule extends HasId {
* TODO(twigg): Specifically walk the Data definition to call out which nodes
* are problematic.
*/
protected def IO[T<:Data](iodef: T): iodef.type = {
protected def IO[T<:Data](iodef: T): T = {

This comment has been minimized.

Copy link
@sdtwigg

sdtwigg Oct 4, 2017

Contributor

IO is actually returning iodef so iodef.type was more precise here.

This comment has been minimized.

Copy link
@jackkoenig

jackkoenig Oct 4, 2017

Author Contributor

What additional value does that give? Is it standard practice for Scala functions that return their argument to return the type of that object or do people usually use a type parameter?

This comment has been minimized.

Copy link
@jackkoenig

jackkoenig Oct 4, 2017

Author Contributor

For the record this change doesn't seem to have any affect on the actual issue at hand.

A quick grep though of functions that return something.type gives that it's basically just cloneType, IO, and for some reason Bits.pad.

I'm just trying to better understand when we should be using type parameters vs. returning the type of an object.

This comment has been minimized.

Copy link
@sdtwigg

sdtwigg Oct 5, 2017

Contributor

In this case, it just makes it explicit (both to the user and to the type system) that precisely iodef is being returned.

This comment has been minimized.

Copy link
@jackkoenig

jackkoenig Oct 5, 2017

Author Contributor

I see, that makes sense

require(!_closed, "Can't add more ports after module close")
requireIsChiselType(iodef, "io type")

Expand Down
6 changes: 3 additions & 3 deletions src/main/scala/chisel3/compatibility.scala
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,10 @@ package object Chisel { // scalastyle:ignore package.object.name
}
}
}
implicit class cloneTypeable[T<:Data](val target: T) extends AnyVal {
implicit class cloneTypeable[T <: Data](val target: T) extends AnyVal {
import chisel3.core.DataMirror
def chiselCloneType: target.type = {
DataMirror.internal.chiselTypeClone(target).asInstanceOf[target.type]
def chiselCloneType: T = {
DataMirror.internal.chiselTypeClone(target).asInstanceOf[T]
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/main/scala/chisel3/package.scala
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,10 @@ package object chisel3 { // scalastyle:ignore package.object.name
}
}

implicit class cloneTypeable[T<:Data](val target: T) extends AnyVal {
implicit class cloneTypeable[T <: Data](val target: T) extends AnyVal {
@deprecated("chiselCloneType is deprecated, use chiselTypeOf(...) to get the Chisel Type of a hardware object", "chisel3")
def chiselCloneType: target.type = {
target.cloneTypeFull.asInstanceOf[target.type]
def chiselCloneType: T = {
target.cloneTypeFull.asInstanceOf[T]
}
}

Expand Down

0 comments on commit edd11b0

Please sign in to comment.