-
Notifications
You must be signed in to change notification settings - Fork 51
Revise modeling and code generation for default string size #913
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
Changes from all commits
828539f
1548baf
326512f
503fbd4
85c2f79
229dd82
605eb1e
1c811db
5b3a8ef
545babc
43d35f3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -129,7 +129,7 @@ object ConstantCppWriter extends CppWriterUtils { | |
| val hppLines = { | ||
| val defLine = line(s"$name = $value") | ||
| List( | ||
| line(s"enum FppConstant_$name {"), | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I removed the enum type here. It was causing the compiler to complain about comparing enum values with different types.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Update: I had to add static casts to remove the warnings on gcc. So removing the enum type isn't necessary. However, I also think that the enum type doesn't really add anything there, so we can remove it. |
||
| line(s"enum {"), | ||
| indentIn(defLine), | ||
| line("};") | ||
| ) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,8 +10,6 @@ case class DictionaryJsonEncoderState( | |
| a: Analysis, | ||
| /** The output directory */ | ||
| dir: String = ".", | ||
| /** The default string size */ | ||
| defaultStringSize: Int = DictionaryJsonEncoderState.defaultDefaultStringSize, | ||
| /** The default bool size */ | ||
| boolSize: Int = DictionaryJsonEncoderState.boolSize, | ||
| /** The Dictionary metadata */ | ||
|
|
@@ -29,17 +27,24 @@ case class DictionaryJsonEncoderState( | |
| } | ||
| } | ||
|
|
||
| def getFwDefaultStringSize: BigInt = { | ||
| a.frameworkDefinitions.constants.fwFixedLengthStringSize match { | ||
| case Some(s: Symbol.Constant) => a.valueMap(s.getNodeId) match { | ||
| case Value.Integer(value) => value | ||
| case _ => throw InternalError("expected integer value") | ||
| } | ||
| case None => throw InternalError("FW_FIXED_LENGTH_STRING_SIZE constant not defined") | ||
| } | ||
| } | ||
|
|
||
| } | ||
|
|
||
| case object DictionaryJsonEncoderState { | ||
|
|
||
| /** The default string size */ | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| val defaultDefaultStringSize = 80 | ||
|
|
||
| /** The default bool size */ | ||
| val boolSize = 8 | ||
|
|
||
| /** Gets the generated JSON file name for a topology definition */ | ||
| def getTopologyFileName(baseName: String): String = s"${baseName}TopologyDictionary.json" | ||
|
|
||
| } | ||
| } | ||
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.
@Kronos3 I decided to simplify the code generation here. Testing the old code was difficult, and IMO it is over-optimized. If you are hitting this case (trying to write a large array of strings into a much smaller string) then your system is mistuned anyway -- so I don't think we should try to optimize it.