Skip to content

Reworking attributes to match Ctx#474

Merged
UnsignedByte merged 11 commits intomainfrom
attributes-rework
Jan 12, 2025
Merged

Reworking attributes to match Ctx#474
UnsignedByte merged 11 commits intomainfrom
attributes-rework

Conversation

@UnsignedByte
Copy link
Collaborator

@UnsignedByte UnsignedByte commented Oct 28, 2024

This reworks attributes to use a system like the Ctx trait for attribute types. This would allow us to use a single get call with any type of attribute. This system currently has a few issues:

  • The Attr 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).

@UnsignedByte
Copy link
Collaborator Author

@rachitnigam Currently attribute types are specified in the utils directory but that doesn't seem right. I could also move them to ast instead if that seems more reasonable but they are also used in IR...

@rachitnigam
Copy link
Member

I don't have strong opinions on where they should live! The utils separation is probably because they are included by both ast and ir but we don't want to include ir in ast.

@UnsignedByte
Copy link
Collaborator Author

Yup, definitely agree with not including ir in AST. My main worry is that although the attribute traits and stuff could go into utils, the actual attributes themselves are somewhat tied to Filament and my impression of utils was that they should be "language-agnostic" compiler utilities... Maybe I will move attribs to AST.

@rachitnigam
Copy link
Member

No, the utils are utilities for the Filament compiler infrastructure. I don't think it is specifically architected to contained truly agnostic utilities.

@UnsignedByte
Copy link
Collaborator Author

Macro-ized a bunch of stuff but I'm holding off on adding port attributes for another pass.

@rachitnigam
Copy link
Member

Minor comments. Merge when resolved!

@UnsignedByte UnsignedByte enabled auto-merge (squash) January 12, 2025 23:30
@UnsignedByte UnsignedByte merged commit 7e8ed87 into main Jan 12, 2025
4 checks passed
@UnsignedByte UnsignedByte deleted the attributes-rework branch January 12, 2025 23:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants