You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@daffodil.apache.org by GitBox <gi...@apache.org> on 2022/02/17 16:31:14 UTC
[GitHub] [daffodil] jadams-tresys opened a new pull request #757: Improve diagnostic for no forward progress
jadams-tresys opened a new pull request #757:
URL: https://github.com/apache/daffodil/pull/757
Improved diagnostic message now contains reference to the array and
describes how it is stuck in an infinite loop.
DAFFODIL-2576
--
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.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil] stevedlawrence commented on pull request #757: Improve diagnostic for no forward progress
Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on pull request #757:
URL: https://github.com/apache/daffodil/pull/757#issuecomment-1043365722
Oops, +1 to the changes :+1:
--
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.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil] jadams-tresys merged pull request #757: Improve diagnostic for no forward progress
Posted by GitBox <gi...@apache.org>.
jadams-tresys merged pull request #757:
URL: https://github.com/apache/daffodil/pull/757
--
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.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil] stevedlawrence commented on a change in pull request #757: Improve diagnostic for no forward progress
Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on a change in pull request #757:
URL: https://github.com/apache/daffodil/pull/757#discussion_r809318641
##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/SequenceParserBases.scala
##########
@@ -62,7 +62,7 @@ abstract class SequenceParserBase(
ais: ArrayIndexStatus): ArrayIndexStatus = {
Assert.invariant(currentPos >= priorPos)
if (currentPos == priorPos && pstate.groupPos > 1) {
- PE(pstate, "No forward progress.")
+ PE(pstate, "Error parsing array '%s': Array element parsed succesfully, but consumed no data and is stuck in an infinite loop as it is unbounded.".format(srd.groupMembers(srd.position-1)))
Review comment:
Is this part correct?
```scala
srd.groupMembers(srd.position-1))
```
Isn't `srd.position` the position of this sequence in its list of siblings, and not the position of the array that we want to print?
I'm not sure this function has the information to know which is the right child of the sequence that failed to make progress. I *think* the caller of `checkForwardProgress` has that information though. Maybe something like `parser.erd`, since `parser` in that context is the thing that actually parsed an array element but failed to make progress?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil] jadams-tresys commented on a change in pull request #757: Improve diagnostic for no forward progress
Posted by GitBox <gi...@apache.org>.
jadams-tresys commented on a change in pull request #757:
URL: https://github.com/apache/daffodil/pull/757#discussion_r809320203
##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/SequenceParserBases.scala
##########
@@ -62,7 +62,7 @@ abstract class SequenceParserBase(
ais: ArrayIndexStatus): ArrayIndexStatus = {
Assert.invariant(currentPos >= priorPos)
if (currentPos == priorPos && pstate.groupPos > 1) {
- PE(pstate, "No forward progress.")
+ PE(pstate, "Error parsing array '%s': Array element parsed succesfully, but consumed no data and is stuck in an infinite loop as it is unbounded.".format(srd.groupMembers(srd.position-1)))
Review comment:
I think you might be right. Fix incoming to use parser.erd
--
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.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil] stevedlawrence commented on a change in pull request #757: Improve diagnostic for no forward progress
Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on a change in pull request #757:
URL: https://github.com/apache/daffodil/pull/757#discussion_r809322369
##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/SequenceParserBases.scala
##########
@@ -62,7 +62,7 @@ abstract class SequenceParserBase(
ais: ArrayIndexStatus): ArrayIndexStatus = {
Assert.invariant(currentPos >= priorPos)
if (currentPos == priorPos && pstate.groupPos > 1) {
- PE(pstate, "No forward progress.")
+ PE(pstate, "Error parsing array '%s': Array element parsed succesfully, but consumed no data and is stuck in an infinite loop as it is unbounded.".format(srd.groupMembers(srd.position-1)))
Review comment:
If the above is correct (I'm still not totally sure), maybe what we really want is to pass in `parser`, and then call `parser.PE(...)`. That way the context of the PE would be on that of the array parser and not of the sequence. And with the right context, you might not even need to include the array name in the error message since it should be included in the context?
If that's true, it might be worth adding the array name to the `<tdml:error>` so that we make sure we get the expected context.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil] jadams-tresys commented on a change in pull request #757: Improve diagnostic for no forward progress
Posted by GitBox <gi...@apache.org>.
jadams-tresys commented on a change in pull request #757:
URL: https://github.com/apache/daffodil/pull/757#discussion_r809331956
##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/SequenceParserBases.scala
##########
@@ -62,7 +62,7 @@ abstract class SequenceParserBase(
ais: ArrayIndexStatus): ArrayIndexStatus = {
Assert.invariant(currentPos >= priorPos)
if (currentPos == priorPos && pstate.groupPos > 1) {
- PE(pstate, "No forward progress.")
+ PE(pstate, "Error parsing array '%s': Array element parsed succesfully, but consumed no data and is stuck in an infinite loop as it is unbounded.".format(srd.groupMembers(srd.position-1)))
Review comment:
I've pushed up changes to use the parser.erd, but that did not automatically get us the array name. Still need to call parser.erd.diagnosticDebugName
--
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.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil] stevedlawrence commented on a change in pull request #757: Improve diagnostic for no forward progress
Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on a change in pull request #757:
URL: https://github.com/apache/daffodil/pull/757#discussion_r809345893
##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/SequenceParserBases.scala
##########
@@ -62,7 +62,7 @@ abstract class SequenceParserBase(
ais: ArrayIndexStatus): ArrayIndexStatus = {
Assert.invariant(currentPos >= priorPos)
if (currentPos == priorPos && pstate.groupPos > 1) {
- PE(pstate, "No forward progress.")
+ PE(pstate, "Error parsing array '%s': Array element parsed succesfully, but consumed no data and is stuck in an infinite loop as it is unbounded.".format(srd.groupMembers(srd.position-1)))
Review comment:
Interesting, I'm not sure if that's a bug or not...
It looks like the reason still getting the sequence as the context is that `RepeatingChildParser` extends `SequenceChildParser`, which extends `CombinatorParser` and passes in `srd` as the context. So when we do `parser.PE`, the context of that parser is still that of the sequence, even though this particular parser is really all about the the child. Maybe we should change `SequenceChildParser` so that is passes in `trd` to the `CombinatorParser` instead of `srd`? I think that feels more correct to me, since I would expect the context of a `SequenceChildParser` to be the child and not the Sequence? And then you wouldn't need diagnosticDebugName?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil] jadams-tresys commented on a change in pull request #757: Improve diagnostic for no forward progress
Posted by GitBox <gi...@apache.org>.
jadams-tresys commented on a change in pull request #757:
URL: https://github.com/apache/daffodil/pull/757#discussion_r809372126
##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/SequenceParserBases.scala
##########
@@ -62,7 +62,7 @@ abstract class SequenceParserBase(
ais: ArrayIndexStatus): ArrayIndexStatus = {
Assert.invariant(currentPos >= priorPos)
if (currentPos == priorPos && pstate.groupPos > 1) {
- PE(pstate, "No forward progress.")
+ PE(pstate, "Error parsing array '%s': Array element parsed succesfully, but consumed no data and is stuck in an infinite loop as it is unbounded.".format(srd.groupMembers(srd.position-1)))
Review comment:
Passing the trd to CombinatorParser did the trick. New commit inbound
--
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.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org