Skip to content
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.

Sign up for GitHub

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

Fixes/standardizes the behavior of a repetition '_index' within a 'repeat-until' expression #245

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Fixes/standardizes the behavior of a repetition '_index' within a 'repeat-until' expression #245

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fe6aa38
fix behavior for repetition index within repeat-until expression
transverberate Apr 4, 2022
0905a2e
changed tab to 2x spaces
transverberate Apr 4, 2022
e53da62
changed to exclusive evaluation
transverberate Apr 7, 2022
54e1c5f
Merge branch 'master' of github.com:kaitai-io/kaitai_struct_compiler …
transverberate Apr 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions shared/src/main/scala/io/kaitai/struct/languages/CSharpCompiler.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ class CSharpCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)
out.inc
out.puts("var i = 0;")
out.puts(s"${kaitaiType2NativeType(dataType)} ${translator.doName("_")};")
out.puts("do {")
out.puts("while (true) {")
out.inc
}

Expand All @@ -336,9 +336,12 @@ class CSharpCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)

override def condRepeatUntilFooter(id: Identifier, io: String, dataType: DataType, untilExpr: expr): Unit = {
typeProvider._currentIteratorType = Some(dataType)
out.puts("i++;")
out.puts(s"if (${expression(untilExpr)}}) {")
out.inc
out.puts("break;")
out.dec
out.puts(s"} while (!(${expression(untilExpr)}));")
out.puts("}")
out.puts("i++;")
out.dec
out.puts("}")
}
Expand Down
9 changes: 6 additions & 3 deletions shared/src/main/scala/io/kaitai/struct/languages/CppCompiler.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -674,7 +674,7 @@ class CppCompiler(
outSrc.inc
outSrc.puts("int i = 0;")
outSrc.puts(s"${kaitaiType2NativeType(dataType.asNonOwning)} ${translator.doName("_")};")
outSrc.puts("do {")
outSrc.puts("while (true) {")
outSrc.inc
}

Expand Down Expand Up @@ -705,9 +705,12 @@ class CppCompiler(

override def condRepeatUntilFooter(id: Identifier, io: String, dataType: DataType, untilExpr: expr): Unit = {
typeProvider._currentIteratorType = Some(dataType)
outSrc.puts("i++;")
outSrc.puts(s"if (${expression(untilExpr)}}) {")
outSrc.inc
outSrc.puts("break;")
outSrc.dec
outSrc.puts(s"} while (!(${expression(untilExpr)}));")
outSrc.puts("}")
outSrc.puts("i++;")
outSrc.dec
outSrc.puts("}")
}
Expand Down
4 changes: 3 additions & 1 deletion shared/src/main/scala/io/kaitai/struct/languages/GoCompiler.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,8 @@ class GoCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)
handleAssignmentRepeatEos(id, r)

override def condRepeatUntilHeader(id: Identifier, io: String, dataType: DataType, untilExpr: Ast.expr): Unit = {
out.puts(s"for i := 1;; i++ {")
out.puts("i := 0")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I anticipate a problem with multiple declarations of the variable i in the same function/scope here (IIRC, the := operator in Go must introduce at least one new variable). This should be solved by wrapping the i declaration along with the for loop into a block scope. Note that this is already done in C# and Java.

The test RepeatUntilComplex (repeat_until_complex.ksy) should test this case.

Technically, there are already common methods blockScope{Header,Footer} in each language compiler. The reason the condRepeatUntil{Header,Footer} methods of CSharpCompiler and JavaCompiler don't use it is probably because blockScope{Header,Footer} were only added later (in 0.9 I think) for another feature and nobody went through the existing codebase to see whether it can't be used somewhere else (but it's no big deal, just two lines here and there).

This redeclaration issue typically only occurs in statically-typed languages, because the dynamically-typed are usually OK with redeclaring arbitrary variables.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry to let my PR go a bit stale.
Thanks for your continued interested in this as I believe this usecase is very important.
I'll check on this later this week and add the appropriate scoping.
Thanks for your patience.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry to let my PR go a bit stale.

You don't have to apologize. I'm sorry, because I was the one who let this PR go stale :) Unfortunately, there is always something with a higher priority, and it's hard to allocate time for less "urgent" things.

I appreciate your quick response and hopefully we get this merged soon.

out.puts("for {")
out.inc
}

Expand All @@ -351,6 +352,7 @@ class GoCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)
out.puts("break")
out.dec
out.puts("}")
out.puts("i++")
out.dec
out.puts("}")
}
Expand Down
9 changes: 6 additions & 3 deletions shared/src/main/scala/io/kaitai/struct/languages/JavaCompiler.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ class JavaCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)
out.inc
out.puts(s"${kaitaiType2JavaType(dataType)} ${translator.doName("_")};")
out.puts("int i = 0;")
out.puts("do {")
out.puts("while (true) {")
out.inc

importList.add("java.util.ArrayList")
Expand All @@ -419,9 +419,12 @@ class JavaCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)

