Upgrades for UAST/SDK v1 #90

juanjux · 2017-09-15T15:33:48Z

Migrated annotations to the new UAST format.
Fixed some Incomplete annotations that now can be better
represented.
Fixed some missing annotations and a couple bugs.
Updated integrated tests, added some new ones.
Versioned imports to v1.

- Migrated annotations to the new UAST format. - Fixed some Incomplete annotations that now can be better represented. - Fixed some missing annotations and a couple bugs. - Updated integrated tests, added some new ones. - Versioned imports to v1.

juanjux · 2017-09-15T15:34:59Z

Part of bblfsh/sdk#167

abeaumont · 2017-09-15T15:39:12Z

ANNOTATION.md

+| /self::\*\[@InternalType='Module'\]//\*\[@InternalType='BinOp'\]/\*\[@internalRole\]\[@internalRole='op'\] | Expression, Binary, Operator |
+| /self::\*\[@InternalType='Module'\]//\*\[@InternalType='BinOp'\]/\*\[@internalRole\]\[@internalRole='left'\] | Expression, Binary, Left |
+| /self::\*\[@InternalType='Module'\]//\*\[@InternalType='BinOp'\]/\*\[@internalRole\]\[@internalRole='right'\] | Expression, Binary, Right |
+| /self::\*\[@InternalType='Module'\]//\*\[@InternalType='Eq'\] | Operator, Equal |


These operators are binary, aren't they?

abeaumont · 2017-09-15T15:39:44Z

ANNOTATION.md

+| /self::\*\[@InternalType='Module'\]//\*\[@InternalType='IsNot'\] | Operator, Identical, Not |
+| /self::\*\[@InternalType='Module'\]//\*\[@InternalType='In'\] | Operator, Contains |
+| /self::\*\[@InternalType='Module'\]//\*\[@InternalType='NotIn'\] | Operator, Contains, Not |
+| /self::\*\[@InternalType='Module'\]//\*\[@InternalType='Add'\] | Add |


This should be Operators too?

abeaumont · 2017-09-15T15:42:50Z

ANNOTATION.md

+| /self::\*\[@InternalType='Module'\]//\*\[@InternalType='AsyncFunctionDef'\] | Function, Declaration, Name, Identifier, Incomplete |
+| /self::\*\[@InternalType='Module'\]//\*\[@InternalType='FunctionDef\.decorator\_list'\] | Function, Call, Incomplete |
+| /self::\*\[@InternalType='Module'\]//\*\[@InternalType='FunctionDef\.body'\] | Function, Declaration, Body |
+| /self::\*\[@InternalType='Module'\]//\*\[@internalRole\]\[@internalRole='arguments'\] | Argument, Incomplete |


These rae part of Function, Declaration or there are other kind of arguments here?

I was thinking that Call also used these, but I was wrong, the only other usage is in LambdaFunctionDef that should be fine having their arguments tagged as Function and Declaration.

abeaumont · 2017-09-15T15:45:04Z

ANNOTATION.md

+| /self::\*\[@InternalType='Module'\]//\*\[@InternalType='Try'\]/\*\[@InternalType='Try\.orelse'\] | Try, Body, Else |
+| /self::\*\[@InternalType='Module'\]//\*\[@InternalType='TryExcept'\] | Try, Catch, Statement |
+| /self::\*\[@InternalType='Module'\]//\*\[@InternalType='ExceptHandler'\] | Try, Catch, Statement |
+| /self::\*\[@InternalType='Module'\]//\*\[@InternalType='ExceptHandler\.name'\] | Identifier |


this is part of a Try, Catch construct as well i think

abeaumont · 2017-09-15T15:48:05Z

ANNOTATION.md

-| /self::\*\[@InternalType='Module'\]//\*\[@InternalType='DictComp'\] | MapLiteral, Expression, Incomplete |
-| /self::\*\[@InternalType='Module'\]//\*\[@InternalType='SetComp'\] | SetLiteral, Expression, Incomplete |
+| /self::\*\[@InternalType='Module'\]//\*\[@InternalType='Ellipsis'\] | Identifier, Incomplete |
+| /self::\*\[@InternalType='Module'\]//\*\[@InternalType='ListComp'\] | Literal, List, For, Expression, Incomplete |


I don't think comprehensions can be considered Literals

abeaumont · 2017-09-15T15:49:55Z

driver/normalizer/pyast/pyast.go

 // Package pyast defines constants from Python 2 and 3 AST.
 package pyast

-// GENERATED BY python-driver/native/gogen/gogen.py


No longer autogenerated?

No, actually it was being manually managed since some time ago, I moved the HasInternalType calls these to have a clearer annotations.go file (like you did in the Java driver) and sorted alphabetically. Also, I was getting slightly different results on gogen depending on the minor Python version used. I'll update or remove gogen.py soon.

