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.

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

Trying to understand enum resolution #1028

Open
wader opened this issue Apr 15, 2023 · 9 comments · May be fixed by kaitai-io/kaitai_struct_compiler#309
Open

Trying to understand enum resolution #1028

wader opened this issue Apr 15, 2023 · 9 comments · May be fixed by kaitai-io/kaitai_struct_compiler#309

Comments

@wader
Copy link

wader commented Apr 15, 2023

Hello, i'm trying to understand how enum:s are resolved. I assumed first that enum were resolve something like this:

  • If no namespace, just a name, look in current types enums
  • If namespace, has ::, look up type first then look in enums.

But when i trying some thing in webide and with ksdump things got confusing, for example:

meta:
  id: enum_resolve

seq:
  - id: s0
    type: u1
    enum: e
  - id: s1
    type: u1
    enum: a::e
  - id: s2
    type: u1
    enum: a::a::e
  - id: s3
    type: u1
    enum: b::e
  - id: s4
    type: u1
    enum: b::b::e
  - id: s5
    type: u1
    enum: a::b::e
  - id: s6
    type: u1
    enum: b::a::e

types:
  a:
    enums:
      e:
        0: zero2
  b: {}

enums:
  e:
    0: zero

Both ksdump and webide result in:

$ docker run -v "$PWD:/share" -i --entrypoint=ksdump kaitai/ksv -f json /dev/zero enum_resolve.ksy
{
  "s0": "e_zero",
  "s1": "e_zero2",
  "s2": "e_zero2",
  "s3": "e_zero",
  "s4": "e_zero",
  "s5": "e_zero",
  "s6": "e_zero2"
}

So seems to behave something like this: If last part(s?) of type namespace resolve to a type that has the enum then use it, otherwise ignore type namespace and just look in current types enums.

@generalmimon
Copy link
Member

@wader Thanks for sharing this. I agree that the behavior you demonstrate is very weird, but I'd say that the expected behavior is simple - only the s0 and s1 fields are valid. If you compile your spec to a statically-typed language like Java, you'll find that only s0 and s1 work (unsurprisingly):

image

It's a very good observation that the compiler doesn't flag any of the incorrect enum references as invalid and reject the .ksy spec - this is clearly a bug. Unfortunately, the compiler is generally notorious for handling error cases poorly.

@wader
Copy link
Author

wader commented Apr 16, 2023

Ok thanks! and good tip to try compile to some typed language as a test. So as i understand it now the enum resolution is something like this:

@generalmimon
Copy link
Member

generalmimon commented Apr 16, 2023

@wader:

So as i understand it now the enum resolution is something like this: (...)

Yes, looks good. Just noting that "look for enum" here:

If there is a type namespace:

  • Use type resolution (...) and then look for enum

should be trivial (no fallbacks, only the direct enums section should be accessed) - same as when resolving type paths (#1019 (comment)):

Now when you know what type: c corresponds to, type: c::d is easy. Given that type: c has been resolved as /types/c, the only valid location for type: c::d is /types/c/types/d. If d is not there, game over - the spec is invalid.

(for enum, if the c type in enum: c::e is /types/c, the e enum has to be in /types/c/enums/e). But I think you get it by now.

@wader
Copy link
Author

wader commented Apr 23, 2023

Close this one and possibly split out the weirdness/bug into new issue?

@generalmimon
Copy link
Member

generalmimon commented Apr 23, 2023

@wader:

possibly split out the weirdness/bug into new issue?

Yeah, makes sense.

After seeing this - #1019 (comment), I realized that the KSC's component for resolving nested type references apparently misbehaves similarly as the component for resolving nested enums, and the faulty behavior you demonstrated on enum references (#1028 (comment)) most likely applies for nested type refs too.

@Mingun
Copy link

Mingun commented Sep 24, 2024

This is the same error as #786, but for enums, and when enum is specified with path it is full duplicate of #786. The recursive search of enum in parent types here:
https://github.com/kaitai-io/kaitai_struct_compiler/blob/c23ec2ca88d84042edba76f70c1f003d062b7585/shared/src/main/scala/io/kaitai/struct/ClassTypeProvider.scala#L96-L108
should be performed only when path to the type is not specified, otherwise enum should be taken only from specified type.

In the original example currently enums resolved as following:

meta:
  id: enum_resolve

seq:
  - id: s0
    type: u1
    enum: e # ::e
  - id: s1
    type: u1
    enum: a::e # ::a::e
  - id: s2
    type: u1
    enum: a::a::e # ::a::e
  - id: s3
    type: u1
    enum: b::e # ::e
  - id: s4
    type: u1
    enum: b::b::e # ::e
  - id: s5
    type: u1
    enum: a::b::e # ::e
  - id: s6
    type: u1
    enum: b::a::e # ::a::e

types:
  a:
    enums:
      e:
        0: zero2
        0x89: v2_x89
        0x50: v2_x50
        0x4e: v2_x4e
        0x47: v2_x47
        0x0d: v2_x0d
        0x0a: v2_x0a
        0x1a: v2_x1a
  b: {}

enums:
  e:
    0: zero
    0x89: v_x89
    0x50: v_x50
    0x4e: v_x4e
    0x47: v_x47
    0x0d: v_x0d
    0x0a: v_x0a
    0x1a: v_x1a

@generalmimon
Copy link
Member

@Mingun:

This is the same error as #786, but for enums, and when enum is specified with path it is full duplicate of #786.

This issue is not "full duplicate of #786". It's related, sure. But enums != types. I think it's perfectly possible to fix #786 and leave this one unresolved. So we should keep both issues open to make sure that we fix and verify both cases.

@generalmimon
Copy link
Member

generalmimon commented Sep 24, 2024

@Mingun:

In the original example currently enums resolved as following:

I don't see what this contributes to this issue. It's the same example, with a bunch of extra enum members for some reason. We can already see clear enough what enums the compiler resolves the type paths into in the initial post #1028 (comment) because of the e_zero and e_zero2 enum values in the ksdump output.

And the "absolute paths" that you added as comments don't make sense. You're making it look like /enums/e and /types/a are elements at the root level when they're really nested into the root type enum_resolve - their "absolute paths" would be ::enum_resolve::e and ::enum_resolve::a. Needless to say that this syntax of absolute paths is theoretical and should be seen as pseudo-code - it may very well not work in practice, because we've never tested it, as I mentioned in #786 (comment).

@Mingun
Copy link

Mingun commented Sep 24, 2024

@generalmimon, it is full duplicate when enum specified with a path. Because in that case enum is fixed in concrete type and how it would be resolved completely determined by the type resolution which suffer from #786.

And the "absolute paths" that you added as comments don't make sense.

Original description hard to understand without visualization. I've only added comments which shows to what enums paths currently resolves. They do not pretend to be valid Kaitai syntax.

Mingun added a commit to Mingun/kaitai_struct_compiler that referenced this issue Sep 26, 2024
Mingun added a commit to Mingun/kaitai_struct_compiler that referenced this issue Sep 26, 2024
This commit also unify enum resolution in expression language and in `enum` keys

Fixes kaitai-io/kaitai_struct#1028
Mingun added a commit to Mingun/kaitai_struct_compiler that referenced this issue Oct 4, 2024
Mingun added a commit to Mingun/kaitai_struct_compiler that referenced this issue Oct 4, 2024
This commit also unify enum resolution in expression language and in `enum` keys

Fixes kaitai-io/kaitai_struct#1028
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants