- ghc-devs - Haskell.org

ExprWithTySigOut
by Alan & Kim Zimmerman 21 Dec '15

21 Dec '15

At the moment HsExpr includes the following two constructors | ExprWithTySig (LHsExpr id) (LHsSigWcType id) | ExprWithTySigOut -- Post typechecking (LHsExpr id) (LHsSigWcType Name) -- Retain the signature, -- as HsSigType Name, for -- round-tripping purposes I do not understand why we need the second one, which hard-codes Name instead of id. There are a couple of places where there is an XXXOut variant with hard coded Name parameter, and these complicate creating any kind of custom class intended to work on a parameterised AST as it ripples out and forces Name instances of it everywhere. Perhaps we should introduce HsExprLR id id to cope with renaming? Alan

3 2

RE: [commit: ghc] master: SrcLoc: Eliminate constructors of RealSrcSpan (987426c)
by Simon Peyton Jones 21 Dec '15

21 Dec '15

Does this change yield any benefits? Eg. if we can now unbox SrcLoc does something go faster? S | -----Original Message----- | From: ghc-commits [mailto:ghc-commits-bounces@haskell.org] On Behalf Of | git(a)git.haskell.org | Sent: 18 December 2015 17:33 | To: ghc-commits(a)haskell.org | Subject: [commit: ghc] master: SrcLoc: Eliminate constructors of RealSrcSpan | (987426c) | | Repository : ssh://git@git.haskell.org/ghc | | On branch : master | Link : | https://na01.safelinks.protection.outlook.com/?url=http%3a%2f%2fghc.haskell. | org%2ftrac%2fghc%2fchangeset%2f987426c04fdea33d9e02c2a195d6885248b77574%2fgh | c&data=01%7c01%7csimonpj%40064d.mgd.microsoft.com%7c02a06bd4542a4d06ef8008d3 | 07d17760%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=SsUgj9Ei9cT5%2fuiDnBAD | G9cc268AQ4cx8s2t2hY94t4%3d | | >--------------------------------------------------------------- | | commit 987426c04fdea33d9e02c2a195d6885248b77574 | Author: Ben Gamari <bgamari.foss(a)gmail.com> | Date: Fri Dec 18 17:21:22 2015 +0100 | | SrcLoc: Eliminate constructors of RealSrcSpan | | This type is occurs very often but previously had multiple, meaning it | could not be unboxed. Even worse, these constructors didn't offer any | compelling safety benefits. Thankfully, the type is abstract, so | changing the representation to be a single-constructor type was quite | straightforward. | | Reviewers: austin | | Subscribers: alanz, thomie, hvr | | Differential Revision: https://phabricator.haskell.org/D1657 | | | >--------------------------------------------------------------- | | 987426c04fdea33d9e02c2a195d6885248b77574 | compiler/basicTypes/SrcLoc.hs | 95 ++++++++++++++++++---------------------- | --- | 1 file changed, 40 insertions(+), 55 deletions(-) | | diff --git a/compiler/basicTypes/SrcLoc.hs b/compiler/basicTypes/SrcLoc.hs | index e171e70..524da78 100644 | --- a/compiler/basicTypes/SrcLoc.hs | +++ b/compiler/basicTypes/SrcLoc.hs | @@ -240,7 +240,7 @@ instance Data SrcSpan where -} | | {- | | -A SrcSpan delimits a portion of a text file. It could be represented | +A 'RealSrcSpan' delimits a portion of a text file. It could be | +represented | by a pair of (line,column) coordinates, but in fact we optimise slightly | by using more compact representations for single-line and zero-length | spans, both of which are quite common. | @@ -250,28 +250,17 @@ span. That is, a span of (1,1)-(1,2) is one character | long, and a span of (1,1)-(1,1) is zero characters long. | -} | data RealSrcSpan | - = SrcSpanOneLine -- a common case: a single line | - { srcSpanFile :: !FastString, | - srcSpanLine :: {-# UNPACK #-} !Int, | - srcSpanSCol :: {-# UNPACK #-} !Int, | - srcSpanECol :: {-# UNPACK #-} !Int | - } | - | - | SrcSpanMultiLine | + = RealSrcSpan' | { srcSpanFile :: !FastString, | srcSpanSLine :: {-# UNPACK #-} !Int, | srcSpanSCol :: {-# UNPACK #-} !Int, | srcSpanELine :: {-# UNPACK #-} !Int, | srcSpanECol :: {-# UNPACK #-} !Int | } | - | - | SrcSpanPoint | - { srcSpanFile :: !FastString, | - srcSpanLine :: {-# UNPACK #-} !Int, | - srcSpanCol :: {-# UNPACK #-} !Int | - } | deriving (Eq, Typeable) | | +-- | A 'SrcSpan' identifies either a specific portion of a text file | +-- or a human-readable description of a location. | data SrcSpan = | RealSrcSpan !RealSrcSpan | | UnhelpfulSpan !FastString -- Just a general indication | @@ -296,15 +285,11 @@ srcLocSpan (UnhelpfulLoc str) = UnhelpfulSpan str | srcLocSpan (RealSrcLoc l) = RealSrcSpan (realSrcLocSpan l) | | realSrcLocSpan :: RealSrcLoc -> RealSrcSpan -realSrcLocSpan (SrcLoc file | line col) = SrcSpanPoint file line col | +realSrcLocSpan (SrcLoc file line col) = RealSrcSpan' file line col line | +col | | -- | Create a 'SrcSpan' between two points in a file mkRealSrcSpan :: | RealSrcLoc -> RealSrcLoc -> RealSrcSpan -mkRealSrcSpan loc1 loc2 | - | line1 == line2 = if col1 == col2 | - then SrcSpanPoint file line1 col1 | - else SrcSpanOneLine file line1 col1 col2 | - | otherwise = SrcSpanMultiLine file line1 col1 line2 col2 | +mkRealSrcSpan loc1 loc2 = RealSrcSpan' file line1 col1 line2 col2 | where | line1 = srcLocLine loc1 | line2 = srcLocLine loc2 | @@ -312,6 +297,16 @@ mkRealSrcSpan loc1 loc2 | col2 = srcLocCol loc2 | file = srcLocFile loc1 | | +-- | 'True' if the span is known to straddle only one line. | +isOneLineRealSpan :: RealSrcSpan -> Bool isOneLineRealSpan | +(RealSrcSpan' _ line1 _ line2 _) | + = line1 == line2 | + | +-- | 'True' if the span is a single point isPointRealSpan :: | +RealSrcSpan -> Bool isPointRealSpan (RealSrcSpan' _ line1 col1 line2 | +col2) | + = line1 == line2 && col1 == col2 | + | -- | Create a 'SrcSpan' between two points in a file mkSrcSpan :: SrcLoc - | > SrcLoc -> SrcSpan mkSrcSpan (UnhelpfulLoc str) _ = UnhelpfulSpan str @@ - | 331,11 +326,7 @@ combineSrcSpans (RealSrcSpan span1) (RealSrcSpan span2) | -- within both spans. Assumes the "file" part is the same in both inputs | combineRealSrcSpans :: RealSrcSpan -> RealSrcSpan -> RealSrcSpan | combineRealSrcSpans span1 span2 | - = if line_start == line_end | - then if col_start == col_end | - then SrcSpanPoint file line_start col_start | - else SrcSpanOneLine file line_start col_start col_end | - else SrcSpanMultiLine file line_start col_start line_end col_end | + = RealSrcSpan' file line_start col_start line_end col_end | where | (line_start, col_start) = min (srcSpanStartLine span1, srcSpanStartCol | span1) | (srcSpanStartLine span2, srcSpanStartCol | span2) @@ -392,21 +383,10 @@ srcSpanEndLine :: RealSrcSpan -> Int | srcSpanStartCol :: RealSrcSpan -> Int srcSpanEndCol :: RealSrcSpan -> Int | | -srcSpanStartLine SrcSpanOneLine{ srcSpanLine=l } = l -srcSpanStartLine | SrcSpanMultiLine{ srcSpanSLine=l } = l -srcSpanStartLine SrcSpanPoint{ | srcSpanLine=l } = l | - | -srcSpanEndLine SrcSpanOneLine{ srcSpanLine=l } = l -srcSpanEndLine | SrcSpanMultiLine{ srcSpanELine=l } = l -srcSpanEndLine SrcSpanPoint{ | srcSpanLine=l } = l | - | -srcSpanStartCol SrcSpanOneLine{ srcSpanSCol=l } = l -srcSpanStartCol | SrcSpanMultiLine{ srcSpanSCol=l } = l -srcSpanStartCol SrcSpanPoint{ | srcSpanCol=l } = l | - | -srcSpanEndCol SrcSpanOneLine{ srcSpanECol=c } = c -srcSpanEndCol | SrcSpanMultiLine{ srcSpanECol=c } = c -srcSpanEndCol SrcSpanPoint{ | srcSpanCol=c } = c | +srcSpanStartLine RealSrcSpan'{ srcSpanSLine=l } = l srcSpanEndLine | +RealSrcSpan'{ srcSpanELine=l } = l srcSpanStartCol RealSrcSpan'{ | +srcSpanSCol=l } = l srcSpanEndCol RealSrcSpan'{ srcSpanECol=c } = c | | {- | ************************************************************************ | @@ -462,14 +442,17 @@ instance Show RealSrcLoc where | | -- Show is used by Lexer.x, because we derive Show for Token instance Show | RealSrcSpan where | - show (SrcSpanOneLine file l sc ec) | + show span@(RealSrcSpan' file sl sc el ec) | + | isPointRealSpan span | + = "SrcSpanPoint " ++ show file ++ " " ++ intercalate " " (map show | + [sl,sc]) | + | + | isOneLineRealSpan span | = "SrcSpanOneLine " ++ show file ++ " " | - ++ intercalate " " (map show [l,sc,ec]) | - show (SrcSpanMultiLine file sl sc el ec) | + ++ intercalate " " (map show [sl,sc,ec]) | + | + | otherwise | = "SrcSpanMultiLine " ++ show file ++ " " | ++ intercalate " " (map show [sl,sc,el,ec]) | - show (SrcSpanPoint file l c) | - = "SrcSpanPoint " ++ show file ++ " " ++ intercalate " " (map show | [l,c]) | | | instance Outputable RealSrcSpan where | @@ -500,15 +483,22 @@ pprUserSpan _ (UnhelpfulSpan s) = ftext s | pprUserSpan show_path (RealSrcSpan s) = pprUserRealSpan show_path s | | pprUserRealSpan :: Bool -> RealSrcSpan -> SDoc -pprUserRealSpan show_path | (SrcSpanOneLine src_path line start_col end_col) | +pprUserRealSpan show_path span@(RealSrcSpan' src_path line col _ _) | + | isPointRealSpan span | = hcat [ ppWhen show_path (pprFastFilePath src_path <> colon) | , int line <> colon | - , int start_col | - , ppUnless (end_col - start_col <= 1) (char '-' <> int (end_col - | 1)) ] | + , int col ] | + | +pprUserRealSpan show_path span@(RealSrcSpan' src_path line scol _ ecol) | + | isOneLineRealSpan span | + = hcat [ ppWhen show_path (pprFastFilePath src_path <> colon) | + , int line <> colon | + , int scol | + , ppUnless (ecol - scol <= 1) (char '-' <> int (ecol - 1)) ] | -- For single-character or point spans, we just | -- output the starting column number | | -pprUserRealSpan show_path (SrcSpanMultiLine src_path sline scol eline ecol) | +pprUserRealSpan show_path (RealSrcSpan' src_path sline scol eline ecol) | = hcat [ ppWhen show_path (pprFastFilePath src_path <> colon) | , parens (int sline <> comma <> int scol) | , char '-' | @@ -516,11 +506,6 @@ pprUserRealSpan show_path (SrcSpanMultiLine src_path | sline scol eline ecol) | where | ecol' = if ecol == 0 then ecol else ecol - 1 | | -pprUserRealSpan show_path (SrcSpanPoint src_path line col) | - = hcat [ ppWhen show_path (pprFastFilePath src_path <> colon) | - , int line <> colon | - , int col ] | - | {- | ************************************************************************ | * * | | _______________________________________________ | ghc-commits mailing list | ghc-commits(a)haskell.org | https://na01.safelinks.protection.outlook.com/?url=http%3a%2f%2fmail.haskell | .org%2fcgi-bin%2fmailman%2flistinfo%2fghc- | commits&data=01%7c01%7csimonpj%40064d.mgd.microsoft.com%7c02a06bd4542a4d06ef | 8008d307d17760%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=L7r%2buyrlMk0mSh | hWTNgi7%2bfhTgu2Osarn6rI6z706cw%3d

