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 2021/01/27 16:16:38 UTC
[GitHub] [incubator-daffodil] bsloane1650 commented on a change in pull request #482: Update debugger support of found delimiters/fields/diffs
bsloane1650 commented on a change in pull request #482:
URL: https://github.com/apache/incubator-daffodil/pull/482#discussion_r565429533
##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/debugger/InteractiveDebugger.scala
##########
@@ -1528,37 +1542,91 @@ class InteractiveDebugger(runner: InteractiveDebuggerRunner, eCompilers: Express
}
}
- object InfoFoundDelimiter extends DebugCommand with DebugCommandValidateZeroArgs {
+ object InfoFoundDelimiter extends DebugCommand with DebugCommandValidateZeroArgs with InfoDiffable {
val name = "foundDelimiter"
override lazy val short = "fd"
val desc = "display the current found delimiter"
val longDesc = desc
+
+ private def getDelimOpt(s: StateForDebugger): Option[String] = {
Review comment:
Since we are in runtime1, we want to use Maybe.
This is just the debugger, so Option is probably fine if you actually need the extra functionality.
##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/debugger/InteractiveDebugger.scala
##########
@@ -1528,37 +1542,91 @@ class InteractiveDebugger(runner: InteractiveDebuggerRunner, eCompilers: Express
}
}
- object InfoFoundDelimiter extends DebugCommand with DebugCommandValidateZeroArgs {
+ object InfoFoundDelimiter extends DebugCommand with DebugCommandValidateZeroArgs with InfoDiffable {
val name = "foundDelimiter"
override lazy val short = "fd"
val desc = "display the current found delimiter"
val longDesc = desc
+
+ private def getDelimOpt(s: StateForDebugger): Option[String] = {
+ if (s.delimitedParseResult.isDefined) {
+ val pr = s.delimitedParseResult.get
+ val delim = Misc.remapStringToVisibleGlyphs(pr.matchedDelimiterValue.get)
+ Some(delim)
+ } else {
+ None
+ }
+ }
+
def act(args: Seq[String], prestate: StateForDebugger, state: ParseOrUnparseState, processor: Processor): DebugState.Type = {
- state match {
- case pstate: PState => {
- if (pstate.delimitedParseResult.isDefined) {
- val pr = pstate.delimitedParseResult.get
- debugPrintln("%s:".format(name))
- debugPrintln("foundField: %s".format(Misc.remapStringToVisibleGlyphs(pr.field.get)), " ")
- debugPrintln("foundDelimiter: %s".format(Misc.remapStringToVisibleGlyphs(pr.matchedDelimiterValue.get)), " ")
- } else {
- debugPrintln("%s: nothing found".format(name))
- }
- }
- case ustate: UState => {
- // TODO
+ val delimOpt = getDelimOpt(state)
+ val str = delimOpt.getOrElse("nothing found")
+ debugPrintln("%s: %s".format(name, str))
+ DebugState.Pause
+ }
+
+ def diff(prestate: StateForDebugger, state: ParseOrUnparseState): Boolean = {
+ val delimOptPre = getDelimOpt(prestate)
+ val delimOptPost = getDelimOpt(state)
+
+ (delimOptPre, delimOptPost) match {
+ case (None, None) => false
+ case (Some(pre), Some(post)) if pre == post => false
Review comment:
These cases are the same: case (a, b) if a == b => false
##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/debugger/InteractiveDebugger.scala
##########
@@ -1567,6 +1635,15 @@ class InteractiveDebugger(runner: InteractiveDebuggerRunner, eCompilers: Express
}
DebugState.Pause
}
+
+ def diff(prestate: StateForDebugger, state: ParseOrUnparseState): Boolean = {
+ if (prestate.groupPos != state.groupPos) {
+ debugPrintln("%s: %d -> %d".format(name, prestate.groupPos, state.groupPos), " ")
Review comment:
We are doing this for bassically every diff.
Would it make more sense for the interface to instead be:
```
getValue(state: State): Option[Any]
```
Then let the trait itself implement the diff() function?
##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/debugger/InteractiveDebugger.scala
##########
@@ -1528,37 +1542,91 @@ class InteractiveDebugger(runner: InteractiveDebuggerRunner, eCompilers: Express
}
}
- object InfoFoundDelimiter extends DebugCommand with DebugCommandValidateZeroArgs {
+ object InfoFoundDelimiter extends DebugCommand with DebugCommandValidateZeroArgs with InfoDiffable {
val name = "foundDelimiter"
override lazy val short = "fd"
val desc = "display the current found delimiter"
val longDesc = desc
+
+ private def getDelimOpt(s: StateForDebugger): Option[String] = {
+ if (s.delimitedParseResult.isDefined) {
+ val pr = s.delimitedParseResult.get
+ val delim = Misc.remapStringToVisibleGlyphs(pr.matchedDelimiterValue.get)
+ Some(delim)
+ } else {
+ None
+ }
+ }
+
def act(args: Seq[String], prestate: StateForDebugger, state: ParseOrUnparseState, processor: Processor): DebugState.Type = {
- state match {
- case pstate: PState => {
- if (pstate.delimitedParseResult.isDefined) {
- val pr = pstate.delimitedParseResult.get
- debugPrintln("%s:".format(name))
- debugPrintln("foundField: %s".format(Misc.remapStringToVisibleGlyphs(pr.field.get)), " ")
- debugPrintln("foundDelimiter: %s".format(Misc.remapStringToVisibleGlyphs(pr.matchedDelimiterValue.get)), " ")
- } else {
- debugPrintln("%s: nothing found".format(name))
- }
- }
- case ustate: UState => {
- // TODO
+ val delimOpt = getDelimOpt(state)
+ val str = delimOpt.getOrElse("nothing found")
+ debugPrintln("%s: %s".format(name, str))
+ DebugState.Pause
+ }
+
+ def diff(prestate: StateForDebugger, state: ParseOrUnparseState): Boolean = {
+ val delimOptPre = getDelimOpt(prestate)
+ val delimOptPost = getDelimOpt(state)
+
+ (delimOptPre, delimOptPost) match {
+ case (None, None) => false
+ case (Some(pre), Some(post)) if pre == post => false
+ case (_, _) => {
+ val strPre = delimOptPre.getOrElse("none")
+ val strPost = delimOptPost.getOrElse("none")
+ debugPrintln("%s: %s -> %s".format(name, strPre, strPost), " ")
+ true
}
- case _ => Assert.impossibleCase()
}
+ }
+ }
+
+ object InfoFoundField extends DebugCommand with DebugCommandValidateZeroArgs with InfoDiffable {
+ val name = "foundField"
+ override lazy val short = "ff"
+ val desc = "display the current found field when delimiter scanning"
+ val longDesc = desc
+
+ private def getFieldOpt(s: StateForDebugger): Option[String] = {
+ if (s.delimitedParseResult.isDefined) {
+ val pr = s.delimitedParseResult.get
+ val field = Misc.remapStringToVisibleGlyphs(pr.field.get)
+ Some(field)
+ } else {
+ None
+ }
+ }
+
+ def act(args: Seq[String], prestate: StateForDebugger, state: ParseOrUnparseState, processor: Processor): DebugState.Type = {
+ val fieldOpt = getFieldOpt(state)
+ val str = fieldOpt.getOrElse("nothing found")
+ debugPrintln("%s: %s".format(name, str))
DebugState.Pause
}
+
+ def diff(prestate: StateForDebugger, state: ParseOrUnparseState): Boolean = {
+ val fieldOptPre = getFieldOpt(prestate)
+ val fieldOptPost = getFieldOpt(state)
+ (fieldOptPre, fieldOptPost) match {
+ case (None, None) => false
+ case (Some(pre), Some(post)) if pre == post => false
+ case (_, _) => {
+ val strPre = fieldOptPre.getOrElse("none")
+ val strPost = fieldOptPost.getOrElse("none")
Review comment:
See InfoFoundDelimiter
##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/debugger/InteractiveDebugger.scala
##########
@@ -1459,47 +1505,15 @@ class InteractiveDebugger(runner: InteractiveDebuggerRunner, eCompilers: Express
override lazy val short = "diff"
val desc = "display the differences from the previous state"
val longDesc = desc
+
+ private lazy val infoDiffables = Info.subcommands.collect { case diffable: InfoDiffable => diffable }
+
def act(args: Seq[String], prestate: StateForDebugger, state: ParseOrUnparseState, processor: Processor): DebugState.Type = {
debugPrintln("%s:".format(name))
- var diff = false
- (prestate, state) match {
- case (prestate: StateForDebugger, state: ParseOrUnparseState) => {
- if (prestate.bytePos != state.bytePos) { debugPrintln("position (bytes): %d -> %d".format(prestate.bytePos, state.bytePos), " "); diff = true }
- if (prestate.bitLimit0b != state.bitLimit0b) { debugPrintln("bitLimit: %d -> %d".format(prestate.bitLimit0b, state.bitLimit0b), " "); diff = true }
- if (prestate.arrayPos != state.arrayPos) { debugPrintln("occursIndex: %d -> %d".format(prestate.arrayPos, state.arrayPos), " "); diff = true }
- if (prestate.groupPos != state.groupPos) { debugPrintln("groupIndex: %d -> %d".format(prestate.groupPos, state.groupPos), " "); diff = true }
- if (prestate.childPos != state.childPos) { debugPrintln("childIndex: %d -> %d".format(prestate.childPos, state.childPos), " "); diff = true }
- prestate.variableMap.qnames.foreach { qname =>
- val pre_instance = prestate.variableMap.find(qname).get
- val pre_value = pre_instance.value
- val pre_state = pre_instance.state
-
- val cur_instance = state.variableMap.find(qname).get
- val cur_value = cur_instance.value
- val cur_state = cur_instance.state
-
- if (pre_value != cur_value || pre_state != cur_state) {
- debugPrintln("variable: %s: %s -> %s".format(
- qname,
- InfoVariables.variableInstanceToDebugString(pre_instance),
- InfoVariables.variableInstanceToDebugString(cur_instance),
- ), " ")
- diff = true
- }
- }
- }
- case _ => // ok
+ val differencesExist = infoDiffables.foldLeft(false) { case (foundDiff, ic) =>
+ ic.diff(prestate, state) || foundDiff
Review comment:
At first glance, this looks like it could be: infoDiffables.exists(_.diff(prestate, state)).
This doesn't work, because we are relying on the side effect of diff calling debugPrint.
It is very unclear here that this expression has any side effect.
The more FP style approach would be to have diff return Maybe[String], and let the caller collect and print the messages.
Practically; perhaps we could rename diff -> printDiff to make it clear that diff is effectfull.
##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/debugger/InteractiveDebugger.scala
##########
@@ -1528,37 +1542,91 @@ class InteractiveDebugger(runner: InteractiveDebuggerRunner, eCompilers: Express
}
}
- object InfoFoundDelimiter extends DebugCommand with DebugCommandValidateZeroArgs {
+ object InfoFoundDelimiter extends DebugCommand with DebugCommandValidateZeroArgs with InfoDiffable {
val name = "foundDelimiter"
override lazy val short = "fd"
val desc = "display the current found delimiter"
val longDesc = desc
+
+ private def getDelimOpt(s: StateForDebugger): Option[String] = {
+ if (s.delimitedParseResult.isDefined) {
+ val pr = s.delimitedParseResult.get
+ val delim = Misc.remapStringToVisibleGlyphs(pr.matchedDelimiterValue.get)
+ Some(delim)
+ } else {
+ None
+ }
+ }
+
def act(args: Seq[String], prestate: StateForDebugger, state: ParseOrUnparseState, processor: Processor): DebugState.Type = {
- state match {
- case pstate: PState => {
- if (pstate.delimitedParseResult.isDefined) {
- val pr = pstate.delimitedParseResult.get
- debugPrintln("%s:".format(name))
- debugPrintln("foundField: %s".format(Misc.remapStringToVisibleGlyphs(pr.field.get)), " ")
- debugPrintln("foundDelimiter: %s".format(Misc.remapStringToVisibleGlyphs(pr.matchedDelimiterValue.get)), " ")
- } else {
- debugPrintln("%s: nothing found".format(name))
- }
- }
- case ustate: UState => {
- // TODO
+ val delimOpt = getDelimOpt(state)
+ val str = delimOpt.getOrElse("nothing found")
+ debugPrintln("%s: %s".format(name, str))
+ DebugState.Pause
+ }
+
+ def diff(prestate: StateForDebugger, state: ParseOrUnparseState): Boolean = {
+ val delimOptPre = getDelimOpt(prestate)
+ val delimOptPost = getDelimOpt(state)
+
+ (delimOptPre, delimOptPost) match {
+ case (None, None) => false
+ case (Some(pre), Some(post)) if pre == post => false
+ case (_, _) => {
+ val strPre = delimOptPre.getOrElse("none")
+ val strPost = delimOptPost.getOrElse("none")
Review comment:
Could just do: case (Some(strPre), Some(strPost)) instead of deconstructing again.
----------------------------------------------------------------
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:
users@infra.apache.org