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.

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

Fix a number of errors in loops #308

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
ad126f2
Document `_currentIteratorType` and `_currentSwitchType` fields
Mingun Sep 15, 2024
d72dbc1
Raise human-readable error in case of incorrect usage of `_on` or `_`…
Mingun Nov 25, 2021
82a4584
Perl: Remove excess set of `_currentSwitchType`, it is already set in…
Mingun Sep 15, 2024
e30f6c2
Move set of type of `_` from each compiler implementation to the comm…
Mingun Sep 15, 2024
5712669
Rust: Remove unnecessary set of type of `_` variable, it is already s…
Mingun Sep 15, 2024
c96fcf1
Rust: no need to check that type of `_` is defined, it MUST be define…
Mingun Sep 15, 2024
9d29cd8
Set type of `_` for type validation only in `repeat-until` loops wher…
Mingun Sep 15, 2024
8e75690
Rename _currentIteratorType -> _lastParsedType
Mingun Sep 15, 2024
81a1fa2
Use Identifier.ITERATOR instead of "_"
Mingun Sep 15, 2024
86261a7
Remove unused parameters from `condRepeatUntilHeader` and `condRepeat…
Mingun Sep 15, 2024
f8076a6
Document `condRepeatUntilHeader` and `condRepeatUntilFooter` methods
Mingun Sep 15, 2024
f14d459
Fix inconsistency in values of `_index` variable in loops
Mingun Sep 15, 2024
ba43d20
Always calculate count of iterations only once in `repeat-expr` loops
Mingun Sep 15, 2024
5ab979b
Remove unused parameters from `condRepeatExprHeader` and document `co…
Mingun Sep 15, 2024
b8c280e
Perl: declare `_index` variable in `repeat: eos` loops
Mingun Sep 15, 2024
53fa661
Go, Rust: Fix usage of incorrect stream in `repeat: eos` loops when `…
Mingun Sep 15, 2024
3c21915
Remove unused parameters from `condRepeatEosHeader` and document `con…
Mingun Sep 15, 2024
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
27 changes: 20 additions & 7 deletions shared/src/main/scala/io/kaitai/struct/ClassTypeProvider.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,24 @@ import io.kaitai.struct.datatype.DataType
import io.kaitai.struct.datatype.DataType._
import io.kaitai.struct.exprlang.Ast
import io.kaitai.struct.format._
import io.kaitai.struct.precompile.{EnumNotFoundError, FieldNotFoundError, TypeNotFoundError, TypeUndecidedError}
import io.kaitai.struct.precompile.{EnumNotFoundError, ExpressionError, FieldNotFoundError, TypeNotFoundError, TypeUndecidedError}
import io.kaitai.struct.translators.TypeProvider