2 2

Re: Haddock for GHC 8.0
by Gautier DI FOLCO 21 Dec '15

21 Dec '15

------------------------------ > tl;dr: Help needed to bring Haddock up-to-date for the GHC 8.0 release. > > > Hello everyone, > > As you likely know, the GHC 8.0 release cycle is quickly drawing to a > close. One of the tasks that remains outstanding is porting the patches > on Haddock's `ghc-head` branch to `master`. This task is bound to > require a bit more work than usual this time around as both GHC and > Haddock have evolved significantly (which is a good thing!). Sadly, > Austin and I are quite busy at the moment and would be hard-pressed to > undertake such a task ourselves. Consequently, we need a volunteer to > pick up this work. > > At the moment the GHC release schedule is largely determined by progress > on the no-kinds branch but we expect to have a release candidate out > within the next two weeks. If we aren't able to find someone to bring > haddock up-to-date by mid-January I'm afraid we'll have to release GHC > 8.0 with the current state of the `ghc-head` branch. Considering the > significant effort that went into this summer's GSoC projects, this > would be a shame. > > If you think you might be able to contribute to this effort, please say > so in this thread. > > Thanks for all of your effort, > > - Ben >

2 2

Re: Performance of pattern checker on OptCoercions
by Thomas Miedema 20 Dec '15

