Skip to content

Commit

Permalink
Ensure that using imported nested types doesn't affect their parent type
Browse files Browse the repository at this point in the history
Related:
* kaitai-io/kaitai_struct#703
* kaitai-io/kaitai_struct#963

Until now, using a nested type of an imported spec could affect the
derived parent type of that nested type. For example:

`main.ksy`

```ksy
meta:
  id: main
  imports:
    - imported

seq:
  - id: foo
    type: imported::nested_type
```

`imported.ksy`

```ksy
meta:
  id: imported
types:
  nested_type: {}
```

If you compile `main.ksy` for Java (`kaitai-struct-compiler -t java
main.ksy`), `Imported.java` looks like this:

```java
public class Imported extends KaitaiStruct {
    // ...
    public static class NestedType extends KaitaiStruct {
        public NestedType(KaitaiStream _io, Main _parent, Imported _root) { /* ... */ }
        //                                  ^^^^^^^^^^^^
    }
    // ...
}
```

Notice the `Main _parent` parameter - the compiler saw that
`imported::nested_type` is only used from the `main` type, so it decided
that the `_parent` type of `nested_type` will be `main`.

However, this means that the `_parent` crosses KSY spec boundaries and
the generated `Imported.java` code will be different depending on
whether you compile it as imported from `main.ksy` or standalone.
Furthermore, you could even access fields of `main` in
`imported::nested_type` via `_parent`, but that would mean that
`imported.ksy` would only work when imported and compiled via
`main.ksy`.

From <kaitai-io/kaitai_struct#71 (comment)>,
I suppose none of this should be possible:

> It would be a huge mess if `_root` relied on this particular ksy being
> imported from some other ksy, and only work in that case.

I agree that `_root` and `_parent` arguably shouldn't cross spec
boundaries at all, they should only be passed locally within one .ksy
spec, and therefore also parent types should only be derived from local
type usages.

This commit only adjusts the parent type derivation, not invocations of
imported nested types with `_parent` and `_root` still being passed (see
also kaitai-io/kaitai_struct#963) - they will
be fixed later.
  • Loading branch information
generalmimon committed Mar 30, 2024
1 parent 15959f0 commit cb2782b
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 1 deletion.
8 changes: 8 additions & 0 deletions shared/src/main/scala/io/kaitai/struct/format/ClassSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,14 @@ case class ClassSpec(

def parentType: DataType = parentClass.toDataType

/**
* Determines whether this `ClassSpec` represents a type that is external
* (i.e. not defined in the same .ksy file) from the perspective of the given
* `ClassSpec`.
*/
def isExternal(curClass: ClassSpec): Boolean =
name.head != curClass.name.head

/**
* Recursively traverses tree of types starting from this type, calling
* certain function for every type, starting from this one.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ class ParentTypes(classSpecs: ClassSpecs) {
}

def markupParentAs(curClass: ClassSpec, ut: UserType): Unit = {
Log.typeProcParent.info(() => s"..... class=$ut has parent=${curClass.nameAsStr}")
ut.classSpec match {
case Some(usedClass) =>
markupParentAs(curClass, usedClass)
Expand All @@ -79,6 +78,10 @@ class ParentTypes(classSpecs: ClassSpecs) {
}

def markupParentAs(parent: ClassSpec, child: ClassSpec): Unit = {
// Don't allow type usages across spec boundaries to affect parent resolution
if (child.isExternal(parent))
return
Log.typeProcParent.info(() => s"..... class=${child.nameAsStr} has parent=${parent.nameAsStr}")
child.parentClass match {
case UnknownClassSpec =>
child.parentClass = parent
Expand Down

0 comments on commit cb2782b

Please sign in to comment.