class ClassTypeProvider(classSpecs: ClassSpecs, var topClass: ClassSpec) extends TypeProvider {
var nowClass = topClass
val allClasses: ClassSpecs = classSpecs

var _currentIteratorType: Option[DataType] = None
/**
* Type of the `_` variable in the expression. That variable is defined in
* `repeat-until` and `valid: expr` contexts and refers to the attribute
* just parsed.
*/
var _lastParsedType: Option[DataType] = None
/**
* Type of the `_on` variable in the expression. That variable is defined in
* `cases.<case>` contexts and refers to the value of `switch-on` expression.
*/
var _currentSwitchType: Option[DataType] = None
def currentIteratorType: DataType = _currentIteratorType.get
def currentSwitchType: DataType = _currentSwitchType.get

override def determineType(attrName: String): DataType = {
determineType(nowClass, attrName)
Expand All @@ -30,14 +37,20 @@ class ClassTypeProvider(classSpecs: ClassSpecs, var topClass: ClassSpec) extends
topClass.toDataType
case Identifier.PARENT =>
if (inClass.parentClass == UnknownClassSpec)
throw new RuntimeException(s"Unable to derive ${Identifier.PARENT} type in ${inClass.name.mkString("::")}")
throw new ExpressionError(s"Unable to derive '${Identifier.PARENT}' type in '${inClass.nameAsStr}'")
inClass.parentClass.toDataType
case Identifier.IO =>
KaitaiStreamType
case Identifier.ITERATOR =>
currentIteratorType
_lastParsedType match {
case Some(value) => value
case None => throw new ExpressionError(s"Context variable '$attrName' is available only in the 'repeat-until' and 'valid/expr' attributes")
}
case Identifier.SWITCH_ON =>
currentSwitchType
_currentSwitchType match {
case Some(value) => value
case None => throw new ExpressionError(s"Context variable '$attrName' is available only in the 'cases.<case>' expressions")
}
case Identifier.INDEX =>
CalcIntType
case Identifier.SIZEOF =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ class ConstructClassCompiler(classSpecs: ClassSpecs, topClass: ClassSpec) extend
case RepeatExpr(expr) =>
s"Array(${translator.translate(expr)}, $typeStr)"
case RepeatUntil(expr) =>
provider._currentIteratorType = Some(dataType)
provider._lastParsedType = Some(dataType)
s"RepeatUntil(lambda obj_, list_, this: ${translator.translate(expr)}, $typeStr)"
case RepeatEos =>
s"GreedyRange($typeStr)"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ class GraphvizClassCompiler(classSpecs: ClassSpecs, topClass: ClassSpec) extends
case ValidationInEnum() =>
"must be defined in the enum"
case ValidationExpr(expr) =>
provider._currentIteratorType = Some(dataType)
provider._lastParsedType = Some(dataType)
s"must satisfy ${expression(expr, fullPortName, STYLE_EDGE_VALID)}"
}
case None => return
Expand All @@ -212,7 +212,7 @@ class GraphvizClassCompiler(classSpecs: ClassSpecs, topClass: ClassSpec) extends
expression(ex, s"$currentTable:$portName", STYLE_EDGE_REPEAT) +
" times</TD></TR>")
case RepeatUntil(ex) =>
provider._currentIteratorType = Some(dataType)
provider._lastParsedType = Some(dataType)
out.puts("<TR><TD COLSPAN=\"4\" PORT=\"" + portName + "\">repeat until " +
expression(ex, s"$currentTable:$portName", STYLE_EDGE_REPEAT) +
"</TD></TR>")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ class CSharpCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)
out.puts(s"${privateMemberName(id)} = new ${kaitaiType2NativeType(ArrayTypeInStream(dataType))}();")
}

override def condRepeatEosHeader(id: Identifier, io: String, dataType: DataType): Unit = {
override def condRepeatEosHeader(io: String): Unit = {
out.puts("{")
out.inc
out.puts("var i = 0;")
Expand All @@ -300,8 +300,8 @@ class CSharpCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)
out.puts("}")
}

override def condRepeatExprHeader(id: Identifier, io: String, dataType: DataType, repeatExpr: expr): Unit = {
out.puts(s"for (var i = 0; i < ${expression(repeatExpr)}; i++)")
override def condRepeatExprHeader(countExpr: expr): Unit = {
out.puts(s"for (var i = 0, _end = ${expression(countExpr)}; i < _end; ++i)")
out.puts("{")
out.inc
}
Expand All @@ -311,12 +311,12 @@ class CSharpCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)

override def condRepeatExprFooter: Unit = fileFooter(null)

override def condRepeatUntilHeader(id: Identifier, io: String, dataType: DataType, untilExpr: expr): Unit = {
override def condRepeatUntilHeader(itemType: DataType): Unit = {
out.puts("{")
out.inc
out.puts("var i = 0;")
out.puts(s"${kaitaiType2NativeType(dataType)} ${translator.doName("_")};")
out.puts("do {")
out.puts(s"${kaitaiType2NativeType(itemType)} ${translator.doName(Identifier.ITERATOR)};")
out.puts("while (true) {")
out.inc
}

Expand All @@ -330,13 +330,17 @@ class CSharpCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)
out.puts(s"${privateMemberName(id)}.Add($tempVar);")
}

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

override def handleAssignmentSimple(id: Identifier, expr: String): Unit =
Expand Down
28 changes: 15 additions & 13 deletions shared/src/main/scala/io/kaitai/struct/languages/CppCompiler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -571,7 +571,7 @@ class CppCompiler(
outSrc.puts(s"${privateMemberName(id)} = ${newVector(dataType)};")
}