20 Dec '15

> I have a small question though: do we need the opposites too (I mean > `-fno-full-guard-reasoning` and ` -Wtoo-many-guards`)? I cannot think of > a scenario where we do or am I missing something? > A scenario where `-fno-full-guard-reasoning` could be useful: you add `-ffull-guard-reasoning` to ghc-options in your .cabal file, but switch it off in an OPTIONS_GHC pragma in a specific file. But `-f` and `-W` flags always come in pairs, so that is easy. You would add `-Wtoo-many-guards` to `standardWarnings` in compiler/main/DynFlags.hs. Others? If I get the green light, this is a matter of minutes to change :) > If it is not too much trouble, please submit your patch to Phabricator. Then others can review, and harbormaster can validate it.

1 0

Re: Performance of pattern checker on OptCoercions
by Richard Eisenberg 20 Dec '15

20 Dec '15

I hereby abstain from debating the color on this particular bikeshed. My taste in bikeshed colors has proven to be unreliable. Richard On Dec 19, 2015, at 5:32 PM, George Karachalias <george.karachalias(a)gmail.com> wrote: > Hi Thomas, > > On Sat, Dec 19, 2015 at 8:36 PM, Thomas Miedema <thomasmiedema(a)gmail.com> wrote: > Hi George, > > here is an even simpler design! > > When `-ffull-guard-reasoning` is on, use the new pattern match checker. > > When `-ffull-guard-reasoning` is off (the default), try to use the new pattern match checker, but if the number of guards is more than 20, > 1. fall back to the old (simple) pattern match checker, and > 2. issue a `too-many-guards` warning > > The warning can be turned off with `-Wno-too-many-guards`. > > What do you think? > > Sounds nice and easy to change :) > I have a small question though: do we need the opposites too (I mean > `-fno-full-guard-reasoning` and ` -Wtoo-many-guards`)? I cannot think of > a scenario where we do or am I missing something? > > Others? If I get the green light, this is a matter of minutes to change :) > > George > > -- > things you own end up owning you

