Skip to content
This repository has been archived by the owner on Mar 8, 2020. It is now read-only.

Upgrade to sdk v1 #47

Merged
merged 4 commits into from
Sep 18, 2017
Merged

Upgrade to sdk v1 #47

merged 4 commits into from
Sep 18, 2017

Conversation

abeaumont
Copy link
Contributor

No description provided.

@abeaumont abeaumont self-assigned this Sep 18, 2017
| /self::\*\[@InternalType='CompilationUnit'\]//\*\[@InternalType='TypeDeclaration'\] | Declaration, Type |
| /self::\*\[@InternalType='CompilationUnit'\]//\*\[@InternalType='TypeDeclarationStatement'\] | Statement, Declaration, Type, Incomplete |
| /self::\*\[@InternalType='CompilationUnit'\]//\*\[@InternalType='MethodDeclaration'\] | Declaration, Function |
| /self::\*\[@InternalType='CompilationUnit'\]//\*\[@InternalType='MethodDeclaration'\]/\*\[@internalRole\]\[@internalRole='name'\] | Function, Name |
Copy link
Contributor

Choose a reason for hiding this comment

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

These shouldn't have also Declaration?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question. I'm not sure about it, but I've considered that the Declaration role could be part of the parent node, while its children have different roles, in the same way that, in the case of the If constructs, there's a parent If, Statement, while the children don't have the Statement role anymore, even if they're part of the if statement (which is already noted using the If role).
This makes it easier to differentiate the declaration node itself from its nodes, and that's why I think it may be better this way, but it's arguable.

Copy link
Contributor

@juanjux juanjux Sep 18, 2017

Choose a reason for hiding this comment

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

Yes, this is something I also tough about this, specially with some monster role lists for some nodes where I apply the Declaration and similar roles, we should probably decide what to do in this case and document it somewhere.

| /self::\*\[@InternalType='CompilationUnit'\]//\*\[@InternalType='Assignment'\]/self::\*\[@operator\]\[@operator='<<='\] | Bitwise, LeftShift |
| /self::\*\[@InternalType='CompilationUnit'\]//\*\[@InternalType='Assignment'\]/self::\*\[@operator\]\[@operator='>>='\] | Bitwise, RightShift |
| /self::\*\[@InternalType='CompilationUnit'\]//\*\[@InternalType='Assignment'\]/self::\*\[@operator\]\[@operator='>>>='\] | Bitwise, RightShift, Unsigned |
| /self::\*\[@InternalType='CompilationUnit'\]//\*\[@InternalType='ArrayType'\] | Type, Primitive, List |
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure Primitive+List is the same as an array. We probably need an Array role for consecutive access memory.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I agree. I used List because List role is currently defined as a generic sequence (as it was in SDKv0)

@abeaumont abeaumont merged commit 3984d57 into bblfsh:master Sep 18, 2017
@abeaumont abeaumont deleted the feature/upgrade-to-sdk-v1 branch September 18, 2017 16:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants