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

ks-opaque-types not working with imports #295

Closed
GreyCat opened this issue Dec 4, 2017 · 3 comments · Fixed by kaitai-io/kaitai_struct_compiler#267
Closed

ks-opaque-types not working with imports #295

GreyCat opened this issue Dec 4, 2017 · 3 comments · Fixed by kaitai-io/kaitai_struct_compiler#267
Assignees
@generalmimon
Labels
bug found

Comments

@GreyCat
Copy link
Member

GreyCat commented Dec 4, 2017

Moved from #1203, originally submitted by @ii8:

ks-opaque-types: true seems to have no effect whenever there is an imports directive, even if they are completely unrelated.

Example:

filea.ksy:

meta:
  id: filea
  ks-opaque-types: true
  imports:
    - fileb

seq:
  - id: a
    type: i_am_opaque

fileb.ksy:

meta:
  id: fileb

command:

λ /usr/local/Cellar/kaitai-struct-compiler/0.7/bin/kaitai-struct-compiler --target graphviz filea.ksy

/seq/0: unable to find type 'i_am_opaque', searching from filea
@GreyCat GreyCat mentioned this issue Dec 4, 2017
Closed
@KOLANICH
Copy link

Any progress with it? It affects npy format.

@generalmimon generalmimon self-assigned this Jun 11, 2020
@generalmimon generalmimon mentioned this issue Apr 22, 2021
Closed
@generalmimon generalmimon mentioned this issue Jul 22, 2022
Closed
@Puyodead1
Copy link

Seeing the same problem, is there any progress on this?

@KOLANICH
Copy link

KOLANICH commented Jan 3, 2023

@Puyodead1, it seems that opaque types specified in meta don't work without providing the CLI flag to the compiler.

generalmimon added a commit to kaitai-io/kaitai_struct_compiler that referenced this issue Mar 4, 2024
@generalmimon
Fix ks-opaque-types: true and duplicate warnings if imports are used
869f551 
Fixes kaitai-io/kaitai_struct#295

Fixes duplication of warnings and non-fatal errors as shown in
kaitai-io/kaitai_struct#1063

The existing code structure hinted that the `precompile(classSpecs:
ClassSpecs, topClass: ClassSpec)` overload should precompile only the
`topClass` specified, but its actual implementation for the most part
didn't use `topClass` at all and only passed `classSpecs` to all
precompilation steps, which processed all types in the given
`ClassSpecs`. Given that the `precompile(classSpecs: ClassSpecs,
topClass: ClassSpec)` overload is called from the `precompile(specs:
ClassSpecs)` overload for each type in `ClassSpecs`, this resulted in
unnecessary repeated precompilation of all types when imports are used
(because then there are more types in `ClassSpecs`).