1 0

Re: Performance of pattern checker on OptCoercions
by Richard Eisenberg 19 Dec '15

19 Dec '15

On Dec 13, 2015, at 1:07 PM, George Karachalias <george.karachalias(a)gmail.com> wrote: > Sounds nice and I think it is much better than giving up guards completely. The only thing I > don't like much is having so many different flags concerning the check because I don't want > it to become "too complex" for users. I guess we have to live with it though :) Yes. I'd like this to be simpler. Perhaps there is a way to combine all of this into one flag. I see 4 settings: 1. No guard match checking. 2. Check guards; bail and issue warning on tough cases. 3. Check guards; bail with no warning on tough cases. 4. Check guards at any cost. We could have these be 1. -fwarn-pattern-guards=none 2. -fwarn-pattern-guards=try 3. -fwarn-pattern-guards=try-quiet 4. -fwarn-pattern-guards=do -- there is no "try" Is that simpler? Maybe. Richard

2 2

arc land changes author
by Joachim Breitner 19 Dec '15

19 Dec '15

Hi, I was just about to apply a DR by David. First I ran $ arc patch D390 which put me on a feature branch. Then I reworded the commit with $ git commit --amend and then I tried to land it with $ arc land Luckily I also passed --hold... It even asked me if I want to land the commit although I am not the author: This branch has revision 'D390: Reorder GHC.List; fix performance regressions' but you are not the author. Land this revision by dfeuer? [y/N] y But it still put me in the author field: $ git show HEAD commit a387c359b99667ce55016de3816b5d9873a155db Author: Joachim Breitner <mail(a)joachim-breitner.de> Date: Wed Oct 29 08:17:05 2014 +0100 Reorder GHC.List; fix performance regressions Summary: Rearrange some oddly placed code. (and undid my commit message changes....) Did I do something wrong? Anyways, it said something along the lines of Cleaning up feature branch... (Use `git checkout -b 'arcpatch-D390' 'a3ceb0e7f44d53f728fb8bd0dfb4c97297818029'` if you want it back.) so with $ git commit --amend -C a3ceb0e7f44d53f728fb8bd0dfb4c97297818029 I was able to get the edited commit message with the right author back. Greetings, Joachim -- Joachim “nomeata” Breitner mail(a)joachim-breitner.de • http://www.joachim-breitner.de/ Jabber: nomeata(a)joachim-breitner.de • GPG-Key: 0xF0FBF51F Debian Developer: nomeata(a)debian.org

