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

Solving CompareSymbol and AppendSymbol #2511

Merged
merged 3 commits into from Dec 30, 2016
Merged

Solving CompareSymbol and AppendSymbol #2511

merged 3 commits into from Dec 30, 2016

Conversation

LiamGoodacre
Copy link
Member

@LiamGoodacre LiamGoodacre commented Dec 28, 2016

Opened for discussion.

I haven't been able to figure out why, but currently the tests fail to build the support libraries: http://lpaste.net/1293011848764850176

Any help working that out would be appreciated.

EDIT: building tests works - I needed to clear .test_modules. However, updating test support is causing a lot of tests to now fail.

forClassName _ C.IsSymbol [TypeLevelString sym] = [TypeClassDictionaryInScope (IsSymbolInstance sym) [] C.IsSymbol [TypeLevelString sym] Nothing]
forClassName _ C.IsSymbol [TypeLevelString sym] =
[TypeClassDictionaryInScope (IsSymbolInstance sym) [] C.IsSymbol [TypeLevelString sym] Nothing]
forClassName _ C.CompareSymbol [arg0@(TypeLevelString lhs), arg1@(TypeLevelString rhs), _] =
Copy link
Contributor

Choose a reason for hiding this comment

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

You're going to need to do some unification on the result type here, I think.

Copy link
Member Author

Choose a reason for hiding this comment

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

Unification happens later using matches when computing instances in solve:go, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

and when I say later, I mean immediately after:

let instances =
      [ (substs, tcd)
      | tcd <- forClassName (combineContexts context inferred) className' tys''
        -- Make sure the type unifies with the type in the type instance definition
      , substs <- maybeToList (matches typeClassDependencies tcd tys'')
      ]

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, yes, you're right.

fs <- liftIO $ readInput inputFiles
ms <- P.parseModulesFromFiles id fs
foreigns <- inferForeignModules ms
liftIO (check (map snd ms))
let actions = makeActions foreigns
let actions = makeActions (foreigns `M.union` supportForeigns)
Copy link
Member Author

Choose a reason for hiding this comment

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

So here is where we makeActions. This makes a codegen function in Make.hs which checks for an FFI file using the foreigns argument passed to makeActions. But we were only passing in the foreign files for the modules of the test input files (I think). So I've passed through the FFI support files too. The tests started passing once I did this.

@paf31 paf31 merged commit 88fdcee into purescript:master Dec 30, 2016
@paf31
Copy link
Contributor

paf31 commented Dec 30, 2016

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants