Regex support in valid key

[jocelynke]

Working for python, cpp_stl_11. Have to be tested for other implementations (C#, Go, Java, JS, Ruby, Rust, PHP, Perl), not implemented on remaining. Can't work on cpp_stl_98 without other library.

[GreyCat]

Will this fail if it will be used twice in the same function?

[generalmimon]

And I think it's not wise to ignore the error in case it occurs (doing matched, _ :=). Please follow the same approach used in the rest of the GoTranslator, notice that the methods you added are ones of the few in the file returning String and not TranslatorResult. There is as special method outVarCheckRes handling the errors returned from functions, I suggest using that one.

[jocelynke]

I tried to modify the translateExpr func in GoTranslator, but it seems to me that this function is not executed (I use some logs), could you explain it to me ?

[generalmimon]

Will this fail if it will be used twice in the same function?

Yes, it probably will, because the variables can't be redeclared in the same scope in Go unless some new variables are introduced. I've made a simple test: https://repl.it/repls/WonderfulJointProcedure

I tried to modify the translateExpr func in GoTranslator, but it seems to me that this function is not executed

@jocelynke Go language didn't play well with the classic BaseTranslator model that returns Strings, because of the error handling in Go. Go doesn't have try/catch as other languages, it treats errors as values. This means that if you want the error from a function you call to bubble up in the call stack, it won't happen automatically. You have to check if the error value returned from the function is nil and if it's not, then an error occured and you should return it to the caller. (Just returning it isn't exactly debug-friendly, because the caller loses any context which function in the stack actually caused the error, but it's unfortunately done this way in KS currently.)

This means that parsing a single u1 field from the stream can't be done in a single expression (because it can of course result in an EOF error), but instead it is a 3-phase process:

func (this *HelloWorld) Read(...) (err error) {
 // ...
    tmp1, err := this._io.ReadU1()
    if err != nil {
        return err
    }
    this.One = tmp1

Because of this, GoTranslator can't just extend BaseTranslator as other languages do, because the methods in BaseTranslator have return type String which doesn't fit for Go (and it can't be changed in the derived classes). Not all methods in GoTranslator can be implemented in a single expression which can be passed as String, for these the return value of the called error-returning method is saved to a temporary variable (named tmp1, tmp2, ... to avoid name conflicts), the error is handled and only the "serial number" of the temp variable is returned. This is done by the method outVarCheckRes.

Hence most methods in GoTranslator return TranslatorResult, which is defined on the top:

kaitai_struct_compiler/shared/src/main/scala/io/kaitai/struct/translators/GoTranslator.scala

Lines 11 to 13 in ef7178c

	sealed trait TranslatorResult
	case class ResultString(s: String) extends TranslatorResult
	case class ResultLocalVar(n: Int) extends TranslatorResult

And now how you should do it. I think the doRegexMatchOp here could look like this:

override def doRegexMatchOp(str: String, regex: String): TranslatorResult = {
  importList.add("regexp")
  outVarCheckRes(s"regexp.MatchString(`${regex}`, ${trStringLiteral(str)})")

Then you'll have to move the abstract definition of this method from CommonOps to BaseTranslator:

kaitai_struct_compiler/shared/src/main/scala/io/kaitai/struct/translators/CommonOps.scala

Line 68 in 7120c8e

def doRegexMatchOp(str: String, regex: String): String

The reason is that CommonOps is inherited by GoTranslator too (whereas BaseTranslator is not), but in GoTranslator you need to specify a different return type, which won't work in Scala. And the Ast.expr.RegexMatch handling in BaseTranslator

kaitai_struct_compiler/shared/src/main/scala/io/kaitai/struct/translators/BaseTranslator.scala

Lines 105 to 112 in 7120c8e

	case Ast.expr.RegexMatch(str: Ast.expr, regex: String) => {
	detectType(str) match {
	case (_: StrType) =>
	doRegexMatchOp(translate(str), doRegex(regex))
	case _ =>
	throw new TypeMismatchError(s"regex match need strings")
	}
	}

needs to be in GoTranslator as well, because again, it doesn't extend BaseTranslator.

[jocelynke]

Thank you for this deep explanation, I had implemented what you told me, but the thing is I have no log of a call to translateExpr in the GoTranslator. The consequence is that no code is generated to validate the field that needs it.

[jocelynke]

@generalmimon I found that the valid key is not supported by Go in kaitai, so it's normal that no code is generated...

[generalmimon]

@jocelynke Could you please push your changes to your branch jocelynke:regex_support so we can see the changes you've made? It seems that you're just marking the things as resolved, but you don't publish anything so we can't say. The commits you push into this branch will appear in this PR.

[jocelynke]

@generalmimon I wanted to correct everything before pushing. Here are the remaining to do : Go support, Rust dynamic include

[GreyCat]

For equality checks (and storing original value in question for C++), we need to know original value data type. But regular expression matches would likely only work for strings. What's the point in passing _dt: DataType here?

[GreyCat]

Nit: unnecessary double newline.

Suggested change

[GreyCat]

Nit: unnecessary newline

Suggested change

[GreyCat]

Nit: formatting.

Suggested change

	def doRegexMatchOp(str: String, regex: String): String
	def doRegexMatchOp(str: String, regex: String): String