2 2

ghc-split??
by Simon Peyton Jones 18 Dec '15

18 Dec '15

I’m getting this with a clean HEAD on Linux. Any ideas? Simon ghc-stage1: could not execute: /5playpen/simonpj/HEAD-4/inplace/lib/bin/ghc-split make[1]: *** [libraries/ghc-prim/dist-install/build/GHC/Types.o] Error 1

3 4

Re: Performance of pattern checker on OptCoercions
by Richard Eisenberg 17 Dec '15

17 Dec '15

On Dec 17, 2015, at 1:31 PM, George Karachalias <george.karachalias(a)gmail.com> wrote: > > My single last concern is what I wrote earlier: "Flags 2 and 3 are a bit tricky: Actual bailing > will uglify the code and make it much more complicated so I would prefer in practice to just > check up front with a good metric whether the possibility to be non-performant is high and in > these cases just not call the checker at all." > > If we agree on this one, I am gonna wrap it up in the next 2 days. :-) I'm quite happy with this. My thought is that if it's a bit wrong, that's OK -- we'll refine the settings over time. Others? Richard

1 0

Re: Performance of pattern checker on OptCoercions
by Richard Eisenberg 17 Dec '15

17 Dec '15

On Dec 17, 2015, at 8:15 AM, George Karachalias <george.karachalias(a)gmail.com> wrote: > What do you think? I have already implemented a first patch with two options: If the number of > guards is small run the check and say nothing. If not, check for 2 possible flags: > -fwarn-pattern-guards ==> Give it your best > -fwarn-ignore-pattern-guards ==> Do not run the check on the cases with many guards without a warning > None of the two enabled? ==> Give a warning of the form: > > T783.hs:4:1: warning: > Pattern match checking failed for an equation for foo > due to the big number of guards > (Use -fwarn-ignore-pattern-guards if you want to hide this message > or -fwarn-pattern-guards if you want the checker to run no matter what) I don't have much comment on your implementation techniques, as I don't know this end of the operation well enough to have an opinion. For the point above: what if both flags are enabled? Richard

1 0