Make the namespace resolution code closer to flatc

[stephanemagnenat]

Given the two files (available here):

math.fbs

namespace MyProject.Math;

struct Vector2 {
        x: float;
        y: float;
}

game.fbs

include "math.fbs";

namespace MyProject.Game;

table GameObject {
        position: Math.Vector2;
}

Using google's flatbuffers works as expected (using flatc version 2.0.6): flatc -o rust -r --gen-all game.fbs

However, planus is not able to resolve namespaces: planus check *.fbs

error: Unknown type
  ┌─ game.fbs:6:12
  │
6 │     position: Math.Vector2;
  │   

[TethysSvensson]

Honestly I am very surprising that this compiles using flatc. It certainly does not seem to work though. If I try to add game_generated.rs and math_generated.rs to a project, I get name resolution errors:

error[E0433]: failed to resolve: could not find `math` in `super`
  --> src/game_generated.rs:66:47
   |
66 |   pub fn position(&self) -> Option<&'a super::math::Vector2> {
   |                                               ^^^^ could not find `math` in `super`

[TethysSvensson]

Though perhaps that is a problem with flatc 22.11.23 specifically. I can try with a newer flatc and see what happens.

[TethysSvensson]

To use this with planus, replace position: Math.Vector2 with position: MyProject.Math.Vector2.

I have tried to use a newer flatc unsuccessfully with or without this replacement using the rust backend. As far as I can tell they don't support generating correct code for this type of nested namespaces.

[stephanemagnenat]

I will investigate more, we are using such nested namespaces for years on our project with both Typescript and Rust versions of flatbuffers.

[TethysSvensson]

Ahh, I missed the --gen-all flag, which seems to be required for this to work.

In any case, I am still very confused about why flatc would accept this.

The resolution code in planus is here. We currently only support two resolution modes: Relative to current namespace, and relative to the root namespace. I personally think this makes the most sense, but even so I think there is value in following the behavior of flatc.

I would definitely accept a patch to bring us closer to flatc, assuming the behavior of flatc is actually intentional and not just a bug.

[TethysSvensson]

@stephanemagnenat Feel free to ping my in our discord if you want to chat more about this.

[stephanemagnenat]

I did not find a specification about namespace resolution, and I cannot recall how I chose the model we use. However, a discussion in an issue seems to indicate that namespace resolution uses a notion of closest distance while walking namespaces backwards, "similar to how C++ works", which seems to indicate that our usage is "correct" and not the exploitation of a bug ;-)

[TethysSvensson]

Nice find!

Unfortunately I don't have the capacity to work on this myself, but I will definitely accept a patch to make our resolution code closer to the flatc behavior.

[stephanemagnenat]

Of course, that's kind of an arcane feature. For my experiments with planus, I will simplify our namespace structure.

[stephanemagnenat]

Maybe there could be a "conformity" tag for such issues?