Pass _root and _parent to all local and no external types

[generalmimon]

• Fixes _root / _parent chain broken by recursive use of the top-level type kaitai_struct#1089

  Previously, _root and _parent weren't passed to the local top-level type, which is fixed by this PR.

  This was demonstrated by tests NavRootRecursive and NavParentRecursive added in kaitai-io/kaitai_struct_tests@5941262e

• Fixes "incompatible types" error on constructor _root parameter when importing type from other KSY file kaitai_struct#963

  _root and _parent were incorrectly passed to nested types of imported specs (i.e. external types).

  This was demonstrated by asserts in the KST spec of test NestedTypesImport.

[Mingun]

I correctly understand, that the 3rd commit disables passing _parent to the types defined in imported specs? If yes, why? It is clear for _root because you just does not have the correct type for it in scope, but _parent still can be passed as this, right?

Using your example from the first commit:

# main.ksy
meta:
  id: main
  imports:
    - imported
seq:
  - id: foo
    type: imported::nested_type

# imported.ksy
meta:
  id: imported
types:
  nested_type: {}

I think, that these changes should be expected for Java target because of your fix (you mentioned that Java does not pass _parent today):

 // Imported.java
 public class Imported extends KaitaiStruct {
     public static class NestedType extends KaitaiStruct {
-        public NestedType(KaitaiStream _io, Main _parent, Imported _root) { /* ... */ }
+        public NestedType(KaitaiStream _io, KaitaiStruct _parent, Imported _root) { /* ... */ }
     }
 }
 // Main.java
 public class Main extends KaitaiStruct {
     void _read() {
-        this.foo = new Imported.NestedType(this.io);
+        this.foo = new Imported.NestedType(this.io, this, null);
     }
 }

[generalmimon]

@Mingun:

I correctly understand, that the 3rd commit disables passing _parent to the types defined in imported specs?

Yes.

It is clear for _root because you just does not have the correct type for it in scope, but _parent still can be passed as this, right?

Well, not in general. The change in 10b0588 means that cross-spec type usages don't influence the _parent type, so you cannot assume that this in the importing type has a type compatible with the target _parent parameter. In particular, passing _parent between different .ksy specs would be a problem e.g. in this case:

main.ksy

meta:
  id: main
  imports:
    - imported
seq:
  - id: foo
    type: imported::nested_type(false)

imported.ksy

meta:
  id: imported
seq:
  - id: len_body
    type: u1
  - id: wrapper
    type: nested_type(true)
types:
  nested_type:
    params:
      - id: use_parent
        type: bool
    seq:
      - id: body
        size: _parent.len_body
        if: use_parent

(The use_parent parameter used in if isn't really necessary for the demo, it's just to handwave the somewhat questionable runtime behavior that otherwise the instantiation of the main module would always result in a runtime error.)

As you can see, imported::nested_type relies on having imported as _parent, so if you try to pass main as _parent instead, that will be a compile error in statically typed languages, as you can imagine:

$ javac -classpath /c/temp/kaitai_struct/runtime/java/target/kaitai-struct-runtime-0.11-SNAPSHOT.jar Main.java Imported.java
Picked up JAVA_TOOL_OPTIONS: -Dfile.encoding=UTF-8
Main.java:28: error: incompatible types: Main cannot be converted to Imported
        this.foo = new Imported.NestedType(this._io, this, false);
                                                     ^
Note: Some messages have been simplified; recompile with -Xdiags:verbose to get full output
1 error

... since the possible constructor signatures of class Imported.NestedType are the following:

    public static class NestedType extends KaitaiStruct {

        public NestedType(KaitaiStream _io, boolean useParent) {
            this(_io, null, null, useParent);
        }

        public NestedType(KaitaiStream _io, Imported _parent, boolean useParent) {
            this(_io, _parent, null, useParent);
        }

        public NestedType(KaitaiStream _io, Imported _parent, Imported _root, boolean useParent) {
            // ...

---

In conclusion, I'm convinced we need 10b0588 because letting different .ksy specs affect the derived _parent types of each other seems to be a big can of worms (it means that a .ksy spec behaves differently depending on whether it's imported from somewhere or compiled directly, namely a .ksy spec could compile without errors directly and with errors when imported from somewhere or vice versa, or the generated code of the imported module would depend on where the module is imported from etc.), and not actually passing _parent between specs is a direct logical consequence of that.

So, as @GreyCat pointed out in kaitai-io/kaitai_struct#71 (comment), if someone feels like they need to pass _parent between different .ksy specs, it's better to pass what they need using params.

Keeping both _root and _parent local just seems to be the only sane way to me once you think about the consequences of doing otherwise. Also, this was already discussed (though not in great detail), e.g.:

kaitai-io/kaitai_struct#71 (comment) (@ams-tschoening)

If one externalizes a KSY and imports it later in some other type, how are _root and _parent supposed to work in the extern type?

kaitai-io/kaitai_struct#71 (comment) (@KOLANICH)

Modules are modular. If they accessed parent types, for example using _root, they would be not modular, so they would not be modules. And this was discussed somewhere.

(...) IMHO passing something (a reference to parent's field?) as a param is a better approach.

---

kaitai-io/kaitai_struct#275 (comment) (@ams-tschoening)

With your example CTOR invocation, _parent is crossing KSY-borders and didn't you say you don't want that or is that only meant for _root? Might not be a good idea to handle both differently.

kaitai-io/kaitai_struct#275 (comment) (@GreyCat)

With your example CTOR invocation, _parent is crossing KSY-borders

Yeah, and that things actually bothers me as well. (...)

So I believe the consensus is clear.

In the long run, I'm not completely opposed to kaitai-io/kaitai_struct#770, i.e. getting rid of _parent and _root completely in favor of explicit params - I actually like it more and more over time. Especially the implicitly derived type of _parent seems to create many problems that would not exist without it. Also, you can never be sure whether both _parent and _root are actually non-null until runtime, etc.

[Mingun]

Thanks for that so detailed explanation. Yes, falling asleep, I realized that when _parent is deduced to some local type of imported spec it would be impossible to pass types from the main spec there. So you implemented approach is correct. Maybe document those observations somewhere in the code?

[GreyCat]

Looks great, thanks for making this reality, @generalmimon!