asn-compiler: Getting started with Tags resolution (#75)

[gabhijit]

Added Asn1ResolvedTag to a base type as an Option. In the 'resolve' value of each ingeger etc, we'll be resolving the actual tag if present (or the module level support suggests IMPLICIT/EXPLICIT tags etc.

Right now just basic structure definitions etc.

[xpromache]

Hey, I'm back a little bit on the CCSDS-SLE thing, and I was looking to complete the BER encoding.
I'm looking to continue with your tag-resolution branch and have few comments:

1. I think we can make the tag (Asn1ResolvedTag) mandatory rather than optional in the various types. From what I understand (but I admit my knowledge of ASN.1 is rather limited) the tags exist no matter if you use BER encoding or not.

2. The existing Asn1ResolvedTag misses the explicit/implicit mode which is important for the BER encoding.

3. On the other hand (but I'm not sure about this one) the current Asn1ResolvedTag has the class (Universal/Application,..) which I'm not sure it's so important; once resolved we can encode it directly into the Asn1ResolvedTag::num.

4. looking at the SEQUENCE and CHOICE referencing other types - their components can specify tag definition overriding the one from the referenced type. I think they can also specify constraints.

For example in this definition:

MyInt ::= [8] EXPLICIT INTEGER

            Rec1 ::= SEQUENCE {
                age      [10] MyInt 
            }
            Rec2 ::= SEQUENCE {
                age      MyInt 
            }

Rec1.age has the tag 10 whereas the Rec2.age has the tag 8 coming from MyInt. In the currently generated code, both those sequences make reference to the MyInt type which has tag 8.

Not sure how you want to go about this one:
a. either duplicating the type: instead of generating Asn1ResolvedType::Reference() we can generate Asn1ResolvedType::Base or Asn1ResolvedType::Constructed with the type info duplicated
b. or adding the tag (and possibly the constraint) as part of the Asn1ResolvedType::Reference enum variant.

[xpromache]

Answering my own points:
Asn1ResolvedTag - we can keep the class and add the mode, it's simpler like that:

pub(crate) struct Asn1ResolvedTag {
    num: u32,
    class: Asn1TagClass,
    mode: Asn1TagMode,
}

For the point of overridden definitions it does not make sense to duplicate the type because in our Rust generated code we don't want to end up with two definitions of MyInt. So we can add the tag into the Asn1ResolvedType::Reference().
What do you think?

[gabhijit]

@xpromache : Sorry to get back late to you about this (traveling). I am out of context with this PR and I won't be able to make progress in the near future. Do you think it will help if I merge this open PR so that you can start a new PR? I can do that.

[xpromache]

@xpromache : Sorry to get back late to you about this (traveling). I am out of context with this PR and I won't be able to make progress in the near future. Do you think it will help if I merge this open PR so that you can start a new PR? I can do that.

no need to merge, I started from the branch and I will make a pull request that will include everything.

[xpromache]

@gabhijit I need your advice on something: do you think it is worth turning the asn.1 modules into Rust modules in the generated code? From what I can see now all modules are generated flat in the same file, resulting basically in one Rust module.

Splitting in Rust modules may help avoiding some name clashes.

It may also help me with the tag definitions - the tag mode (implicit/explicit/auto) are inherited from the module definition and can be overriden in the individual type definition.
I was looking to propagate the module level definition down to individual types in the resolver. But if I can keep it at the module level then the code will be cleaner, it will resemble more the ASN.1 definitions.

[gabhijit]

@xpromache : That is indeed a very good idea. I had this in mind as well, but have not been able to spend time on it for a few months due to some other commitments.

One big advantage of that is compile time would improve substantially, which is kind of bad at the moment.

The way I would typically go about it is - have a top level module say ngap and then sub modules like ngap:;constants etc.

A couple of things that would need to be kept in mind is - when the code is generated the public facing modules (ngap above) will re-export some of the structs and consts from the inner modules. (Initially we can keep that as optional). But I believe we'll have a better idea about it once we have actual code generated.

Makes sense?

[xpromache]

Yes, make sense. I will put the BER encoding aside for a moment and look at the generation of modules.

[gabhijit]

@xpromache : We can approach it either way, if it is convenient to split into modules first we can do that. Once we have that support we'll need to increase the minor version number anyways, I was thinking of increasing it on BER support, But if this goes in first that's fine too.

[gabhijit]

@xpromache : Are you working on this issue and if you have anything that can be merged (it's okay even if it's in partially complete form). I was thinking of picking up this again in the coming days. Let me know if you are in the middle of something and would want to merge something in the next week or so.

[xpromache]

@xpromache : Are you working on this issue and if you have anything that can be merged (it's okay even if it's in partially complete form). I was thinking of picking up this again in the coming days. Let me know if you are in the middle of something and would want to merge something in the next week or so.

Sorry, I have stared at it for a couple of days not being sure what to do to minimize the amount of changes. Finally I thought I would do this:

pub(crate) struct Resolver {
    // Resolved definitions
    pub(crate) resolved_defs: BTreeMap<String, Asn1ResolvedDefinition>,

    // Unresolved Parameterized Definitions used by individual 'Type's to resolve themselves.
    pub(crate) parameterized_defs: HashMap<String, Asn1Definition>,

    // Object Classes: Used by Objects and Object Sets to resolves themselves.
    pub(crate) classes: HashMap<String, Asn1Definition>,

    pub(crate) modules: HashMap<String, ResolvedAsn1Module>,
}

struct ResolvedAsn1Module {
       // Resolved definitions
       pub(crate) resolved_defs: BTreeMap<String, Asn1ResolvedDefinition>,

       // Unresolved Parameterized Definitions used by individual 'Type's to resolve themselves.
       pub(crate) parameterized_defs: HashMap<String, Asn1Definition>,
   
       // Object Classes: Used by Objects and Object Sets to resolves themselves.
       pub(crate) classes: HashMap<String, Asn1Definition>, 
}

and in the Resolver::resolve_definitions I would move the resolved_defs/parameterized_defs/classes into a new ResolvedAsn1Module.

But then I had to switch to something else and didn't check if this idea would actually work.... Pretty sure there would be some more things to adapt when locking up things from modules.

[gabhijit]

One idea that I can think of is keeping the module name in the Asn1ResolvedDefinition. Though I do not recollect readily whether we do pass module name during resolution and use that in the generator code.

I was planning to take a look at the BER (and Tag resolution) part, is it okay if I keep pushing changes to this? FYI: There were a few fixes that I have rebased this branch against.

[xpromache]

One idea that I can think of is keeping the module name in the Asn1ResolvedDefinition. Though I do not recollect readily whether we do pass module name during resolution and use that in the generator code.

I was planning to take a look at the BER (and Tag resolution) part, is it okay if I keep pushing changes to this? FYI: There were a few fixes that I have rebased this branch against.

Please go whichever way you see fit. I'm currently busy with some other stuff, I will come back to this when time allows maybe in a couple of months.
thanks!

[xpromache]

One idea that I can think of is keeping the module name in the Asn1ResolvedDefinition. Though I do not recollect readily whether we do pass module name during resolution and use that in the generator code.
I was planning to take a look at the BER (and Tag resolution) part, is it okay if I keep pushing changes to this? FYI: There were a few fixes that I have rebased this branch against.

Please go whichever way you see fit. I'm currently busy with some other stuff, I will come back to this when time allows maybe in a couple of months. thanks!

One idea that I can think of is keeping the module name in the Asn1ResolvedDefinition. Though I do not recollect readily whether we do pass module name during resolution and use that in the generator code.

Currently you have

resolved_defs: BTreeMap<String, Asn1ResolvedDefinition>

what if two definitions in different modules have the same name?