override def condRepeatUntilFooter(id: Identifier, io: String, dataType: DataType, untilExpr: expr): Unit = {
typeProvider._currentIteratorType = Some(dataType)
out.puts("i++;")
out.puts(s"if (${expression(untilExpr)}}) {")
out.inc
out.puts("break;")
out.dec
out.puts(s"} while (!(${expression(untilExpr)}));")
out.puts("}")
out.puts("i++;")
out.dec
out.puts("}")
}
Expand Down
9 changes: 7 additions & 2 deletions shared/src/main/scala/io/kaitai/struct/languages/JavaScriptCompiler.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ class JavaScriptCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)

override def condRepeatUntilHeader(id: Identifier, io: String, dataType: DataType, untilExpr: expr): Unit = {
out.puts("var i = 0;")
out.puts("do {")
out.puts("while (true) {")
out.inc
}

Expand All @@ -350,9 +350,14 @@ class JavaScriptCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)

override def condRepeatUntilFooter(id: Identifier, io: String, dataType: DataType, untilExpr: expr): Unit = {
typeProvider._currentIteratorType = Some(dataType)
out.puts(s"if (${expression(untilExpr)}}) {")
out.inc
out.puts("break;")
out.dec
out.puts("}")
out.puts("i++;")
out.dec
out.puts(s"} while (!(${expression(untilExpr)}));")
out.puts("}")
}

override def handleAssignmentSimple(id: Identifier, expr: String): Unit = {
Expand Down
9 changes: 7 additions & 2 deletions shared/src/main/scala/io/kaitai/struct/languages/PHPCompiler.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ class PHPCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)

override def condRepeatUntilHeader(id: Identifier, io: String, dataType: DataType, untilExpr: Ast.expr): Unit = {
out.puts("$i = 0;")
out.puts("do {")
out.puts("while (1) {")
out.inc
}

Expand All @@ -313,9 +313,14 @@ class PHPCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)

override def condRepeatUntilFooter(id: Identifier, io: String, dataType: DataType, untilExpr: Ast.expr): Unit = {
typeProvider._currentIteratorType = Some(dataType)
out.puts(s"if ${expression(untilExpr)} {")
out.inc
out.puts("break")
out.dec
out.puts("}")
out.puts("$i++;")
out.dec
out.puts(s"} while (!(${expression(untilExpr)}));")
out.puts("}")
}

override def handleAssignmentSimple(id: Identifier, expr: String): Unit = {
Expand Down
7 changes: 5 additions & 2 deletions shared/src/main/scala/io/kaitai/struct/languages/PerlCompiler.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,8 @@ class PerlCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)
handleAssignmentRepeatEos(id, expr)

override def condRepeatUntilHeader(id: Identifier, io: String, dataType: DataType, untilExpr: expr): Unit = {
out.puts("do {")
out.puts("my $i = 0;")
out.puts("while (1) {")
out.inc
}

Expand All @@ -279,8 +280,10 @@ class PerlCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)

override def condRepeatUntilFooter(id: Identifier, io: String, dataType: DataType, untilExpr: expr): Unit = {
typeProvider._currentIteratorType = Some(dataType)
out.puts(s"last if(${expression(untilExpr)});")
out.puts("$i++;")
out.dec
out.puts(s"} until (${expression(untilExpr)});")
out.puts("}")
}

override def handleAssignmentSimple(id: Identifier, expr: String): Unit =
Expand Down
5 changes: 3 additions & 2 deletions shared/src/main/scala/io/kaitai/struct/languages/RubyCompiler.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ class RubyCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)

override def condRepeatUntilHeader(id: Identifier, io: String, dataType: DataType, untilExpr: expr): Unit = {
out.puts("i = 0")
out.puts("begin")
out.puts("while true")
out.inc
}

Expand All @@ -325,9 +325,10 @@ class RubyCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)

override def condRepeatUntilFooter(id: Identifier, io: String, dataType: DataType, untilExpr: expr): Unit = {
typeProvider._currentIteratorType = Some(dataType)
out.puts(s"break if ${expression(untilExpr)}")
out.puts("i += 1")
out.dec
out.puts(s"end until ${expression(untilExpr)}")
out.puts("end")
}

override def handleAssignmentSimple(id: Identifier, expr: String): Unit =
Expand Down
10 changes: 8 additions & 2 deletions shared/src/main/scala/io/kaitai/struct/languages/RustCompiler.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,7 @@ class RustCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)
handleAssignmentRepeatEos(id, expr)

override def condRepeatUntilHeader(id: Identifier, io: String, dataType: DataType, untilExpr: Ast.expr): Unit = {
out.puts("let mut i = 0;")
out.puts("while {")
out.inc
}
Expand All @@ -299,9 +300,14 @@ class RustCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)

override def condRepeatUntilFooter(id: Identifier, io: String, dataType: DataType, untilExpr: Ast.expr): Unit = {
typeProvider._currentIteratorType = Some(dataType)
out.puts(s"!(${expression(untilExpr)})")
out.puts(s"if ${expression(untilExpr)}} {")
out.inc
out.puts("break;")
out.dec
out.puts("}")
out.puts("i += 1;")
out.dec
out.puts("} { }")
out.puts("}")
}

override def handleAssignmentSimple(id: Identifier, expr: String): Unit = {
Expand Down