#8607: Invalid location reported for type constructors -------------------------------------+------------------------------------ Reporter: edsko | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.7 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Unknown/Multiple Type of failure: None/Unknown | Difficulty: Unknown Test Case: | Blocked By: Blocking: | Related Tickets: -------------------------------------+------------------------------------ Comment (by edsko): The same problem goes for class definitions; the location of `C` in {{{ class C a where f :: a -> a }}} is reported as the span of the entire class, rather than the span of just the identifier. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8607#comment:2 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#8607: Invalid location reported for type constructors -------------------------------------+------------------------------------ Reporter: edsko | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.7 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Unknown/Multiple Type of failure: None/Unknown | Difficulty: Unknown Test Case: | Blocked By: Blocking: | Related Tickets: -------------------------------------+------------------------------------ Comment (by edsko): .. and type families .. Location of `Bar` in {{{ type family Bar a }}} is reported as the entire line (type family instances do appear to be correct). -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8607#comment:4 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#8607: Invalid location reported for type constructors -------------------------------------+------------------------------------ Reporter: edsko | Owner: goldfire Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.7 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Unknown/Multiple Type of failure: None/Unknown | Difficulty: Unknown Test Case: | Blocked By: Blocking: | Related Tickets: -------------------------------------+------------------------------------ Changes (by goldfire): * owner: => goldfire Comment: Yes, it does appear to be somewhat intentional. But, it looks like some plumbing change in !TcTyClsDecls could restore the old behavior. (I'm thinking of returning `TcM [Located TyThing]` from `tcTyClDecl`.) I can take a look at this next week. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8607#comment:6 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#8607: Invalid location reported for type constructors -------------------------------------+------------------------------------ Reporter: edsko | Owner: goldfire Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.7 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Unknown/Multiple Type of failure: None/Unknown | Difficulty: Unknown Test Case: | Blocked By: Blocking: | Related Tickets: -------------------------------------+------------------------------------ Comment (by goldfire): I'm not sure what chunk of code you're talking about. While I agree that a `Located Name` is redundant, I don't see how that helps us here. It seems to me that the function of interest is `checkValidTyCl`, which just takes a `TyThing`, devoid of any location information other than what is stored in the `n_loc` field of `Name`s. By passing in an extra location (by using `Located TyThing`), we can get good error messages and proper spans on identifiers. I do feel a little dirty decorating the Core-ish `TyThing` with the Haskell-ish `Located`, but I think this is a reasonable place to do so. Other suggestions are welcome. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8607#comment:8 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#8607: Invalid location reported for type constructors -------------------------------------+------------------------------------ Reporter: edsko | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.7 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Unknown/Multiple Type of failure: None/Unknown | Difficulty: Unknown Test Case: | Blocked By: Blocking: | Related Tickets: -------------------------------------+------------------------------------ Changes (by goldfire): * owner: goldfire => Comment: So, I took a look at this, and it's ugly. The problem is that each `TyClDecl` can give rise to potentially many top-level things. Of these things, only the `TyCon`s are checked for validity, so we really only need location info for `TyCon`s. It's a little painful returning a `[Located TyThing]` when the `Located` bit applies to only one disjunct of `TyThing`. But, I was willing to deal with that. The real problem comes from the fact that one declaration can actually produce many `TyCon`s: a class with associated types. According to the validity checking code -- which checks all `TyCon`s, top-level and not -- we would need these associated type `TyCon`s to have correct locations. Unfortunately, there is no clean way to get good locations for associated types without polluting the code in `tcClassATs` and possibly `ClassATItem`, which is persisted within the `Class` datatype. Very yuck. Is all of this doable? Absolutely, but it would make Simon wish he had just stayed on holiday. So, I propose that, to fix this problem, we store location information ''in the `TyCon`''. This should be easy to get correct and easy to use. It would be cleaner than adding `Located` in various places throughout !TcTyClsDecls. But, it stores location information in a very Core-ish place, and I would rather consult with others before going ahead with this plan. There is precedent: `CoAxBranch`es store locations for similar reasons. But, of course, I was the chief person behind `CoAxBranch`es, and it's a little silly to use my own design decision as precedent. Thoughts? -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8607#comment:10 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#8607: Invalid location reported for type constructors -------------------------------------+------------------------------------ Reporter: edsko | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.7 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Unknown/Multiple Type of failure: None/Unknown | Difficulty: Unknown Test Case: | Blocked By: Blocking: | Related Tickets: -------------------------------------+------------------------------------ Comment (by goldfire): Yes, I see what you're saying much better now. But, it still doesn't seem to quite correspond with what's going on in Simon's original change. That commit (1745779...) seems to change only the location in the `Located` bit, not the one in the `nameSrcSpan`. Oh, it's all suddenly clear. Simon's commit was overzealous. It turns out that the location stored in the `Located` bit in a `tcdLName` seems to be ignored in the validity checker -- it's only the location in the `nameSrcSpan` that does any work. So, the changes Simon made to !RdrHsSyn were totally unnecessary to correspond to the refactoring in !TcTyClsDecls. The solution is dead easy: I can just revert the changes in !RdrHsSyn and revert the corresponding changes in the testsuite, while keeping the refactoring in !TcTyClsDecls, which was the whole point to begin with. Have to run now, but will do this later today. Thanks for pointing me in this direction! -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8607#comment:12 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#8607: Invalid location reported for type constructors -------------------------------------+------------------------------------ Reporter: edsko | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.7 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Unknown/Multiple Type of failure: None/Unknown | Difficulty: Unknown Test Case: | Blocked By: Blocking: | Related Tickets: -------------------------------------+------------------------------------ Comment (by Richard Eisenberg ): In [changeset:"e4afeedc5b8ac0f48cbeac09aa702c8d10433cdb/ghc"]: {{{ #!CommitTicketReference repository="ghc" revision="e4afeedc5b8ac0f48cbeac09aa702c8d10433cdb" Fix #8607. The solution (after many false starts) is to change the behavior of hsLTyClDeclBinders. The idea is that the locations of the names that the parser generates should really be the names' locations, unlike what was done in 1745779... But, when the renamer is creating Names from the RdrNames, the locations stored in the Names should be the declarations' locations. This is now achieved in hsLTyClDeclBinders, which returns [Located name], but the location is that of the *declaration*, not the name itself. }}} -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8607#comment:14 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#8607: Invalid location reported for type constructors -------------------------------------+------------------------------------ Reporter: edsko | Owner: Type: bug | Status: closed Priority: normal | Milestone: Component: Compiler | Version: 7.7 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: Unknown/Multiple Type of failure: None/Unknown | Difficulty: Unknown Test Case: | Blocked By: Blocking: | Related Tickets: -------------------------------------+------------------------------------ Changes (by goldfire): * status: new => closed * resolution: => fixed Comment: See the commit message above for details -- somewhat different than my last Trac post on the subject. The solution I found was much cleaner than what I proposed here. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8607#comment:16 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#8607: Invalid location reported for type constructors -------------------------------------+------------------------------------ Reporter: edsko | Owner: Type: bug | Status: closed Priority: normal | Milestone: Component: Compiler | Version: 7.7 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: Unknown/Multiple Type of failure: None/Unknown | Difficulty: Unknown Test Case: | Blocked By: Blocking: | Related Tickets: -------------------------------------+------------------------------------ Comment (by simonpj): I like it, thank you. Simon -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8607#comment:18 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler