Access to foreign types using :: syntax

[KOLANICH]

For now we cannot access foreign types using :: syntax. It is bad:
1 We cannot access foreign types enums which makes us to lift enum definition into upper scope. This is needed for example when iterating and a specific enum value means end of iteration.
2 We cannot access foreign types members. It's the most critical when we cannot access subtypes of imported types.

So I propose to add a C++-like syntax to access types and enums of types.

[ams-tschoening]

I have the following KSY:

meta:
  id:      fmt_oms_apl_error
  endian:  le
  imports:
    - ../fmt_oms_rec

seq:
  [...]    
  - id:   dyn_app_error
    type: fmt_oms_rec::fmt_oms_apl_vdb
    if:   (not _io.eof) and (value == 0xF0)

which results in the following Java-snippet:

[...]
private void _read() {
    if (!(_io().isEof())) {
        this.value = this._io.readU1();
    }
    if ( ((!(_io().isEof())) && (value() == 240)) ) {
        this.dynAppError = new FmtOmsRec.FmtOmsAplVdb(this._io);
    }
}
[....]
private FmtOmsAplVdb dynAppError;
public FmtOmsAplVdb dynAppError() { return dynAppError; }

As you can see, KS successfully compiles a type using :: and handles the call to new correctly already. The only thing missing is the variable declaration. Can this be easily fixed, any hints where I need to look at?

[GreyCat]

Declarations and getters (they're called "readers" in the code) are done around here.

As you can see, that in turn passes the torch to either kaitaiType2JavaTypePrim or kaitaiType2JavaTypeBoxed, which, in their turn, invoke type2class and types2class, i.e. in a slightly different ways (in prim),
in boxed).

Most likely for Java we could do that by replacing that with types2class everywhere, and that should work as Java compiler is intelligent enough to resolve the proper class for us using these specifications. For other languages, we'd need to get slightly more intelligent, i.e. by using pre-resolved t.classSpec reference, not just t.name as a token.

[ams-tschoening]

I've followed your suggestion and got JavaCompiler to output the properly referenced type. ResolveTypes.realResolveUserType needed to be adjusted a bit to recursively check top-level classes as well. Now I have the problem that using the examples above I get the following line:

this.dynAppError = new FmtOmsRec.FmtOmsAplVdb(this._io, this, _root);

Notice the changed CTOR, which I guess is because the type is properly resolved and not opaque anymore. The problem is that because that type comes from another type, its CTOR looks like the following:

public FmtOmsAplVdb(KaitaiStream _io, KaitaiStruct _parent, FmtOmsRec _root) {

FmtOmsRec is not available for the first line, because the type of the first line is an individual KSY, so _root is of type FmtOmsAplError instead. I think the problem is most likely in JavaCompiler, which assumes either an opaque type or the 3 args CTOR:

  case t: UserType =>
    val addArgs = if (t.isOpaque) {
      ""
    } else {
      val parent = t.forcedParent match {
        case Some(USER_TYPE_NO_PARENT) => "null"
        case Some(fp) => translator.translate(fp)
        case None => "this"
      }
      val addEndian = t.classSpec.get.meta.endian match {
        case Some(InheritedEndian) => ", _is_le"
        case _ => ""
      }
      s", $parent, _root$addEndian"
    }
    val addParams = Utils.join(t.args.map((a) => translator.translate(a)), ", ", ", ", "")
    s"new ${types2class(t.name)}($io$addArgs$addParams)"

What would be the proper behaviour, which CTOR should be used and how do I achieve that? To start things, I guess dealing with the invocation as if it was an opaque type again would be a good idea. But how do I check the context I have, that a type is used in some external KSY?

I even thought of using params in the external KSY to get an instance of FmtOmsRec, but I' cant forward that as _root, only as additional argument to the CTOR.

[GreyCat]

I guess this part needs some adjustment as well. The first thing that comes to mind is that it would be probably the right way out would be to generate ctor invocation like that in this case:

this.dynAppError = new FmtOmsRec.FmtOmsAplVdb(this._io, this);

which would effectively make new root for FmtOmsRec, with root pointing to itself. You're completely right, this case t: UserType => is what needs to be fixed to accomodate such a possibility.

However, things get slightly messier here, because one actually has to support more than 3 arg ctors here, i.e. for stuff like parameters and calculated endianness. This trip into a rabbit hole could be slightly more complicated than expected originally. As the very least, we'll need several new tests for that:

• invocation of externally declared type
• invocation of externally declared type, passing parameters
• invocation of externally declared type, passing default endianness

[ams-tschoening]

With your example CTOR invocation, _parent is crossing KSY-borders and didn't you say you don't want that or is that only meant for _root? Might not be a good idea to handle both differently.

I'm not sure how much time I can spend on this anymore. Is there any interest to get my two changes so far as PR?

[GreyCat]

With your example CTOR invocation, _parent is crossing KSY-borders

Yeah, and that things actually bothers me as well. The problem is that right now parent is defined to point to this for root object, and thus "parent" is effectively unusable for root objects. Probably it's better to be honest here and make it somehow disappear at all — but that would, in turn, require some distinction between "root types" and "non-root types"... OTOH, it kind of makes sense, as _root should be handled differently for root types too, as there's no point to store it, for example. One can just declare _root() as return this;...

Is there any interest to get my two changes so far as PR?

Sure :)

