bsloane1650 commented on a change in pull request #259: Incremental progress on
schema compilation space/speed issue.
URL: https://github.com/apache/incubator-daffodil/pull/259#discussion_r301873121
##########
File path:
daffodil-core/src/main/scala/org/apache/daffodil/dpath/Expression.scala
##########
@@ -943,155 +950,148 @@ sealed abstract class StepExpression(val step: String,
val pred: Option[Predicat
NodeInfo.ArrayIndex
}
+}
+
+sealed abstract class DownStepExpression(s: String, predArg:
Option[PredicateExpression])
+ extends StepExpression(s, predArg) {
+
override lazy val inherentType: NodeInfo.Kind = {
if (!isLastStep) NodeInfo.Complex
else {
- if (stepElement.optPrimType.isDefined) {
+ if (stepElements.head.optPrimType.isDefined) {
Review comment:
Are we still able to make the conservative argument? Daffodil already has an
established behavior, and that behavior is required by the spec (at least for
implementations that choice to implement DPath). This sounds to me like we will
be deliberately introducing a regression, and it is not clear to me we have a
compelling reason to do so. Further, the broken behavior is just weird from a
user standpoint. You can define a valid type T, and use it at location A xor
location B. That type of restriction does not strike me as reasonable.
I see a few ways around this problem without introducing a regression:
A) Detect when such a situation would arise and specialize the
expression/term/type/etc as many times as needed. Eg. If we have an element
<bar> that references { ../foo } at 3 locations where foo is a string, and 5
where for is an int, then we compile 2 instances of <bar>. I thought this was
the idea behind the memoizedApply function I saw earlier. Further, I suspect we
will find this mechanism would be useful as a way of avoiding other tricky
cases arising from de-duplication. Having said that, as I noted in my comment
on memoizedApply, we would need to be careful to avoid overusing this mechanism.
B) Detect when such a situation would occur and add a runtime cast that can
dispatch on the type (or on a flag set in an ERD object). This wouldn't add any
overhead in the common case since we only need to include such a node when the
types do not match.
----------------------------------------------------------------
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]
With regards,
Apache Git Services
