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.

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

Parse automatic tags. #22

Closed
wants to merge 1 commit into from
Closed

Parse automatic tags. #22

wants to merge 1 commit into from

Conversation

james-ward
Copy link
Contributor

During the parse phase when the grammar is generated, if the
AUTOMATIC TAGS keyword is declared, a callback is added to parse
of constructed types. This callback adds sequential tags to the
elements of the constructed type (unless a tag exists, as per the
spec). The callback is not added when the module tag default is
IMPLICIT or EXPLICIT.

During creation of the semantic model, if the default tag type is
AUTOMATIC, the tagged element is tagged IMPLICIT unless it is a
CHOICE type or resolves to one.

Minor bugfix: if default tag mode is IMPLICIT, CHOICEs must still
be tagged EXPLICIT.

Resolves #20

During the parse phase when the grammar is generated, if the
AUTOMATIC TAGS keyword is declared, a callback is added to parse
of constructed types. This callback adds sequential tags to the
elements of the constructed type (unless a tag exists, as per the
spec). The callback is not added when the module tag default is
IMPLICIT or EXPLICIT.

During creation of the semantic model, if the default tag type is
AUTOMATIC, the tagged element is tagged IMPLICIT unless it is a
CHOICE type or resolves to one.

Minor bugfix: if default tag mode is IMPLICIT, CHOICEs must still
be tagged EXPLICIT.

Resolves #20
# If there are tags then auto-tagging does not apply
components = t.asList()[0].elements[1]
for component in components:
if component.ty == "ComponentType":
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use single quotes for strings here and throughout? Thanks!

@kimgr
Copy link
Owner

kimgr commented Sep 20, 2015

This is really cool, thanks! So the enumeration of automatic tags is always local to a constructed type? That makes the problem much easier -- I thought it was something that spanned the entire module.

It makes more sense to me to solve this in sema -- from my cursory review it looks like the explicit building of a tagged-type parse tree in auto_tags generates a lot of complexity that isn't necessary on the sema side of things (you can just replace Type instances by wrapping them inTaggedType.) I could be missing something, and I probably am :-)

@james-ward
Copy link
Contributor Author

Hey Kim,
Thanks for the feedback - I'll make those changes and repush.

I agree that the current approach seems a little fragile and hackish,
maybe we can massage it into something more acceptable.

Auto tags restart from 0 in the current constructed type, they are not
global.
The reason I didn't do it in sema was because I couldn't see a nice way
to keep track of the tag count. If you can think of a good way to do
that, I'll move it over to sema.

James

On 09/21/2015 01:46 AM, Kim Gräsman wrote:

This is really cool, thanks! So the enumeration of automatic tags is
always local to a constructed type? That makes the problem much easier
-- I thought it was something that spanned the entire module.

It makes more sense to me to solve this in sema -- from my cursory
review it looks like the explicit building of a tagged-type parse tree
in |auto_tags| generates a lot of complexity that isn't necessary on
the sema side of things (you can just replace |Type| instances by
wrapping them in|TaggedType|.) I could be missing something, and I
probably am :-)


Reply to this email directly or view it on GitHub
#22 (comment).

@kimgr
Copy link
Owner

kimgr commented Sep 21, 2015

I'm not as close to this problem as you are, so I could be overlooking something, but it seems to me the tag count is simple -- a constructed type already knows its components, and it should be trivial to assign an index to each of them.

The challenge, however, is to find the tag implicity (I think I've made this word up, so I hope it carries the right connotations :-)) from within a stand-alone SemaNode.

I think this could be made to work by making _create_sema_node part of SemaNode itself, and then feeding the default implicity from the module through to all other nodes, and then implement a variant of resolve_tag_implicity on SemaNode instead of on module.

I haven't thought this all the way through, but it might be a way forward.

@james-ward
Copy link
Contributor Author

The only thing to bear in mind is that the effect of the automatic tag is dependent on the other components of a constructed type - if any other components are tagged then the automatic tag has no effect on any components in that type. I'll have a go at doing something in SemaNode.

@kimgr
Copy link
Owner

kimgr commented Oct 2, 2015

I think I see what you're saying...

# with AUTOMATIC TAGS enabled

ManuSeq ::= SEQUENCE {
    [1] foo INTEGER,
    bar BOOLEAN
}

AutoSeq ::= SEQUENCE {
    baz INTEGER,
    quux BOOLEAN
}

bar should not be automatically tagged, but baz and quux would?

Also, if you can, please commit other cleanups separately from the SemaNode experiment, because if it gets too messy, I think we can fall back to handling it in the parser.

Thanks for working on this!

@kimgr
Copy link
Owner

kimgr commented Nov 22, 2015

This should be closed in favor of the sema-autotag branch, right?

@kimgr
Copy link
Owner

kimgr commented Dec 22, 2015

Abandoning, this was solved in Sema in PR #29

@kimgr kimgr closed this Dec 22, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Automatic tagging not implemented
2 participants