-
Notifications
You must be signed in to change notification settings - Fork 63
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
Clean up AttributeUtils #1470
Clean up AttributeUtils #1470
Conversation
b6b619d
to
88125f7
Compare
I understand where this is coming from, but maybe we should wait to move these classes. The AST nodes themselves don't live in the I suggest we should have a ticket about planned package restructurings, and do everything in bulk as a planned effort when that achieves a critical mass. |
OK, we could add it as an item to #1276. It looks like the tests do not pass anyway, I will probably only get to fix them end of next week. |
This is cool, but I'd prefer to defer merging this into |
f84644d
to
0042b99
Compare
org.lflang.diagram/src/org/lflang/diagram/synthesis/LinguaFrancaSynthesis.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Clément Fournier <[email protected]>
I force pushed a more minimalist version today, that only cleans up the attributes but avoids additional refactoring. |
While trying to use the Attribute syntax I found that the
AttributeUtils
could use some cleanups. In particular, this change removes the grammar ruleAttrParmValue
and instead simply usesLiteral
. Since some of the type checking done required new functions for checking literals, I also decided to move all functions regarding expressions to a new classExpressionUtils
. And while doing so I also realized that AstUtils, ExpressionUtils, and AttributeUtils woulb probably better located in theorg.lflang.ast
subpackage. I realize that this last change might be causing conflicts with other large outstanding PRs. Feel free to revert the last commit if it seems too intrusive.