Skip to content

Print missing language extensions on TH generation of labels#352

Merged
arybczak merged 4 commits intomasterfrom
better-th-errors
Sep 27, 2020
Merged

Print missing language extensions on TH generation of labels#352
arybczak merged 4 commits intomasterfrom
better-th-errors

Conversation

@arybczak
Copy link
Collaborator

@arybczak arybczak commented Sep 6, 2020

Fixes #342.

unknown@electronics optics $ cabal repl optics-th
Build profile: -w ghc-8.10.2 -O1
In order, the following will be built (use -v for more details):
 - optics-th-0.3.0.2 (lib) (file src/Optics/TH/Internal/Product.hs changed)
Preprocessing library for optics-th-0.3.0.2..
GHCi, version 8.10.2: https://www.haskell.org/ghc/  :? for help
Loaded GHCi configuration from /home/unknown/.ghci
[1 of 5] Compiling Language.Haskell.TH.Optics.Internal ( src/Language/Haskell/TH/Optics/Internal.hs, interpreted )
[2 of 5] Compiling Optics.TH.Internal.Utils ( src/Optics/TH/Internal/Utils.hs, interpreted )
[3 of 5] Compiling Optics.TH.Internal.Sum ( src/Optics/TH/Internal/Sum.hs, interpreted )
[4 of 5] Compiling Optics.TH.Internal.Product ( src/Optics/TH/Internal/Product.hs, interpreted )
[5 of 5] Compiling Optics.TH        ( src/Optics/TH.hs, interpreted )
Ok, five modules loaded.
λ> :set -XTemplateHaskell
λ> data X = X { xX :: Int };
λ> ; makeFieldLabels ''X;

<interactive>:3:3: error:
    Generating LabelOptic instances requires the following language extensions:

- DataKinds
- FlexibleInstances
- MultiParamTypeClasses
- TypeFamilies
- UndecidableInstances

Please enable the extensions by copy/pasting these lines into the top of your file:

{-# LANGUAGE DataKinds #-}
{-# LANGUAGE FlexibleInstances #-}
{-# LANGUAGE MultiParamTypeClasses #-}
{-# LANGUAGE TypeFamilies #-}
{-# LANGUAGE UndecidableInstances #-}

To enable them in the GHCi session, use the following command:

:set -XDataKinds -XFlexibleInstances -XMultiParamTypeClasses -XTypeFamilies -XUndecidableInstances

Copy link
Member

@adamgundry adamgundry left a comment

Choose a reason for hiding this comment

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

@arybczak Lovely! Thanks for taking care of this. A couple of minor suggestions in addition to the review comments:

  • makeFields could have a requireExtensions call.
  • I'm slightly tempted to include the list of required extensions explicitly in the haddocks.

@arybczak
Copy link
Collaborator Author

makeFields could have a requireExtensions call

I wanted to do that, the problem is that its need for FunctionalDependencies depends on whether the HasX class will be generated.

@arybczak
Copy link
Collaborator Author

I'm slightly tempted to include the list of required extensions explicitly in the haddocks.

There are 7 variations for functions that generate labels (makeFieldLabels, makeFieldLabelsFor, makeFieldLabelsWith, declareFieldLabels, declareFieldLabelsFor, declareFieldLabelsWith, makePrismLabels), so I'd skip it.

@phadej
Copy link
Contributor

phadej commented Sep 17, 2020

FunctionalDependencies is "just" a better MultiParamTypeClasses. Yes, HLint (and maybe some other tools) will say that you don't need FunDep, as MPTC is sufficient, but they are wrong. IMO better just to FunDep, after all LabelOptic class uses fundeps, so it's not like you ask for something extra.

@arybczak
Copy link
Collaborator Author

arybczak commented Sep 17, 2020

Fair enough, I added check for FunctionalDependencies to makeFields.

@arybczak
Copy link
Collaborator Author

Though makeFields also needs FlexibleInstances for higher rank fields 🙄 Sigh.

@arybczak
Copy link
Collaborator Author

I made it require both. HLint will not complain about redundant pragmas because it can't inspect TH.

@phadej
Copy link
Contributor

phadej commented Sep 18, 2020

HLint will not complain about redundant pragmas because it can't inspect TH.

It may for users who copy-paste the LANGUAGE pragma block. (But it's ok to me).

@adamgundry
Copy link
Member

I'm okay with this. A slight over-approximation to the required extensions seems relatively unproblematic, although it might conceivably result in users needing to enable additional extensions on a working build. And fair enough regarding docs.

@arybczak arybczak merged commit 9f61bb5 into master Sep 27, 2020
@arybczak arybczak deleted the better-th-errors branch September 27, 2020 10:07
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

Successfully merging this pull request may close these issues.

Better docs/errors for missing extensions with TH declaration splices

3 participants