[ams-tschoening]

I would like to additionally mention something I just recognized in my use case, because I have 4 types in 2 KSY files currently using each other:

Type1.ksy -> Type1.1 -> Type1.1.1
          -> Type2
Type2.ksy -> Type1.1 -> Type1.1.1

Type1.ksy contains and uses all subtypes starting with 1., while Type2.ksy doesn't contain any subtypes, but uses those of Type1.ksy. The problem now is that Type1.1.1 accesses Type1 using _root, which is not possible if used by Type2 anymore. Depending on how CTORs are created and called, this might only break on runtime instead of compile time.

this.dynAppError = new FmtOmsRec.FmtOmsAplVdb(this._io, this, _root);

Breaks on compile time, but might be logically wrong and might get fixed, which most likely leads to something like the following, breaking on runtime, because _root is null:

this.dynAppError = new FmtOmsRec.FmtOmsAplVdb(this._io[, this]);

To solve the problem on runtime in Type2, I would need to add a params declaration forwarding the logical root Type1, which is actually the same as _root already. But I'm unable to specify this, because _root and params are handled independently. Additionally, I need to rework access to _root in Type1.1.1 to access the additionally provided params.

this.dynAppError = new FmtOmsRec.FmtOmsAplVdb(this._io[, this], logicalRootType1);

Maybe I'm missing some important discussion, but aren't _parent and _root creating problems here which we wouldn't have without those? Now that we have params, wouldn't it be better to explicitly define if types use _parent and _root using those? This way KSY-authors could opt-in into having those, those would need to be assigned a type, so no wrong invocations for _parent because it's KaitaiStruct and matches everything, and there wouldn't be a problem with params allowed to cross KSY-borders while comparable _parent and _root are not.

Actually, I'm forwarding my logical root using params already more often than really wanted because the data I'm working on does contain a lot of errors which I fix at runtime, but to decide things I need the logical root at some places. So in my case, I need to deal now with NOT providing _root while DO providing the same thing as some logical root instead using params. Would be easier if I would only need to use params instead.

I saw things like USER_TYPE_NO_PARENT somewhere as well, which I didn't fully understand, but it sounds like those concepts would not be needed anymore as well if _parent is an opt-in-param.

For backwards compatibility, one could create some meta-key to switch between implicit _parent and _root per KSY and if those are missing, handle them like currently. I would switch that off for my KSYs and define params where I need them.

[GreyCat]

I've added nested_types3 test that demonstrates and tests new syntax. Hopefully, it would work already for Java, JS, Python, Ruby, C++, and hopefully we'll get it working for the rest of the languages soon.

@tschoening I'll try to reply tomorrow, sorry for it taking so long.

[ams-tschoening]

Guten Tag Mikhail Yakshin, am Freitag, 16. März 2018 um 03:38 schrieben Sie:

@tschoening I'll try to reply tomorrow, sorry for it taking so long.

No problem, take your time, at least I am not forced to wait for KS enhancements on this topic currently. Mit freundlichen Grüßen, Thorsten Schöning

…

-- Thorsten Schöning E-Mail: [email protected] AM-SoFT IT-Systeme http://www.AM-SoFT.de/ Telefon...........05151- 9468- 55 Fax...............05151- 9468- 88 Mobil..............0178-8 9468- 04 AM-SoFT GmbH IT-Systeme, Brandenburger Str. 7c, 31789 Hameln AG Hannover HRB 207 694 - Geschäftsführer: Andreas Muchow

[lrodri29]

Has this been addressed by the Kaitai team?

[GreyCat]

@lrodri29 Yes, this should be implemented by now. This ticket is still pending a reply to Thorsten Schöning, so this is why it's still not closed.

[generalmimon]

@lrodri29 Yes, this should be implemented by now. This ticket is still pending a reply to Thorsten Schöning, so this is why it's still not closed.

As it doesn't make sense to keep this issue open just because we're waiting for a reply to this comment by @ams-tschoening, which is off-topic in this issue, I transferred it into a new issue #770. Please continue the discussion there, if you want to comment on that topic.

According to @GreyCat, this issue is resolved, so I'm closing it.

Note: we've discussed this matter on Gitter with @GreyCat.