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

Nested type paths starting with the top-level type don't work correctly #1032

Open
generalmimon opened this issue May 14, 2023 · 0 comments
Open

Nested type paths starting with the top-level type don't work correctly #1032

generalmimon opened this issue May 14, 2023 · 0 comments
Labels
bug found

Comments

@generalmimon
Copy link
Member

In #1019 (comment), I mentioned that you should be able to qualify a type reference with the name of the top-level ("root") type:

meta:
  id: nested_types_paths
seq:
  - id: obj_a
    type: a
types:
  a:
    seq:
      - id: obj_cd
        type: nested_types_paths::c::d

  c:
    seq: []
    types:
      d:
        seq:
          - id: d_attr
            type: s1

It's not often done because it's redundant in this case (over just type: c::d), but there's no reason why it shouldn't work.

However, some parts of the compiler apparently don't handle this properly. If we compile these two .ksy specs:

--- 1/nested_types_paths_rel.ksy
+++ 2/nested_types_paths_abs.ksy
@@ -7,7 +7,7 @@ types:
   a:
     seq:
       - id: obj_cd
-        type: c::d
+        type: nested_types_paths::c::d
 
   c:
     seq: []
kaitai-struct-compiler-0.10 -- --verbose file -t all -d outdir-rel nested_types_paths_rel.ksy
kaitai-struct-compiler-0.10 -- --verbose file -t all -d outdir-abs nested_types_paths_abs.ksy

and compare the generated modules, only the type reference should be different, if anything. Unfortunately, a few more incorrect changes occur:

  1. In some target languages, the generated code refers to the top-level type itself, without actually resolving the full type path (note that the snippets below are not full diffs, only cherry-picked relevant parts):

    Construct 
    --- 1/outdir-rel/construct/nested_types_paths.py
