Fix haddock: internal error: links: UnhelpfulSpan #561
Conversation
This fixes haskell#554 for me. I believe this is another fall out of `wildcard-refactor`, like haskell#549.
|
I'm not 100% sure this is the way we want to fix it, but I have this patch lying around and for a lack of a better alternative I'm publishing it here. |
| @@ -311,7 +311,7 @@ synifyDataCon use_gadt_syntax dc = | |||
| , con_doc = Nothing } | |||
|
|
|||
| synifyName :: NamedThing n => n -> Located Name | |||
| synifyName = noLoc . getName | |||
| synifyName n = L (srcLocSpan (getSrcLoc n)) (getName n) | |||
There was a problem hiding this comment.
@niteria I understand this change. It is a great one! We were previously assuming noLoc where we now use the location of the name, great 100% more information!
| @@ -646,7 +646,7 @@ ppInstanceSigs links splice unicode qual sigs = do | |||
| TypeSig lnames typ <- sigs | |||
| let names = map unLoc lnames | |||
| L loc rtyp = get_type typ | |||
| return $ ppSimpleSig links splice unicode qual loc names rtyp | |||
| return $ ppSimpleSig links splice unicode qual (getLoc $ head $ lnames) names rtyp | |||
There was a problem hiding this comment.
I'm not so sure about this. Why not use loc, the span associated with the type? It likely makes little difference in the result, but it would be nice to understand why this change is necessary.
There was a problem hiding this comment.
@niteria can you explain this change here? Would it make sense to extend the synify business to give more meaningful src locs if possible to avoid not doing the obvious thing here (to take loc instead)?
|
I'd love to see this PR land. I observed that a case of UnhelpfulSpan is resolved by applying this patch. I didn't try to minimize the problematic source. Let me know if this would be useful. |
|
@blackgnezdo please take a look at haddock release 2.17.5 (http://hackage.haskell.org/package/haddock-2.17.5/changelog). You will find a fix to the problem there. I was thinking about reviewing this PR though as this seems to be a better solution to the problem. |
|
If you have suggestions on how to improve this patch and make it mergeable I'm happy to do the legwork. |
|
@alexbiehl my apologies, I failed to include the version information. I am already using v2_17_5. It exhibits the bug without this patch. |
|
@blackgnezdo Interesting! Have you got a reproducer? |
|
I'll have to untangle it from the internal codebase. Time is an issue as usual... |
|
We can reproduce it with haddock 2.7.14 with module Test(Addable(..)) where
class Addable a where
plus :: a -> a -> a
instance Addable Integer where
plus = (+) |
|
I rebased and added a comment. Thanks @niteria! |
|
Thanks! Sorry I missed your inline comment, I must have my github notifications set up wrong. |
This fixes #554 for me. I believe this is another fall out
of
wildcard-refactor, like #549.