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
add Path property type #3511
add Path property type #3511
Conversation
def apply(data: Data): PathType = { | ||
new PathType { | ||
def toTarget(): IsMember = data.toAbsoluteTarget | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to approve this as is, but we really validate at this point if the Data
can have a target. I'm mainly thinking about things like PathType(3.U)
which will result an an exception thrown later when .toAbsoluteTarget
is actually called.
|
||
/** Represent a Path type for referencing a hardware instance or member in a Property[PathType] | ||
*/ | ||
sealed abstract class PathType { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be PathType
or perhaps just Path
? It's Path
in firrtl.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
renamed to Path
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implementation LGTM, just wondering if we should be calling this Path
instead of PathType
.
Interesting. Maybe this is the real replacement to |
Contributor Checklist
docs/src
?Type of Improvement
Adds new public
PathType
class that represents a path to a hardware instance or member. AddsPathPropertyType
/PathPropertyLiteral
andPropertyType
instances for module/data/mem.Desired Merge Strategy
Release Notes
Reviewer Checklist (only modified by reviewer)
3.5.x
,3.6.x
, or5.x
depending on impact, API modification or big change:6.0
)?Enable auto-merge (squash)
, clean up the commit message, and label withPlease Merge
.Create a merge commit
.