+++ 2/outdir-abs/construct/nested_types_paths.py
@@ -2,7 +2,7 @@ from construct import *
 from construct.lib import *
 
 nested_types_paths__a = Struct(
-	'obj_cd' / LazyBound(lambda: nested_types_paths__c__d),
+	'obj_cd' / LazyBound(lambda: nested_types_paths),
 )
 
 nested_types_paths__c__d = Struct(
    Go 
    --- 1/outdir-rel/go/src/nested_types_paths.go
+++ 2/outdir-abs/go/src/nested_types_paths.go
@@ -27,7 +27,7 @@ func (this *NestedTypesPaths) Read(io *kaitai.Stream, parent interface{}, root *
 	return err
 }
 type NestedTypesPaths_A struct {
-	ObjCd *NestedTypesPaths_C_D
+	ObjCd *NestedTypesPaths
 	_io *kaitai.Stream
 	_root *NestedTypesPaths
 	_parent *NestedTypesPaths
@@ -42,8 +42,8 @@ func (this *NestedTypesPaths_A) Read(io *kaitai.Stream, parent *NestedTypesPaths
 	this._parent = parent
 	this._root = root
 
-	tmp2 := NewNestedTypesPaths_C_D()
-	err = tmp2.Read(this._io, this, this._root)
+	tmp2 := NewNestedTypesPaths()
+	err = tmp2.Read(this._io, this, nil)
 	if err != nil {
 		return err
 	}
    Lua 
    --- 1/outdir-rel/lua/nested_types_paths.lua
+++ 2/outdir-abs/lua/nested_types_paths.lua
@@ -29,7 +29,7 @@ function NestedTypesPaths.A:_init(io, parent, root)
 end
 
 function NestedTypesPaths.A:_read()
-  self.obj_cd = NestedTypesPaths.C.D(self._io, self, self._root)
+  self.obj_cd = NestedTypesPaths(self._io)
 end
    Nim 
    --- 1/outdir-rel/nim/nested_types_paths.nim
+++ 2/outdir-abs/nim/nested_types_paths.nim
@@ -42,7 +42,7 @@ proc read*(_: typedesc[NestedTypesPaths_A], io: KaitaiStream, root: KaitaiStruct
   this.root = root
   this.parent = parent
 
-  let objCdExpr = NestedTypesPaths_C_D.read(this.io, this.root, this)
+  let objCdExpr = NestedTypesPaths.read(this.io, this.root, this)
   this.objCd = objCdExpr
 
 proc fromFile*(_: typedesc[NestedTypesPaths_A], filename: string): NestedTypesPaths_A =
    Perl 
    --- 1/outdir-rel/perl/NestedTypesPaths.pm
+++ 2/outdir-abs/perl/NestedTypesPaths.pm
@@ -72,7 +72,7 @@ sub new {
 sub _read {
     my ($self) = @_;
 
-    $self->{obj_cd} = NestedTypesPaths::C::D->new($self->{_io}, $self, $self->{_root});
+    $self->{obj_cd} = NestedTypesPaths->new($self->{_io});
 }
 
 sub obj_cd {
    PHP 
    --- 1/outdir-rel/php/NestedTypesPaths.php
+++ 2/outdir-abs/php/NestedTypesPaths.php
@@ -24,7 +24,7 @@ namespace NestedTypesPaths {
         }
 
         private function _read() {
-            $this->_m_objCd = new \NestedTypesPaths\C\D($this->_io, $this, $this->_root);
+            $this->_m_objCd = new \NestedTypesPaths($this->_io);
         }
         protected $_m_objCd;
         public function objCd() { return $this->_m_objCd; }
    Python 
    --- 1/outdir-rel/python/nested_types_paths.py
+++ 2/outdir-abs/python/nested_types_paths.py
@@ -25,7 +25,7 @@ class NestedTypesPaths(KaitaiStruct):
             self._read()
 
         def _read(self):
-            self.obj_cd = NestedTypesPaths.C.D(self._io, self, self._root)
+            self.obj_cd = NestedTypesPaths(self._io)
 
 
     class C(KaitaiStruct):

    However, in other languages, it works as expected:

    C++ 
    --- 1/outdir-rel/cpp_stl_11/nested_types_paths.cpp
+++ 2/outdir-abs/cpp_stl_11/nested_types_paths.cpp
@@ -28,7 +28,7 @@ nested_types_paths_t::a_t::a_t(kaitai::kstream* p__io, nested_types_paths_t* p__
 }
 
 void nested_types_paths_t::a_t::_read() {
-    m_obj_cd = std::unique_ptr<c_t::d_t>(new c_t::d_t(m__io, this, m__root));
+    m_obj_cd = std::unique_ptr<nested_types_paths_t::c_t::d_t>(new nested_types_paths_t::c_t::d_t(m__io));
 }
 
 nested_types_paths_t::a_t::~a_t() {
    C# 
    --- 1/outdir-rel/csharp/NestedTypesPaths.cs
+++ 2/outdir-abs/csharp/NestedTypesPaths.cs
@@ -36,12 +36,12 @@ namespace Kaitai
             }
             private void _read()
             {
-                _objCd = new C.D(m_io, this, m_root);
+                _objCd = new NestedTypesPaths.C.D(m_io);
             }
    Java 
    --- 1/outdir-rel/java/src/NestedTypesPaths.java
+++ 2/outdir-abs/java/src/NestedTypesPaths.java
@@ -47,12 +47,12 @@ public class NestedTypesPaths extends KaitaiStruct {
             _read();
         }
         private void _read() {
-            this.objCd = new C.D(this._io, this, _root);
+            this.objCd = new NestedTypesPaths.C.D(this._io);
         }
    JavaScript 
    --- 1/outdir-rel/javascript/NestedTypesPaths.js
+++ 2/outdir-abs/javascript/NestedTypesPaths.js
@@ -30,7 +30,7 @@ var NestedTypesPaths = (function() {
       this._read();
     }
     A.prototype._read = function() {
-      this.objCd = new C.D(this._io, this, this._root);
+      this.objCd = new NestedTypesPaths.C.D(this._io, this, null);
     }
 
     return A;
    Ruby 
    --- 1/outdir-rel/ruby/nested_types_paths.rb
+++ 2/outdir-abs/ruby/nested_types_paths.rb
@@ -23,7 +23,7 @@ class NestedTypesPaths < Kaitai::Struct::Struct
     end
 
     def _read
-      @obj_cd = C::D.new(@_io, self, @_root)
+      @obj_cd = NestedTypesPaths::C::D.new(@_io)
       self
     end
     attr_reader :obj_cd

  2. In all typed languages, the _parent type of the nested_types_paths::c::d class changes from nested_types_paths::a to a generic KaitaiStruct (again, here are only cherry-picked parts of the full diffs):

    C++ 
    diff --git 1/outdir-rel/cpp_stl_11/nested_types_paths.cpp 2/outdir-abs/cpp_stl_11/nested_types_paths.cpp
index 6dc0068..6c22f3c 100644
--- 1/outdir-rel/cpp_stl_11/nested_types_paths.cpp
+++ 2/outdir-abs/cpp_stl_11/nested_types_paths.cpp
@@ -54,7 +54,7 @@ nested_types_paths_t::c_t::~c_t() {
 void nested_types_paths_t::c_t::_clean_up() {
 }
 
-nested_types_paths_t::c_t::d_t::d_t(kaitai::kstream* p__io, nested_types_paths_t::a_t* p__parent, nested_types_paths_t* p__root) : kaitai::kstruct(p__io) {
+nested_types_paths_t::c_t::d_t::d_t(kaitai::kstream* p__io, kaitai::kstruct* p__parent, nested_types_paths_t* p__root) : kaitai::kstruct(p__io) {
     m__parent = p__parent;
     m__root = p__root;
     _read();
    C# 
    diff --git 1/outdir-rel/csharp/NestedTypesPaths.cs 2/outdir-abs/csharp/NestedTypesPaths.cs
index d982bf7..5bc0453 100644
--- 1/outdir-rel/csharp/NestedTypesPaths.cs
+++ 2/outdir-abs/csharp/NestedTypesPaths.cs
@@ -68,7 +68,7 @@ namespace Kaitai
                     return new D(new KaitaiStream(fileName));
                 }
 
-                public D(KaitaiStream p__io, NestedTypesPaths.A p__parent = null, NestedTypesPaths p__root = null) : base(p__io)
+                public D(KaitaiStream p__io, KaitaiStruct p__parent = null, NestedTypesPaths p__root = null) : base(p__io)
                 {
                     m_parent = p__parent;
                     m_root = p__root;
    Go 
    diff --git 1/outdir-rel/go/src/nested_types_paths.go 2/outdir-abs/go/src/nested_types_paths.go
index d28b22c..7318649 100644
--- 1/outdir-rel/go/src/nested_types_paths.go
+++ 2/outdir-abs/go/src/nested_types_paths.go
@@ -71,14 +71,14 @@ type NestedTypesPaths_C_D struct {
 	DAttr int8
 	_io *kaitai.Stream
 	_root *NestedTypesPaths
-	_parent *NestedTypesPaths_A
+	_parent interface{}
 }
 func NewNestedTypesPaths_C_D() *NestedTypesPaths_C_D {
 	return &NestedTypesPaths_C_D{
 	}
 }
 
-func (this *NestedTypesPaths_C_D) Read(io *kaitai.Stream, parent *NestedTypesPaths_A, root *NestedTypesPaths) (err error) {
+func (this *NestedTypesPaths_C_D) Read(io *kaitai.Stream, parent interface{}, root *NestedTypesPaths) (err error) {
 	this._io = io
 	this._parent = parent
 	this._root = root
    Java 
    diff --git 1/outdir-rel/java/src/NestedTypesPaths.java 2/outdir-abs/java/src/NestedTypesPaths.java
index 737fed9..ead5f18 100644
--- 1/outdir-rel/java/src/NestedTypesPaths.java
+++ 2/outdir-abs/java/src/NestedTypesPaths.java
@@ -86,11 +86,11 @@ public class NestedTypesPaths extends KaitaiStruct {
                 this(_io, null, null);
             }
 
-            public D(KaitaiStream _io, NestedTypesPaths.A _parent) {
+            public D(KaitaiStream _io, KaitaiStruct _parent) {
                 this(_io, _parent, null);
             }
 
-            public D(KaitaiStream _io, NestedTypesPaths.A _parent, NestedTypesPaths _root) {
+            public D(KaitaiStream _io, KaitaiStruct _parent, NestedTypesPaths _root) {
                 super(_io);
                 this._parent = _parent;
                 this._root = _root;
    Nim 
    diff --git 1/outdir-rel/nim/nested_types_paths.nim 2/outdir-abs/nim/nested_types_paths.nim
index 20e42e9..169ae07 100644
--- 1/outdir-rel/nim/nested_types_paths.nim
+++ 2/outdir-abs/nim/nested_types_paths.nim
@@ -60,7 +60,7 @@ proc read*(_: typedesc[NestedTypesPaths_C], io: KaitaiStream, root: KaitaiStruct
 proc fromFile*(_: typedesc[NestedTypesPaths_C], filename: string): NestedTypesPaths_C =
   NestedTypesPaths_C.read(newKaitaiFileStream(filename), nil, nil)
 
-proc read*(_: typedesc[NestedTypesPaths_C_D], io: KaitaiStream, root: KaitaiStruct, parent: NestedTypesPaths_A): NestedTypesPaths_C_D =
+proc read*(_: typedesc[NestedTypesPaths_C_D], io: KaitaiStream, root: KaitaiStruct, parent: KaitaiStruct): NestedTypesPaths_C_D =
   template this: untyped = result
   this = new(NestedTypesPaths_C_D)
   let root = if root == nil: cast[NestedTypesPaths](this) else: cast[NestedTypesPaths](root)
    PHP 
    diff --git 1/outdir-rel/php/NestedTypesPaths.php 2/outdir-abs/php/NestedTypesPaths.php
index 55382ec..dae0bcd 100644
--- 1/outdir-rel/php/NestedTypesPaths.php
+++ 2/outdir-abs/php/NestedTypesPaths.php
@@ -45,7 +45,7 @@ namespace NestedTypesPaths {
 
 namespace NestedTypesPaths\C {
     class D extends \Kaitai\Struct\Struct {
-        public function __construct(\Kaitai\Struct\Stream $_io, \NestedTypesPaths\A $_parent = null, \NestedTypesPaths $_root = null) {
+        public function __construct(\Kaitai\Struct\Stream $_io, \Kaitai\Struct\Struct $_parent = null, \NestedTypesPaths $_root = null) {
             parent::__construct($_io, $_parent, $_root);
             $this->_read();
         }

On top of that, as @Mingun discovered in #921, KSC doesn't validate a nested type reference that begins with the root type and happily accepts type paths like type: nested_types_paths::xxx_does_not_exist without raising a compile error.
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
No milestone
Development

No branches or pull requests
1 participant
@generalmimon