ghci: fixity declarations for infix data constructors (#10018)
ClosedPublic

Authored by bgamari on Jul 2 2015, 11:43 AM.

Details

Summary

Declaring a custom fixity for an infix data constructor should work:

Prelude> data Infix a b = a :@: b; infixl 4 :@:

This is a followup to Trac #2947, which handled fixity declarations in ghci
statements (e.g. let add = (+); infixl 6 add).

Support for declarations (data, type, newtype, class, instance,
deriving, and foreign) was added to GHCi in Trac #4929.

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
thomie updated this revision to Diff 3375.Jul 2 2015, 11:43 AM
thomie retitled this revision from to ghci: fixity declarations for infix data constructors (#10018).
thomie updated this object.
thomie edited the test plan for this revision. (Show Details)
thomie updated the Trac tickets for this revision.
bgamari accepted this revision.Jul 5 2015, 3:10 AM
bgamari added a reviewer: bgamari.

This look great!

This revision is now accepted and ready to land.Jul 5 2015, 3:10 AM
simonpj requested changes to this revision.Jul 6 2015, 4:18 AM
simonpj edited edge metadata.

The goal is right, but there is something very wrong with the implementation. See my comments above.

compiler/main/HscMain.hs
1455

Wait! An InteractiveContext contains a FixityEnv, so why return them separately?

The easiest fix, surely, is simply to set the FixityEnv in the returned InteractiveContext from the tc_gblenv.

compiler/main/InteractiveEval.hs
256 ↗(On Diff #3375)

Having laboriously passed out the new FixityEnv you now stuff it into the InteractiveContext!!! Far easier to do that hscDeclsWithLocation.

compiler/typecheck/TcRnDriver.hs
595 ↗(On Diff #3375)

getFixityEnv just extracts the fixity-env from the TcGblEnv! (Assuming that tcg_env and the one in the monad are the same.

1906 ↗(On Diff #3375)

Wait! A TcGblEnv contains a FixityEnv so why return them separately?

This revision now requires changes to proceed.Jul 6 2015, 4:18 AM
thomie updated this revision to Diff 3482.Jul 8 2015, 4:27 PM
thomie edited edge metadata.

Please review this update.

My original thinking was that the functions getFixityEnv and
updateFixityEnv exist, so we should probably use them. If something changes
in 5 years time in the way FixityEnvs need to be updated, we don't
accidentally forget to do so in one place.

Much better! Less code, more robust.

compiler/main/HscMain.hs
1470

i'd put this defn of fix_env near the call to extendInteractiveContext

compiler/main/HscTypes.hs
1439

Very cryptic comment. Worth elaborating with a Note giving an example.

bgamari requested changes to this revision.Jul 10 2015, 12:50 PM
bgamari edited edge metadata.

@thomie, it looks like there is another set of changes needed here.

This revision now requires changes to proceed.Jul 10 2015, 12:50 PM
thomie abandoned this revision.Jul 21 2015, 7:47 AM

I don't understand this code well enough to write a Note about it. Probably better I don't touch it at all.

bgamari commandeered this revision.Jul 21 2015, 3:00 PM
bgamari edited reviewers, added: thomie; removed: bgamari.

I think I can finish this up. Thanks @thomie!

This revision was automatically updated to reflect the committed changes.
bgamari marked 2 inline comments as done.