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

alphaEqSymDefault ignores type annotations. #13

Open
pjonsson opened this issue Aug 30, 2014 · 5 comments
Open

alphaEqSymDefault ignores type annotations. #13

pjonsson opened this issue Aug 30, 2014 · 5 comments

Comments

@pjonsson
Copy link

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
Copy link

jankner commented Sep 23, 2014

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
Copy link
Author

That sounds like a problem elsewhere.

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

@jankner
Copy link

jankner commented Sep 27, 2014

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
Copy link
Author

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
Copy link
Author

pjonsson commented Oct 1, 2014

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.

@pjonsson pjonsson changed the title CSE2; "rebuild: type mismatch" alphaEqSymDefault ignores type annotations. Oct 1, 2014
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

No branches or pull requests

2 participants