search

mouse07410 / asn1c

The ASN.1 Compiler
http://lionet.info/asn1c/
BSD 2-Clause "Simplified" License
93 stars 70 forks source link

Class default syntax support #172

Closed v0-e closed 6 months ago

v0-e commented 7 months ago

Adds support for CLASS default syntax. This complements the already implemented defined syntax (through the construct WITH SYNTAX {}).

Class instances with no defined syntax, must be implemented using the default syntax. The default syntax follow the format: "{" FieldSetting "," * "}", where FieldSetting is PrimitiveFieldName Setting. A PrimitiveFieldName is a reference to a class element, and Setting is a value, type, or object. FieldSetting's can appear in any order in the class instance definition.

Fixes the compilation of ASN.1 definitions with classes without WITH SYNTAX {}, such as the one in issue #171.

A new test (164) was added.

Current issue: tests related with the 18-class-OK.asn1 file currently fail. There are several reasons, and they only came to light now because the proposed PR now actually handles the fields inside of classes with default syntax.

  1. a comma is missing after { PosPair | NegPair }. Nokalva's playground also cannot compile this file for the same reason;
  2. now if you define PosPair and NegPair as some types, asn1c still cannot handle the construct { PosPair | NegPair };
  3. the 0 defined for &result-if-error cannot be handled because &ResultType can be any type (with NULL as default).

Where issue 1. should be fixed by changing the ASN definition itself itself, 2. should be fixed in asn1c. This last one is probably related with this warning message, where most FieldSpec's are not yet handled. About 3., I'm not sure if setting 0 is correct if the underlying type can be any.

I did not yet check how hard or easy it is to fix 2., and I don't know if I'll have the time to look into it. In the meantime, we can modify/remove 18-class-OK.asn1 to pass the tests.

Followed X.681 (2021) standard.

mouse07410 commented 7 months ago

In the meantime, we can modify/remove 18-class-OK.asn1 to pass the tests.

You want to split this PR into those that pass CI and can be merged, and those that require more work and will be on hold for now?

v0-e commented 7 months ago

Do you mean, e.g., modifying the mentioned test file to pass the tests for now?

mouse07410 commented 7 months ago

Do you mean, e.g., modifying the mentioned test file to pass the tests for now?

I guess it depends on why it's failing.

If this PR causes a correct and expected change in the output that the current test-file does not account for, then probably yes. If the output just changes and gets out-of-sync with the test-file, then definitely no.

v0-e commented 6 months ago

I've modified 18-class-OK.asn1 to remove the class fields not currently supported, namely VariableTypeValueFieldSpec and VariableTypeValueSetFieldSpec. The related issue affects both default syntax classes and defined syntax classes.

To be clear, the relevant test file is only used in contained unit tests like the fixer tests, and was not being thoroughly parsed/analyzed/tested before the proposed changes.