abeaumont · 2017-09-18T09:05:55Z

ANNOTATION.md

+| /self::\*\[@InternalType='Module'\]//\*\[@InternalType='BitOr'\] | Binary, Operator, Bitwise, Or |
+| /self::\*\[@InternalType='Module'\]//\*\[@InternalType='BitXor'\] | Binary, Operator, Bitwise, Xor |
+| /self::\*\[@InternalType='Module'\]//\*\[@InternalType='BitAnd'\] | Binary, Operator, Bitwise, And |
+| /self::\*\[@InternalType='Module'\]//\*\[@InternalType='And'\] | Operator, Boolean, And |


Boolean operators are also either binary or unary, aren't they?

Yes and no. Yes on the (Python) language syntax. Not on the (Python) AST, where the boolean operators (and only them) produce prefixed lists. I tough it would be confusing to mark them as Binary but having the user of the UAST following another structure so I left the Binary out in this case.

I don't think the structure of the nodes in the AST should define their semantics, it should be defined by the language. I don't think that leaving out boolean operators when someone looks for binary operators is correct.

But also probably most users would expect a binary operator to have Left and Right nodes going with it, and in this case those won't be found:

1 and 2 and 3 and 4 # => Expr(And(1, 2, 3, 4))

If we are ok with and expression having an operator tagged as binary but not left and right nodes I could add the Binary role to them, but I'm not totally satisfied with this.

Ok, so the problem here is not only that the AST produces prefixed lists, but that these lists are not binary themselves... so that's even a worse scenario, since they're using a single node to represent various binary operators, right?

Exactly. So we have to choose to represent the node as the source code reader sees it (binary) or as the AST represents it (prefixed functional style). I think tagging it as Binary is probably the least-surprising way for the moment. Eventually this is probably one of the cases that could be handed by the future tree normalizer.

abeaumont · 2017-09-18T09:13:46Z

ANNOTATION.md

-| /self::\*\[@InternalType='Module'\]//\*\[@InternalType='DictComp'\] | MapLiteral, Expression, Incomplete |
-| /self::\*\[@InternalType='Module'\]//\*\[@InternalType='SetComp'\] | SetLiteral, Expression, Incomplete |
+| /self::\*\[@InternalType='Module'\]//\*\[@InternalType='Ellipsis'\] | Identifier, Incomplete |
+| /self::\*\[@InternalType='Module'\]//\*\[@InternalType='ListComp'\] | List, For, Expression, Incomplete |


A comprehension in python could be considered a for expression, I think, so this annotation could be considered complete.

It's almost there with the new UAST roles, but not totally, I think. It doesn't have nodes for the part of the for that assign the aggregates of the iteration to the destination container. If we added a Generator role of something similar then it could be complete. I'll prefer to leave it Incomplete for the moment so remember to revisit it later.

I don't see the link between having Generators and comprehensions. A generator would be a child of the comprehension, not the comprehension itself, wouldn't it? That is, the Generator role shouldn't be added to this node even if it existed.

Now that you say it, the List/Set/Map comprehensions have the expression nodes (i*2) and one or more comprehension subnodes (currently tagged as For, Expression) for every for inside the comprehension (you can add more for after the first one to have N-dimension containers). Probably this is the place where the future Generator or similar node would have to go since they define the shape of the produced container, so I'll move the Incomplete tag there.

…incomplete to the inner node

Upgrades for UAST/SDK v1

4392d60

- Migrated annotations to the new UAST format. - Fixed some Incomplete annotations that now can be better represented. - Fixed some missing annotations and a couple bugs. - Updated integrated tests, added some new ones. - Versioned imports to v1.

juanjux requested a review from abeaumont September 15, 2017 15:33

juanjux self-assigned this Sep 15, 2017

abeaumont suggested changes Sep 15, 2017

View reviewed changes

Feedback from review

b3950cb

abeaumont reviewed Sep 18, 2017

View reviewed changes

Added Binary(ish) role to the Boolean operators, move Comprehensions …

a28480f

…incomplete to the inner node

abeaumont approved these changes Sep 18, 2017

View reviewed changes

juanjux merged commit 6b92e7a into bblfsh:master Sep 18, 2017

juanjux deleted the feature/bip003_roles branch September 18, 2017 10:46

Upgrades for UAST/SDK v1 #90

Upgrades for UAST/SDK v1 #90

Uh oh!

Conversation

juanjux commented Sep 15, 2017

Uh oh!

juanjux commented Sep 15, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

juanjux Sep 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

juanjux Sep 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

juanjux Sep 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

juanjux Sep 18, 2017 •

edited

Loading

juanjux Sep 18, 2017 •

edited

Loading

juanjux Sep 18, 2017 •

edited

Loading