-
Notifications
You must be signed in to change notification settings - Fork 9
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
Reworking attributes to match Ctx #474
base: main
Are you sure you want to change the base?
Conversation
@rachitnigam Currently attribute types are specified in the |
I don't have strong opinions on where they should live! The |
Yup, definitely agree with not including |
No, the |
Macro-ized a bunch of stuff but I'm holding off on adding port attributes for another pass. |
@@ -57,7 +58,7 @@ pub struct Component { | |||
|
|||
// ============== Component signature =============== | |||
/// Attributes of the component | |||
pub attrs: ast::Attributes, | |||
pub attrs: utils::comp_attrs::Attrs, |
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.
Any reason to expose the comp_attrs
submodule?
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.
Technically each set of Attrs
comes with three types: Bool, Num
, and Attrs
for boolean and numeric flags as well as a type alias for the struct with these two types of flags.
We could publicize the underlying types instead, but that would then look something like CompAttrs, CompBool, CompNum
and PortAttrs, PortBool, PortNum
(for port attributes). Matter of preference honestly, I don't really mind either way.
This reworks attributes to use a system like the
Ctx
trait for attribute types. This would allow us to use a singleget
call with any type of attribute. This system currently has a few issues:TheAttr
struct is currently used to parse attributes, but is pretty useless otherwise.Ideally, we would like to be able to give different sets of attributes for different attribute contexts (ports vs components).