emilaxelsson / syntactic

Generic representation and manipulation of abstract syntax
BSD 3-Clause "New" or "Revised" License
25 stars 13 forks source link

alphaEqSymDefault ignores type annotations. #13

Open pjonsson opened 10 years ago

pjonsson commented 10 years ago

cc @jankner (did that work?)

Example from the calling convention test suite:

pairRes :: Data Word16 -> (Data WordN, Data IntN)
pairRes a = (i2n a, i2n a)

Gives an exception somewhere in the rebuilding of expressions:

*Main> printExprWith defaultFeldOpts { targets = [CSE] } pairRes
*** Exception: rebuild: type mismatch
jankner commented 9 years ago

I have now investigated this bug, and the problem is simply that the code motion sees the two parts of the pair as identical expressions, despite them having different types.

I don't know exactly how to solve this best. Currently, I'm just using alphaEq to compare expressions. Should I do something else to take the types into account?

pjonsson commented 9 years ago

That sounds like a problem elsewhere.

@emilaxelsson: is the AlphaEq instance for Conversion wrong in Feldspar? It uses alphaEqSymDefault which sounds perfectly sane.

jankner commented 9 years ago

I tried comparing the types of the expressions (like typeRep e1 == typeRep e2) as well as using alphaEq, and that seems to fix the problem. I don't know if that's the right way of doing it, but it seems to work.

pjonsson commented 9 years ago

The real problem is the equality test since two expressions of different types are considered equivalent despite the fact that we can observe they are different.

Your explicit testing for type equality sounds like a good workaround at the moment though since Emil seems to be preoccupied with other things. Can you submit a pull request with that change?

pjonsson commented 9 years ago

For the books: workaround merged as #15. Remember to remove that code when this bug is fixed.

Changing title of the ticket to reflect the underlying issue.