override def condRepeatEosHeader(id: Identifier, io: String, dataType: DataType): Unit = {
override def condRepeatEosHeader(io: String): Unit = {
outSrc.puts("{")
outSrc.inc
outSrc.puts("int i = 0;")
Expand All @@ -591,10 +591,8 @@ class CppCompiler(
outSrc.puts("}")
}

override def condRepeatExprHeader(id: Identifier, io: String, dataType: DataType, repeatExpr: Ast.expr): Unit = {
val lenVar = s"l_${idToStr(id)}"
outSrc.puts(s"const int $lenVar = ${expression(repeatExpr)};")
outSrc.puts(s"for (int i = 0; i < $lenVar; i++) {")
override def condRepeatExprHeader(countExpr: Ast.expr): Unit = {
outSrc.puts(s"for (int i = 0, _end = ${expression(countExpr)}; i < _end; ++i) {")
outSrc.inc
}

Expand All @@ -606,12 +604,12 @@ class CppCompiler(
outSrc.puts("}")
}

override def condRepeatUntilHeader(id: Identifier, io: String, dataType: DataType, untilExpr: expr): Unit = {
override def condRepeatUntilHeader(itemType: DataType): Unit = {
outSrc.puts("{")
outSrc.inc
outSrc.puts("int i = 0;")
outSrc.puts(s"${kaitaiType2NativeType(dataType.asNonOwning())} ${translator.doName("_")};")
outSrc.puts("do {")
outSrc.puts(s"${kaitaiType2NativeType(itemType.asNonOwning())} ${translator.doName(Identifier.ITERATOR)};")
outSrc.puts("while (true) {")
outSrc.inc
}

Expand Down Expand Up @@ -640,13 +638,17 @@ class CppCompiler(
outSrc.puts(s"${privateMemberName(id)}->push_back($wrappedTempVar);")
}

override def condRepeatUntilFooter(id: Identifier, io: String, dataType: DataType, untilExpr: expr): Unit = {
typeProvider._currentIteratorType = Some(dataType)
outSrc.puts("i++;")
outSrc.dec
outSrc.puts(s"} while (!(${expression(untilExpr)}));")
override def condRepeatUntilFooter(untilExpr: expr): Unit = {
outSrc.puts(s"if (${expression(untilExpr)}) {")
outSrc.inc
outSrc.puts("break;")
outSrc.dec
outSrc.puts("}")
outSrc.puts("++i;")
outSrc.dec
outSrc.puts("}") // close while (true)
outSrc.dec
outSrc.puts("}") // close scope of i and _ variables
}

override def handleAssignmentSimple(id: Identifier, expr: String): Unit = {
Expand Down
23 changes: 14 additions & 9 deletions shared/src/main/scala/io/kaitai/struct/languages/GoCompiler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -300,12 +300,12 @@ class GoCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)
// function works even on `nil` slices (https://go.dev/tour/moretypes/15)
}

override def condRepeatEosHeader(id: Identifier, io: String, dataType: DataType): Unit = {
override def condRepeatEosHeader(io: String): Unit = {
out.puts(s"for i := 0;; i++ {")
out.inc

val eofVar = translator.allocateLocalVar()
out.puts(s"${translator.localVarName(eofVar)}, err := this._io.EOF()")
out.puts(s"${translator.localVarName(eofVar)}, err := $io.EOF()")
translator.outAddErrCheck()
out.puts(s"if ${translator.localVarName(eofVar)} {")
out.inc
Expand All @@ -320,8 +320,8 @@ class GoCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)
out.puts(s"$name = append($name, $expr)")
}

override def condRepeatExprHeader(id: Identifier, io: String, dataType: DataType, repeatExpr: Ast.expr): Unit = {
out.puts(s"for i := 0; i < int(${expression(repeatExpr)}); i++ {")
override def condRepeatExprHeader(countExpr: Ast.expr): Unit = {
out.puts(s"for i, _end := 0, int(${expression(countExpr)}); i < _end; i++ {")
out.inc
// FIXME: Go throws a fatal compile error when the `i` variable is not used (unused variables
// can only use the blank identifier `_`, see https://go.dev/doc/effective_go#blank), so we have
Expand All @@ -334,8 +334,11 @@ class GoCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)
override def handleAssignmentRepeatExpr(id: Identifier, r: TranslatorResult): Unit =
handleAssignmentRepeatEos(id, r)

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

Expand All @@ -346,15 +349,17 @@ class GoCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)
out.puts(s"${privateMemberName(id)} = append(${privateMemberName(id)}, $tempVar)")
}

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

private def castToType(r: TranslatorResult, dataType: DataType): TranslatorResult = {
Expand Down
26 changes: 15 additions & 11 deletions shared/src/main/scala/io/kaitai/struct/languages/JavaCompiler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,7 @@ class JavaCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)
override def condRepeatInitAttr(id: Identifier, dataType: DataType): Unit =
out.puts(s"${privateMemberName(id)} = new ${kaitaiType2JavaType(ArrayTypeInStream(dataType))}();")

override def condRepeatEosHeader(id: Identifier, io: String, dataType: DataType): Unit = {
override def condRepeatEosHeader(io: String): Unit = {
out.puts("{")
out.inc
out.puts("int i = 0;")
Expand All @@ -377,8 +377,8 @@ class JavaCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)
out.puts("}")
}

override def condRepeatExprHeader(id: Identifier, io: String, dataType: DataType, repeatExpr: expr): Unit = {
out.puts(s"for (int i = 0; i < ${expression(repeatExpr)}; i++) {")
override def condRepeatExprHeader(countExpr: expr): Unit = {
out.puts(s"for (int i = 0, _end = ${expression(countExpr)}; i < _end; ++i) {")
out.inc

importList.add("java.util.ArrayList")
Expand All @@ -387,12 +387,12 @@ class JavaCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)
override def handleAssignmentRepeatExpr(id: Identifier, expr: String): Unit =
handleAssignmentRepeatEos(id, expr)

override def condRepeatUntilHeader(id: Identifier, io: String, dataType: DataType, untilExpr: expr): Unit = {
override def condRepeatUntilHeader(itemType: DataType): Unit = {
out.puts("{")
out.inc
out.puts(s"${kaitaiType2JavaType(dataType)} ${translator.doName("_")};")
out.puts(s"${kaitaiType2JavaType(itemType)} ${translator.doName(Identifier.ITERATOR)};")
out.puts("int i = 0;")
out.puts("do {")
out.puts("while (true) {")
out.inc

importList.add("java.util.ArrayList")
Expand All @@ -408,13 +408,17 @@ class JavaCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)
out.puts(s"${privateMemberName(id)}.add($tempVar);")
}

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

override def handleAssignmentSimple(id: Identifier, expr: String): Unit =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ class JavaScriptCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)
override def condRepeatInitAttr(id: Identifier, dataType: DataType): Unit =
out.puts(s"${privateMemberName(id)} = [];")

override def condRepeatEosHeader(id: Identifier, io: String, dataType: DataType): Unit = {
override def condRepeatEosHeader(io: String): Unit = {
out.puts("var i = 0;")
out.puts(s"while (!$io.isEof()) {")
out.inc
Expand All @@ -319,8 +319,8 @@ class JavaScriptCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)
out.puts("}")
}

override def condRepeatExprHeader(id: Identifier, io: String, dataType: DataType, repeatExpr: expr): Unit = {
out.puts(s"for (var i = 0; i < ${expression(repeatExpr)}; i++) {")
override def condRepeatExprHeader(countExpr: expr): Unit = {
out.puts(s"for (var i = 0, _end = ${expression(countExpr)}; i < _end; ++i) {")
out.inc
}

Expand All @@ -332,9 +332,10 @@ class JavaScriptCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)
out.puts("}")
}

override def condRepeatUntilHeader(id: Identifier, io: String, dataType: DataType, untilExpr: expr): Unit = {
override def condRepeatUntilHeader(itemType: DataType): Unit = {
// "var" variables in any case have a scope of surrounding function, no need a scope to isolate them
out.puts("var i = 0;")
out.puts("do {")
out.puts("while (true) {")
out.inc
}

Expand All @@ -344,11 +345,15 @@ class JavaScriptCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)
out.puts(s"${privateMemberName(id)}.push($tmpName);")
}

override def condRepeatUntilFooter(id: Identifier, io: String, dataType: DataType, untilExpr: expr): Unit = {
typeProvider._currentIteratorType = Some(dataType)
out.puts("i++;")
override def condRepeatUntilFooter(untilExpr: expr): Unit = {
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("}") // close while (true)
}

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