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

Differences in accessing ENUM-IDs between Ruby and Java. #552

Closed
ams-tschoening opened this issue Apr 1, 2019 · 2 comments · Fixed by kaitai-io/kaitai_struct_compiler#159
Milestone

Comments

@ams-tschoening
Copy link

ams-tschoening commented Apr 1, 2019

I have multiple different types in multiple different KSY-files. One of those types is managing a lot of ENUMs and most of the users of those ENUMs are contained in that type, but not all to not bloat it too much. If other types need access to elements of some external ENUM, I'm currently using params to simply forward the IDs of these elements. In most cases the types are so specific, that only very few IDs to forward are enough already. This way I'm actually working around the limitation that KS can't handle ENUMS in different types yet.

That leads to code like the following:

params:
  - id:   oms_rec
    type: fmt_rec_oms
  - id:   fmt_oms_dll_prod_code_meh
    type: u4
  - id:   fmt_oms_dll_device_type_heat_cost_allocator
    type: u4

[...]

instances:
  is_possible_dev:
    value:  |
            (oms_rec.lid.prod_code.to_i     == fmt_oms_dll_prod_code_meh)                   and
            (oms_rec.lid.address.type.to_i  == fmt_oms_dll_device_type_heat_cost_allocator) and
            (oms_rec.lid.address.version    == 0x00)

That leads to working Java, but ksv breaks because of the following error message and generated Ruby:

[...]/fmt_oms_apl_data_tc_date_g_ams_bug_2543.rb:57:in `is_possible_dev': uninitialized constant FmtOmsAplDataTcDateGAmsBug2543::I__FMT_OMS_DLL_PROD_CODE (NameError)

def is_possible_dev
  return @is_possible_dev unless @is_possible_dev.nil?
  @is_possible_dev =  ((I__FMT_OMS_DLL_PROD_CODE[oms_rec.lid.prod_code] == fmt_oms_dll_prod_code_meh)[...]
  @is_possible_dev
end

The mentioned constant I__FMT_OMS_DLL_PROD_CODE is the reverse mapping of the ENUM I'm interested in and which is contained in the external type oms_rec. That type is properly imported in KSY as well, else things wouldn't compile most likely.

imports:
  - ../../../../../record/oms/fmt_rec_oms

But I don't see any require-statements or else in the generated ruby. Things are easy for Java because you model ENUMs as classes there which simply have the attribute id, so no need to import anything or cross classes using some static, class-level globals or such. But my KSY seems to be correct in the end, it only doesn't work because of differences in the generated code of target languages.

Any idea how to handle my use case?

@ams-tschoening
Copy link
Author

I think the best thing to do would be to change how Ruby handles ENUMs and make them work exactly as those generated for Java. Currently, two low-level hashes are used per ENUM, one mapping decimals to symbols and one providing the inverse of the first. The problem with that approach is that those hashes don't provide any methods, so anyone using either of both needs to use them bei their name and that breaks if borders of classes are crossed. If some ENUM would be some object instead, one could simply access its methods like in Java:

boolean _tmp = (boolean) ( ((omsRec().lid().prodCode().id() == fmtOmsDllProdCodeMeh())[...]

As Ruby seems to support nested classes, that shouldn't be too difficult to achieve? I guess the two hashes currently used individually should only be moved to some inner class and methods added that do what directly accessing the ENUM-hashes did currently.

Should even all target languages implement ENUMs as objects only to avoid differences like I have with Java vs. Ruby? Looks like things are handled very differently depending on the target currently.

I'm still looking for other alternatives a bit as well.

@ams-tschoening
Copy link
Author

It seems I have an easier workaround, qualifying the used constant with its parent class seems to be enough already:

@is_possible_dev = ((I__FMT_OMS_DLL_PROD_CODE[oms_rec.lid.prod_code] == fmt_oms_dll_prod_code_meh)[...]

vs.

@is_possible_dev = ((FmtRecOms::I__FMT_OMS_DLL_PROD_CODE[oms_rec.lid.prod_code] == fmt_oms_dll_prod_code_meh)[...]

Seems the following lines in the compiler need to be changed to fully qualify the ENUM with the first pieces of name:

  override def enumToInt(v: Ast.expr, et: EnumType): String =
    s"${RubyCompiler.inverseEnumName(et.name.last.toUpperCase)}[${translate(v)}]"

I'll try to provide a PR tomorrow.

That raises another question I already had in mind: How is requiring classes from other files handled in Ruby? The above most likely only works because ksv does that on its own after parsing. I don't see any other require than the following in all my generated Ruby-files:

require 'kaitai/struct/struct'

FmtRecOms should be handled using importList like in Java, shouldn't it? Or can this be ignored or am I understanding things wrong?

ams-tschoening added a commit to ams-ts-kaitai/tests that referenced this issue Apr 4, 2019
ams-tschoening added a commit to ams-ts-kaitai/tests that referenced this issue Apr 4, 2019
ams-tschoening added a commit to ams-ts-kaitai/compiler that referenced this issue Apr 5, 2019
"enumToInt" didn't take the fully qualified path of an ENUM into account when rendering usage of the associated inverse map. If that map needed to be accessed in some external KSY compared to where the map is hosted, things failed because the map was wrongly assumed to be in the KSY where it is used. This commit changes "I__ENUM" to "Class::I__ENUM" following what has been available already as "enumDirectMap".

This fixes kaitai-io/kaitai_struct#552.
ams-tschoening added a commit to ams-ts-kaitai/compiler that referenced this issue Apr 5, 2019
"enumToInt" didn't take the fully qualified path of an ENUM into account when rendering usage of the associated inverse map. If that map needed to be accessed in some external KSY compared to where the map is hosted, things failed because the map was wrongly assumed to be in the KSY where it is used. This commit changes "I__ENUM" to "Class::I__ENUM" following what has been available already as "enumDirectMap".

This fixes kaitai-io/kaitai_struct#552.
@generalmimon generalmimon added this to the v0.9 milestone Aug 24, 2020
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