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

Jump to bottom

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

Closed
pfroud opened this issue Apr 23, 2022 · 5 comments · Fixed by kaitai-io/kaitai_struct_compiler#283
Closed

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

pfroud opened this issue Apr 23, 2022 · 5 comments · Fixed by kaitai-io/kaitai_struct_compiler#283
Labels
bug found
Milestone
v0.11

Comments

@pfroud
Copy link

pfroud commented Apr 23, 2022 •
edited by generalmimon
Loading

Here's one KSY file:

meta:
  id: first_ksy_file
  imports:
    - second_ksy_file
seq:
  - id: foo
    type: "second_ksy_file::type_in_second_ksy_file"

Here's another KSY file:

meta:
  id: second_ksy_file
types:
  type_in_second_ksy_file:
    seq:
      - id: bar
        type: u1

In the read() method of FirstKsyFile.java, this line:

this.foo = new SecondKsyFile.TypeInSecondKsyFile(this._io, this, _root);

has this error on the third argument:

incompatible types: FirstKsyFile cannot be converted to SecondKsyFile

The constructor we're trying to call is:

public TypeInSecondKsyFile(KaitaiStream _io, FirstKsyFile _parent, SecondKsyFile _root) { ... }

In an answer on StackOverflow, GreyCat says this should be possible: Referencing Kaitai Struct file (ksy) in external ksy file in Kaitai Struct

Should I edit the generated Java code to use the two-argument constructor of TypeInSecondKsyFile (which does not have the _root parameter) instead?

Or maybe the KS compiler should make the type of the _root parameter be KaitaiStruct?
@hjmallon
Copy link

hjmallon commented Oct 25, 2022 •
edited
Loading

The same problem happens with the C++ cpp_stl generator and it causes compile failures. Also the #include paths are not correctly added.

@akatatsu27
Copy link

akatatsu27 commented Nov 22, 2022 •
edited
Loading

the same issue is present in c#. I have a .ksy file with "extra" types that I reuse in multiple files regularly. a single import statement of that one file, and scoping with "::" is much cleaner than making each type a separate file, and having to resolve multiple imports. The kaitai compiler allows this, and throws no error, but the generated code is filled with errors.
A simple solution seems to be to use the base class KaitaiStruct in the constructor definition instead of the generated from the meta id class

@generalmimon
Copy link
Member

Yeah, as evidenced by occurrences of the same problem in other languages, it doesn't seem to be specific to Java, so I'll rename the issue.

@generalmimon generalmimon changed the title Java: "incompatible types" error on constructor _root parameter when importing type from other KSY file "incompatible types" error on constructor _root parameter when importing type from other KSY file Mar 8, 2024
@generalmimon generalmimon mentioned this issue Mar 8, 2024
Closed
@Mingun
Copy link

Mingun commented Mar 13, 2024

I think, this is duplicate of #414.

@generalmimon
Copy link
Member

@Mingun:

I think, this is duplicate of #414.

No, it's not. #414 has no mention of imports. The Java error might look similar to #961 (or #414), but should be resolved differently: when imports are involved, _parent and _root should be omitted because individual .ksy specs should be self-sufficient, not change their behavior (and the generated code) depending on from where they're imported or whether they're imported at all.

@generalmimon generalmimon mentioned this issue Mar 18, 2024
Closed
generalmimon added a commit to kaitai-io/kaitai_struct_compiler that referenced this issue Mar 20, 2024
@generalmimon
Ensure that using imported nested types doesn't affect their parent type
10b0588 
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.
generalmimon added a commit to kaitai-io/kaitai_struct_compiler that referenced this issue Mar 20, 2024
@generalmimon
Pass _root and _parent to all local and no external types
7d383af 
* Fixes kaitai-io/kaitai_struct#1089

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

* Fixes kaitai-io/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](https://github.com/kaitai-io/kaitai_struct_tests/blob/d07deb4c/spec/ks/nested_types_import.kst#L17-L32).
@generalmimon generalmimon mentioned this issue Mar 20, 2024
Merged
generalmimon added a commit to kaitai-io/kaitai_struct_compiler that referenced this issue Mar 23, 2024
@generalmimon
Ensure that using imported nested types doesn't affect their parent type
250ccd2 
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.
generalmimon added a commit to kaitai-io/kaitai_struct_compiler that referenced this issue Mar 23, 2024
@generalmimon
Pass _root and _parent to all local and no external types
ee0d3aa 
* Fixes kaitai-io/kaitai_struct#1089

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

* Fixes kaitai-io/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](https://github.com/kaitai-io/kaitai_struct_tests/blob/d07deb4c/spec/ks/nested_types_import.kst#L17-L32).
generalmimon added a commit to kaitai-io/kaitai_struct_compiler that referenced this issue Mar 30, 2024
@generalmimon
Ensure that using imported nested types doesn't affect their parent type
cb2782b 
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.
generalmimon added a commit to kaitai-io/kaitai_struct_compiler that referenced this issue Mar 30, 2024
@generalmimon
Pass _root and _parent to all local and no external types
d190403 
* Fixes kaitai-io/kaitai_struct#1089

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

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

* Fixes kaitai-io/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](https://github.com/kaitai-io/kaitai_struct_tests/blob/d07deb4c/spec/ks/nested_types_import.kst#L17-L32).
@brunchboy brunchboy mentioned this issue Aug 29, 2024
Merged
@generalmimon generalmimon added this to the v0.11 milestone Jan 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug found
Projects
None yet
Milestone
v0.11
Development

Successfully merging a pull request may close this issue.

Pass _root and _parent to all local and no external types kaitai-io/kaitai_struct_compiler
5 participants
@Mingun @hjmallon @pfroud @akatatsu27 @generalmimon