stevedlawrence commented on a change in pull request #44:
URL: https://github.com/apache/daffodil-site/pull/44#discussion_r614389828
##########
File path: site/dev/design-notes/runtime2-todos.adoc
##########
@@ -38,108 +38,42 @@ in order to avoid duplication.
=== Error struct instead of error message
-To make internationalized error messages
-easier to construct when an error happens,
-we should return an error struct with some fields
-nstead of an entire error message string.
-It is easier to interpolate values into messages
-in the same function which also prints the messages.
-We still would check for errors
-by doing a null pointer check,
-although we might consider moving that check
-from parser/unparser functions to their callers
-to skip over all remaining function calls:
-
-[source,c]
-----
- unparse_be_float(instance->be_float[0], ustate);
- if (ustate->error) return;
- unparse_be_float(instance->be_float[1], ustate);
- if (ustate->error) return;
- ...
-----
-
-=== Validation errors
-
-We should handle three types of errors differently:
-runtime schema definition errors,
-parser/unparser errors,
-and validation errors.
-Schema definition errors should abort parsing immediately.
-Parser errors may need to allow backtracking in future.
-Validation errors should be gathered up
-without stopping parsing or unparsing.
-We should be able to successfully parse data
-that is "well formed"
-even though it has invalid values,
-report the invalid values,
-and allow users to analyze the data.
-We probably should gather up validation errors
-in a separate PState/UState member field
-pointing to a validation struct with some fields.
+This change is almost complete.
Review comment:
Any reason why the indentation is so short? Looks like it's only like 50
characters or something?
##########
File path: site/dev/design-notes/runtime2-todos.adoc
##########
@@ -38,108 +38,42 @@ in order to avoid duplication.
=== Error struct instead of error message
-To make internationalized error messages
-easier to construct when an error happens,
-we should return an error struct with some fields
-nstead of an entire error message string.
-It is easier to interpolate values into messages
-in the same function which also prints the messages.
-We still would check for errors
-by doing a null pointer check,
-although we might consider moving that check
-from parser/unparser functions to their callers
-to skip over all remaining function calls:
-
-[source,c]
-----
- unparse_be_float(instance->be_float[0], ustate);
- if (ustate->error) return;
- unparse_be_float(instance->be_float[1], ustate);
- if (ustate->error) return;
- ...
-----
-
-=== Validation errors
-
-We should handle three types of errors differently:
-runtime schema definition errors,
-parser/unparser errors,
-and validation errors.
-Schema definition errors should abort parsing immediately.
-Parser errors may need to allow backtracking in future.
-Validation errors should be gathered up
-without stopping parsing or unparsing.
-We should be able to successfully parse data
-that is "well formed"
-even though it has invalid values,
-report the invalid values,
-and allow users to analyze the data.
-We probably should gather up validation errors
-in a separate PState/UState member field
-pointing to a validation struct with some fields.
+This change is almost complete.
+We have replaced error message strings
+with error structs everywhere now.
+However, we should support passing
+a schema location and/or data position
+through the error struct as well as
+an error code, an integer, and a string.
+We may need to store schema locations
+(filenames and line numbers) in ERD objects
+and pass an ERD in the error struct.
+
+=== Different types of errors
+
+When runtime2 grows larger,
+we may need to distinguish & handle
+more types of errors.
+Right now we handle only
+fatal errors and validation errors.
+Fatal errors stop the program immediately.
+Validation errors are collected in an array
+and printed after parsing or unparsing.
+Later we may need to handle
+parser/unparser warnings and errors
+and allow backtracking after these errors.
+
+=== Javadoc-like tool for C code
+
+We should consider adopting
+one of the javadoc-like tools for C code
+and structuring our comments that way.
=== DSOM "fixed" getter
-We need to add DSOM support for the "fixed" attribute
-so runtimes don't have to know about the underlying XML.
-DSOM abstracts the underying XML stuff away
-so we can update the DSOM
-if we ever change the XML stuff
-and all runtimes get schema info the same way.
-
-To give runtimes access to the "fixed" attribute,
-we want to add new members to the DSOM
-to extract the "fixed" value from the schema.
-We would do it very similar to the "default" attribute
-with code like this in ElementDeclMixin.scala:
-
-[source,scala]
-----
- final lazy val fixedAttr = xml.attribute("fixed")
-
- final def hasFixedValue: Boolean = fixedAttr.isDefined
-
- final lazy val fixedValueAsString = {
- ...
- }
-----
-
-We also would convert the string value
-to a value with the correct primitive type
-with code like this in ElementBase.scala:
-
-[source,scala]
-----
- final lazy val fixedValue = {
- ...
- }
-----
-
-Note: If we change runtime1 to validate "fixed" values,
-then we can close
https://issues.apache.org/jira/browse/DAFFODIL-117[DAFFODIL-117].
-
-=== DRY for duplicate code
-
-Refactor duplicate code in
-BinaryBooleanCodeGenerator.scala,
-BinaryFloatCodeGenerator.scala,
-and BinaryIntegerKnownLengthCodeGenerator.scala
-into common code in one place.
-
-=== Count of parserStatements/unparserStatements
-
-In CodeGeneratorState.scala,
-current code checks count of only parserStatements.
-Code should check count of both
-parserStatements and unparserStatements:
-
-[source,scala]
-----
- val hasParserStatements = structs.top.parserStatements.nonEmpty
- val hasUnparserStatements = structs.top.unparserStatements.nonEmpty
- if (hasParserStatements) { ... } else { ... }
- if (hasUnparserStatements) { ... } else { ... }
-----
+Note: If we change runtime1 to validate "fixed" values
+like runtime2 does, then
+we can close https://issues.apache.org/jira/browse/DAFFODIL-117[DAFFODIL-117].
Review comment:
We have a special macro for linking to jira bugs:
```
{% jira 117 %}
```
Thgouh, maybe the syntax is different for adoc and might not work?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