If there are `N` top-level types in `ClassSpecs`, instead of running the
precompile phase only once for each top-level type, it was run `N` times
per each top-level type, resulting in `N**2` total precompilations of
top-level types. This is not only unnecessary, but it also had some
observable negative effects, e.g. a single warning being reported to the
console `N` times (and `N >= 2` any time you use imports) instead of
just once (see kaitai-io/kaitai_struct#295).

Another problem was caused by the only actual use of `topClass` in the
`precompile(classSpecs: ClassSpecs, topClass: ClassSpec)` overload,
namely for checking if opaque types are enabled or not (according to the
`/meta/ks-opaque-types` key) and passing the setting to the
`ResolveTypes` precompile step. This meant that if one spec had opaque
types enabled and the other disabled, they would be rejected even in the
spec where they should be enabled (because `ResolveTypes` processes all
specs first with opaque types enabled and then once more with them
disabled), as reported in
kaitai-io/kaitai_struct#295.

This commit ensures that each `ClassSpec` of `ClassSpecs` is precompiled
only once, eliminating both of the problems mentioned.
@generalmimon generalmimon mentioned this issue Mar 4, 2024
Merged
generalmimon added a commit to kaitai-io/kaitai_struct_compiler that referenced this issue Mar 4, 2024
@generalmimon
Fix ks-opaque-types: true and duplicate warnings if imports are used
143adc9 
Fixes kaitai-io/kaitai_struct#295

Fixes duplication of warnings and non-fatal errors as shown in
kaitai-io/kaitai_struct#1063

The existing code structure hinted that the `precompile(classSpecs:
ClassSpecs, topClass: ClassSpec)` overload should precompile only the
`topClass` specified, but its actual implementation for the most part
didn't use `topClass` at all and only passed `classSpecs` to all
precompilation steps, which processed all types in the given
`ClassSpecs`. Given that the `precompile(classSpecs: ClassSpecs,
topClass: ClassSpec)` overload is called from the `precompile(specs:
ClassSpecs)` overload for each type in `ClassSpecs`, this resulted in
unnecessary repeated precompilation of all types when imports are used
(because then there are more types in `ClassSpecs`).

If there are `N` top-level types in `ClassSpecs`, instead of running the
precompile phase only once for each top-level type, it was run `N` times
per each top-level type, resulting in `N**2` total precompilations of
top-level types. This is not only unnecessary, but it also had some
observable negative effects, e.g. a single warning being reported to the
console `N` times (and `N >= 2` any time you use imports) instead of
just once (see kaitai-io/kaitai_struct#295).

Another problem was caused by the only actual use of `topClass` in the
`precompile(classSpecs: ClassSpecs, topClass: ClassSpec)` overload,
namely for checking if opaque types are enabled or not (according to the
`/meta/ks-opaque-types` key) and passing the setting to the
`ResolveTypes` precompile step. This meant that if one spec had opaque
types enabled and the other disabled, they would be rejected even in the
spec where they should be enabled (because `ResolveTypes` processes all
specs first with opaque types enabled and then once more with them
disabled), as reported in
kaitai-io/kaitai_struct#295.

This commit ensures that each `ClassSpec` of `ClassSpecs` is precompiled
only once, eliminating both of the problems mentioned.
generalmimon added a commit to kaitai-io/kaitai_struct_compiler that referenced this issue Mar 7, 2024
@generalmimon
Fix ks-opaque-types: true and duplicate warnings if imports are used
e2dc7f5 
Fixes kaitai-io/kaitai_struct#295

Fixes duplication of warnings and non-fatal errors as shown in
kaitai-io/kaitai_struct#1063

The existing code structure hinted that the `precompile(classSpecs:
ClassSpecs, topClass: ClassSpec)` overload should precompile only the
`topClass` specified, but its actual implementation for the most part
didn't use `topClass` at all and only passed `classSpecs` to all
precompilation steps, which processed all types in the given
`ClassSpecs`. Given that the `precompile(classSpecs: ClassSpecs,
topClass: ClassSpec)` overload is called from the `precompile(specs:
ClassSpecs)` overload for each type in `ClassSpecs`, this resulted in
unnecessary repeated precompilation of all types when imports are used
(because then there is more than 1 type in `ClassSpecs`).

If there are `N` top-level types in `ClassSpecs`, instead of running the
precompile phase only once for each top-level type, it was run `N` times
per each top-level type, resulting in `N**2` total precompilations of
top-level types. This is not only unnecessary, but it also had some
observable negative effects, e.g. a single warning being reported to the
console `N` times (and `N >= 2` any time you use imports) instead of
just once (see kaitai-io/kaitai_struct#1063).

Another problem was caused by the only actual use of `topClass` in the
`precompile(classSpecs: ClassSpecs, topClass: ClassSpec)` overload,
namely for checking if opaque types are enabled or not (according to the
`/meta/ks-opaque-types` key) and passing the setting to the
`ResolveTypes` precompile step. This meant that if one spec had opaque
types enabled and the other disabled, they would be rejected even in the
spec where they should be enabled (because `ResolveTypes` processed all
specs first with opaque types enabled and then once more with them
disabled), as reported in
kaitai-io/kaitai_struct#295.

This commit ensures that each `ClassSpec` of `ClassSpecs` is precompiled
only once, eliminating both of the problems mentioned.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@generalmimon generalmimon

Labels
bug found
Projects
None yet
Milestone
No milestone
4 participants
@GreyCat @KOLANICH @Puyodead1 @generalmimon