Main icon indicating copy to clipboard operation
Main copied to clipboard

Type inference issue/ arithmetic bug?

Open yossigil opened this issue 9 years ago • 9 comments

This one is not simplified:

a==b+2-5

to

a==b-3

yossigil avatar Dec 04 '16 08:12 yossigil

On hold for now until we solve the namespace issues

dormaayan avatar Dec 12 '16 17:12 dormaayan

Does't seems realistic for the upcoming sprint (although now we have all the tools to handle that) , I'll work on that on my free time

dormaayan avatar Jan 12 '17 18:01 dormaayan

This may be a bit more complex than I thought. This may be a missing case in type's lookup, however this is also a case the current tippers dont' support, since the AST looks like this: (b+2)-3 That means in the ast, there aren't any two number literals in the same infix expression. There is one infix expression with 3 as one operand and (b+2) as the other operand, and another exprassion whose operands are b and 2. This seems to be a specific case of a more general pattern. @yossigil Should I write a new tipper for this, or leave this be for now?

edit: also tested type. It works mostly fine, but thinks b is alphanumeric since currently type stops looking up once it finds an infix expression, which is what we aggreed on when I developed this. I can open an issue for this and change it to look up for more than one infix expression if you want to, but right now that doesn't seem urgent to me

NivShalmon avatar Apr 14 '17 09:04 NivShalmon

Remove parenthesis (in your example) - other tipper. Problem with infix expressions (in Yossi's example b+2-5) - this tipper. Maybe you'll want to make a utility to traverse all possible layouts of an infix expressions, so b+2-5 would give (b+2)-5 and b+(2-5)... just a thought. Another solution may be a utility that receives a predicate + operation for an infix expression and commits changes in relevant sub-tree. Example: for expression a+b+2-5 and this tipper, the utility would detect 2-5 and replace (b+2)-5 with b-3.

OriRoth avatar Apr 14 '17 10:04 OriRoth

I added the parenthesis there to show the structure of the Infix expressions. Not the best way to do so. The idea of traversing all possible layouts is a nice idea, but I have no idea how to implement. The tipper recieves a node in a tree, meaning a specific layout of the tree. We need to work with the layout we get. The last idea may work, I think TermCollector or one of the other Term classes can help with this

NivShalmon avatar Apr 14 '17 10:04 NivShalmon

There are two seperate issues here:

  1. type's lookup only looks up one infix expression. Beacuse of that, in this example type will tell that b is ALPHANUMERIC, despite being able to tell that b is NUMERIC. However, this isn't the reason why the tipper won't work it, since type tells that b+2 and the entire expression are NUMERIC. Thus the second problem.
  2. the tipper doesn't work on this case. This is a problem with the tippers implementation rather than type. type tells the tipper it can work here, (i.e. isNotString returns true), but the tipper doesn't do anything, most likely since the two literals are not part of the same infixExpression. The best way to solve this is probably redoing the tipper using Term, but I believe this isn't urgent enough to do now, since these tippers are complex and work for most cases with the current implementation.

NivShalmon avatar May 28 '17 17:05 NivShalmon

I had a few suggestions in my comment above. Another option I thought of is to improve "type" to give us additional information about expressions: if it is an infix expression, "type" can tell us things about it's left-most expression and it's right-most expression. So for b + 2 we will get 2 for right-most expression, then the tipper would be able to tell it fits with 5 in the infix expression. We can preserve this information: for the infix expression e1 op e2 the left-most expression is the left-most expression of e1- and so on for right-most and the other (simpler) expression variations (a.k.a parenthesis, pre/postfix, etc.). This is only recognition, I think I discussed transition implementation above.

OriRoth avatar May 28 '17 17:05 OriRoth

The problem with this idea is that it requires to completly change type and its functionallity. Currently type is a set of singeltons, each type (a type defined as part of an enum or one created by baptized) is a singelton. It's done this way since type is only supposed to tell you the type of the expression, nothing more. This singelton behaviour, by default, can't represent these type trees. If we do want to create such trees, we can offer a different utility that uses type. However, that doesn't belong to type itself. I belive a better approach to this would be redoing the tipper using Term, since TermCollector will allow us to go over the list of Terms and find all number literals in it, regardless of +/-. Term is bassically a stronger form of the utility you suggested that already exists in the code, so we can try to use that here.

NivShalmon avatar May 28 '17 19:05 NivShalmon

Strong point @NivShalmon

yossigil avatar May 29 '17 20:05 yossigil