mpickering (Matthew Pickering)Administrator
User

Projects

Today

  • Clear sailing ahead.

Tomorrow

  • Clear sailing ahead.

Tuesday

  • Clear sailing ahead.

Badges

User Since
Dec 18 2014, 1:25 PM (118 w, 3 d)
Roles
Administrator
Availability
Available

Recent Activity

Yesterday

mpickering added a comment to D3379: Allow operators as record pattern synonym fields.

It is the same as any other infix operator. You can set it with infix declarations.

Really? I can't seem to make it work with this patch. For instance, if you try to set (·) as infixr:

{-# LANGUAGE PatternSynonyms, ViewPatterns #-}
module T13454 where

pattern MkOp :: Op -> Exp -> Exp -> Exp
pattern MkOp {(·), a, b} <- (splitOp -> Just ((·), a, b))
  where MkOp (·) a b      = a · b · b

infixr 5 ·

data Exp = Val Int | Add Exp Exp | Mul Exp Exp deriving Show

type Op = Exp -> Exp -> Exp

splitOp :: Exp -> Maybe (Op, Exp, Exp)
splitOp (Add a b) = Just (Add, a, b)
splitOp (Mul a b) = Just (Mul, a, b)
splitOp _         = Nothing

main :: IO ()
main = do
  print $ MkOp Add (Val 1) (Val 2)
  print $ Val 1 `Add` (Val 2 `Add` Val 2)

I would expect those two lines in main to print the same thing, but it doesn't:

Add (Add (Val 1) (Val 2)) (Val 2)
Add (Val 1) (Add (Val 2) (Val 2))

Also, this doesn't result in an error:

{-# LANGUAGE PatternSynonyms, ViewPatterns #-}
module T13454 where

pattern MkOp :: Op -> Exp -> Exp -> Exp
pattern MkOp {(·), a, b} <- (splitOp -> Just ((·), a, b))
  where MkOp (·) a b      = a · b · b

infix 5 ·

data Exp = Val Int | Add Exp Exp | Mul Exp Exp deriving Show

type Op = Exp -> Exp -> Exp

splitOp :: Exp -> Maybe (Op, Exp, Exp)
splitOp (Add a b) = Just (Add, a, b)
splitOp (Mul a b) = Just (Mul, a, b)
splitOp _         = Nothing
Sat, Mar 25, 5:53 PM
mpickering added a comment to D3379: Allow operators as record pattern synonym fields.

One thing I'm not clear on is how the fixity of record pattern synonym field operators work. Is there a default fixity? Can it be configured? Is whatever the current behavior is documented in the users' guide?

Sat, Mar 25, 3:52 AM

Fri, Mar 24

mpickering retitled D3378: Print module when dumping rules from Print module when dumping rule to Print module when dumping rules.
Fri, Mar 24, 5:50 PM
mpickering created D3379: Allow operators as record pattern synonym fields.
Fri, Mar 24, 5:47 PM
mpickering created D3378: Print module when dumping rules.
Fri, Mar 24, 5:25 PM
mpickering added a comment to D3361: Show valid substitutions for typed holes.

Yes, this is a good idea but the implementation of the function could be improved.

Fri, Mar 24, 9:58 AM

Thu, Mar 23

mpickering added inline comments to D3377: Only use locally bound variables in pattern synonym declarations.
Thu, Mar 23, 10:06 AM
mpickering updated the diff for D3377: Only use locally bound variables in pattern synonym declarations.
  • updated comments
Thu, Mar 23, 10:05 AM
mpickering created D3377: Only use locally bound variables in pattern synonym declarations.
Thu, Mar 23, 9:20 AM
mpickering created D3376: Remove unused argument from importSuggestions.
Thu, Mar 23, 9:16 AM

Tue, Mar 21

mpickering accepted D3361: Show valid substitutions for typed holes.

Good job!

Tue, Mar 21, 3:51 PM
mpickering added inline comments to D3361: Show valid substitutions for typed holes.
Tue, Mar 21, 6:25 AM
mpickering requested changes to D3361: Show valid substitutions for typed holes.

Ok the code looks good now but still some error message wibbles I think.

Tue, Mar 21, 5:06 AM
mpickering created D3371: Improve tracing in OccurAnal.
Tue, Mar 21, 4:49 AM

Mon, Mar 20

mpickering added a comment to D3361: Show valid substitutions for typed holes.
In D3361#96734, @Tritlo wrote:

So I've tested the performance a bit, and it does not seem to slow it down appreciably. I generated a file with a lot of Ids, with the following

main :: IO ()
main = writeFile f $ unlines $ "module ManyIds where":(map out [0..10000])
 where f = "ManyIds.hs"
       out i = "a" ++ show i ++ " :: Int -> Int\n"
            ++ "a" ++ show i ++ " _ = " ++ show i

in another file, manyIdsTest.hs, was the following:

import ManyIds

test :: Int -> Int
test = _

main :: IO ()
main = return ()

Compiling ManyIds took some time with both my HEAD and GHC 8.0.2, which is why I reduced the number of identifiers down to 10_000 from 100_000.

Timing it with unix time, I got the following:
ghc -o test manyIdsTest.hs 20,28s user 0,83s system 91% cpu 23,041 total
./inplace/bin/ghc-stage2 -o test manyIdsTest.hs -fno-max-valid-substitutions 21,59s user 1,38s system 88% cpu 25,874 total
So even with -fno-max-valid-substitutions, it's not appreciably slower (I suspect the extra second is mostly from writing stdout to a file)

Mon, Mar 20, 12:52 PM
mpickering awarded Contributor to recipient: Tritlo.
Mon, Mar 20, 10:34 AM
mpickering requested changes to D3361: Show valid substitutions for typed holes.
Mon, Mar 20, 6:53 AM
mpickering added a comment to D3361: Show valid substitutions for typed holes.
In D3361#96714, @Tritlo wrote:
In D3361#96696, @Tritlo wrote:

It seems the error messages now contain redundant information when a binding is both "relevant" and "very relevant" (type correct).

There are also other changes to the error messages which are not accounted for.

Please could you add some tests which stress more complicated cases and the distinction between the two implementations you described?

I will look into the unaccounted for changes, though I'm not really sure where the changes originate.

Perhaps it is because you are forcing instances to be loaded which are not imported by them module you are testing.
Try changing the error message to print out all possible instances and then seeing which ones are different before and after your patch.

What do you mean "test the distinction"? Do you mean that I should make a test to show that a -> IO () in the example is not found?

Yes ideally. If that is hard to do don't worry about it but some tests would be good.

Would you prefer for it to not show things that are already displayed in "relevant bindings"? I would like to see them in both places, even though the output is a bit longer, since a relevant binding isn't necessarily a valid substitution.

I don't think showing them in both places is good. There are four distinct classifications of bindings in my mind:

  1. In local scope but not type correct
  2. In local scope but a candidate for the hole
  3. In global scope and a candidate for the hole
  4. In global scope and no type correct

    Currently we display 1 & 2 in the error messages, you are proposing that we display 1,2 and 3. In your patch you display (1 & 2) and (2 & 3) which is where the redundancy comes in.

    This also seems like it could be quite expensive when there are many identifiers in scope (say 100 000) or using lots of holes will cause a lot of work to be repeated. Have you tried this in these situations? A better solution might be to first build a map from types to identifiers and then reuse that for each hole.

I've fixed the additional instances being brought into scope, I didn't realize that tcLookupImported_maybe tried to be smart when it encountered something it didn't know about, which caused it to import interface files (I think). Still new to GHC internals!

I suppose you're right about (1 & 2) and (2 & 3). I'll change it to display only valid substitutions in the global scope. This will also reduce some of the work being duplicated between relevantBindings and validSubstitutions.

I'll also add some more tests, though I'm not sure how. Is there any resource I can look at into how you add tests?

Performance will definitely be an issue if there are a lot of identifiers in scope, but as it currently is, it stops after having found -fmax-valid-substitutions that fit the hole. Having a map from types to identifiers would definitely help for many holes, but I think that for a high number of identifiers, it might slow things down a bit. I believe though that since this only fires when there are holes, this is not a big issue. Any thoughts on that?

Mon, Mar 20, 6:53 AM
mpickering added a comment to D3361: Show valid substitutions for typed holes.
In D3361#96696, @Tritlo wrote:

It seems the error messages now contain redundant information when a binding is both "relevant" and "very relevant" (type correct).

There are also other changes to the error messages which are not accounted for.

Please could you add some tests which stress more complicated cases and the distinction between the two implementations you described?

I will look into the unaccounted for changes, though I'm not really sure where the changes originate.

Perhaps it is because you are forcing instances to be loaded which are not imported by them module you are testing.
Try changing the error message to print out all possible instances and then seeing which ones are different before and after your patch.

Mon, Mar 20, 5:25 AM

Sun, Mar 19

mpickering requested changes to D3361: Show valid substitutions for typed holes.

It seems the error messages now contain redundant information when a binding is both "relevant" and "very relevant" (type correct).

Sun, Mar 19, 4:59 PM

Fri, Mar 17

mpickering added a comment to D3354: Update link to paper about demand analyser in user guide.

The paper this links to is not finished but at least the first half is fairly complete and explains the purpose of the strictness analyser.

Fri, Mar 17, 5:42 AM

Thu, Mar 16

mpickering created D3354: Update link to paper about demand analyser in user guide.
Thu, Mar 16, 2:11 PM

Tue, Mar 14

mpickering accepted D3341: Reimplement minusList using Set.
Tue, Mar 14, 5:36 PM
mpickering added inline comments to D3341: Reimplement minusList using Set.
Tue, Mar 14, 4:04 PM
mpickering added inline comments to D3341: Reimplement minusList using Set.
Tue, Mar 14, 3:59 PM
mpickering added a comment to D3341: Reimplement minusList using Set.

Is this actually faster given the cost of constructing a set each time? I don't have good intuition for this but I have thought about doing it before.

Tue, Mar 14, 2:34 PM

Sun, Mar 12

mpickering planned changes to D3322: WIP: Reduce stderr output by passing -v0 to ghc-pkg.

Not the right place. The stderr output of ./validate is quite different on my machine to harbormaster which makes this difficult to debug.

Sun, Mar 12, 3:22 AM

Sat, Mar 11

mpickering retitled D3326: Replace debugging trace with a proper WARN from Replace debugging trace with a proper WARN Unfortunately this created an import cycle but it is the proper way to do it. Opinions will vary whether this is worth it. to Replace debugging trace with a proper WARN.
Sat, Mar 11, 3:50 PM
mpickering created D3326: Replace debugging trace with a proper WARN.
Sat, Mar 11, 3:40 PM
mpickering created D3322: WIP: Reduce stderr output by passing -v0 to ghc-pkg.
Sat, Mar 11, 2:23 PM
mpickering committed rGHC7b095b991879: Emit Core lint warnings on stderr, fix #13342 (authored by ThreeFx).
Emit Core lint warnings on stderr, fix #13342
Sat, Mar 11, 1:47 PM

Wed, Mar 8

mpickering added inline comments to D3299: Fixed QuasiQuotation test and output.
Wed, Mar 8, 2:26 PM
mpickering added a comment to D3299: Fixed QuasiQuotation test and output.

What are these tests meant to test? Why does qq006 depend on parsec, that doesn't seem very robust.

Wed, Mar 8, 1:40 PM

Mon, Mar 6

mpickering committed rGHCcd9c7094756e: Update .mailmap [skip ci] (authored by mpickering).
Update .mailmap [skip ci]
Mon, Mar 6, 4:44 PM

Fri, Mar 3

mpickering abandoned D3270: Fix completesig04 test properly.

D3266 was merged.

Fri, Mar 3, 10:32 AM
mpickering created D3270: Fix completesig04 test properly.
Fri, Mar 3, 9:32 AM

Wed, Mar 1

mpickering added a comment to D3258: Bump bytes allocated for T12234.

Gipeda graph which will hopefully catch up to this point - https://perf.haskell.org/ghc/#graph/tests/alloc/T12234;hl=55efc9718b520ef354e32c15c4b49cdfecce412f

Wed, Mar 1, 6:14 PM
mpickering resigned from D3073: Do not drop dead code in the desugarer.
Wed, Mar 1, 12:48 PM
mpickering accepted D3243: Don't allow orphan COMPLETE pragmas (#13349).

Thanks Reid

Wed, Mar 1, 3:13 AM

Sun, Feb 26

mpickering added a comment to D3214: Make XNegativeLiterals treat -0.0 as negative 0.

It also appears the repository was set incorrectly. Using arc diff is much easier in my experience!

Sun, Feb 26, 5:29 AM
mpickering changed the repository for D3214: Make XNegativeLiterals treat -0.0 as negative 0 from rGHCDIFF ghc-diffs to rGHC Glasgow Haskell Compiler.
Sun, Feb 26, 5:24 AM
mpickering added reviewers for D3214: Make XNegativeLiterals treat -0.0 as negative 0: bgamari, rwbarton.

Did you create this using the web interface? It is much better if you can create the diff using arc so that CI runs.

Sun, Feb 26, 5:23 AM
mpickering abandoned D3212: Doesn't require review. Delete this please..

@Nolan You can update revision by using arc diff --update D3212 in future to avoid having to create a new differential.

Sun, Feb 26, 5:21 AM

Sat, Feb 25

mpickering added a reviewer for D3213: Add -dno-debug-output to validate GhcStage1HcOpts: rwbarton.

Adding Reid who has previously expressed an opinion about the amount of compiler output.

Sat, Feb 25, 6:07 PM
mpickering added a comment to D3213: Add -dno-debug-output to validate GhcStage1HcOpts.

The build log js around 1200 lines after this change.

Sat, Feb 25, 5:22 PM
mpickering updated the diff for D3213: Add -dno-debug-output to validate GhcStage1HcOpts.

Put the flag in the right place..

Sat, Feb 25, 4:34 PM
mpickering created D3213: Add -dno-debug-output to validate GhcStage1HcOpts.
Sat, Feb 25, 4:09 PM
mpickering updated the diff for D3113: Eliminate ListSetOps from imp_trust_pkgs.

Redundant imports

Sat, Feb 25, 2:21 PM

Fri, Feb 24

mpickering updated the diff for D3113: Eliminate ListSetOps from imp_trust_pkgs.
  • Fix build
Fri, Feb 24, 4:00 PM

Feb 24 2017

mpickering committed rGHC00c01200cd16: Add a comment explaining CompleteMatchSig in HsBinds (authored by mpickering).
Add a comment explaining CompleteMatchSig in HsBinds
Feb 24 2017, 11:38 AM

Feb 22 2017

mpickering closed T297: Phabricator stopped sending email as "Resolved".

@austin tells me that this should now be fixed.

Feb 22 2017, 7:40 AM

Feb 21 2017

mpickering added a comment to D3139: Try to fix boolformula into parsing "foo, bar, baz" into "And [foo, bar, baz]"instead of "And [foo, And [bar, baz]]".

Yes you are right.

Feb 21 2017, 5:29 AM
mpickering added a comment to D3167: Simon's early-inline patch, take 2.

It is really hard to see what is going on here. Perhaps you could put each commit up as a separate diff?

Feb 21 2017, 4:31 AM

Feb 19 2017

mpickering commandeered D3113: Eliminate ListSetOps from imp_trust_pkgs.
Feb 19 2017, 3:59 AM

Feb 18 2017

mpickering accepted rGHC3eb737ee3f90: Generalize CmmUnwind and pass unwind information through NCG.
Feb 18 2017, 12:50 PM

Feb 17 2017

mpickering awarded Contributor to recipient: jcarey.
Feb 17 2017, 3:43 AM

Feb 16 2017

mpickering accepted D3139: Try to fix boolformula into parsing "foo, bar, baz" into "And [foo, bar, baz]"instead of "And [foo, And [bar, baz]]".

Nearly there!

Feb 16 2017, 4:46 AM
mpickering added a comment to D3139: Try to fix boolformula into parsing "foo, bar, baz" into "And [foo, bar, baz]"instead of "And [foo, And [bar, baz]]".

Does your patch actually compile? arc is working again now, please can you update the diff so I can see it more clearly?

Feb 16 2017, 3:47 AM

Feb 15 2017

mpickering requested changes to D3139: Try to fix boolformula into parsing "foo, bar, baz" into "And [foo, bar, baz]"instead of "And [foo, And [bar, baz]]".

I don't think a test is necessary.

Feb 15 2017, 4:53 AM

Feb 14 2017

mpickering removed a reviewer for D3138: Testing...: rwbarton.
Feb 14 2017, 10:59 AM
mpickering accepted D3138: Testing....

Great revision.

Feb 14 2017, 10:53 AM
mpickering requested changes to D3138: Testing....
Feb 14 2017, 10:49 AM
mpickering committed rGHC392cec4da9a7: Update .mailmap [skip ci] (authored by mpickering).
Update .mailmap [skip ci]
Feb 14 2017, 10:16 AM
mpickering awarded Contributor to recipient: amindfv.
Feb 14 2017, 10:13 AM

Feb 13 2017

mpickering raised a concern with rGHC3eb737ee3f90: Generalize CmmUnwind and pass unwind information through NCG.

This commit seems like it broke the OS X build.

Feb 13 2017, 3:14 AM

Feb 12 2017

mpickering awarded Contributor to recipient: bollu.
Feb 12 2017, 4:49 AM

Feb 10 2017

mpickering updated the diff for D3113: Eliminate ListSetOps from imp_trust_pkgs.

rebase

Feb 10 2017, 10:46 AM
mpickering accepted D3124: Relax test TH_addCStub2 so it succeeds on travis..

This passes for me now on OS X.

Feb 10 2017, 10:24 AM
mpickering added a comment to D3124: Relax test TH_addCStub2 so it succeeds on travis..

Like I said on the commit. The test also fails on OS X due to differences between clang and gcc I guess. I can try to test this locally so you are not waiting years for the builder to finish.

Feb 10 2017, 9:22 AM
mpickering added a comment to rGHCb9bebd8cedcc: Implement addCStub in template-haskell..

Also failing on OS X

Feb 10 2017, 3:42 AM

Feb 9 2017

mpickering added a comment to D3117: bufWrite: Save extra syscall when data fills handle buffer completely..

What command were you running?

Feb 9 2017, 7:11 PM

Feb 7 2017

mpickering updated the Trac tickets for D3103: Pass -v0 to ghc-pkg to reduce noise in build ouput.
Feb 7 2017, 5:13 PM
mpickering created D3103: Pass -v0 to ghc-pkg to reduce noise in build ouput.
Feb 7 2017, 5:12 PM
mpickering abandoned D3100: Pass -v0 to ghc-pkg when invoked by ghc-cabal.

This isn't the right fix it seems.

Feb 7 2017, 4:33 PM
mpickering updated the Trac tickets for D3100: Pass -v0 to ghc-pkg when invoked by ghc-cabal.
Feb 7 2017, 4:30 PM
mpickering created D3100: Pass -v0 to ghc-pkg when invoked by ghc-cabal.
Feb 7 2017, 4:29 PM
mpickering retitled D2822: Allow type defaulting for multi-param type classes with ExtendedDefaultRules from Update: ExtendedDefaultRules to default multiparameter typeclasses to Allow type defaulting for multi-param type classes with ExtendedDefaultRules.
Feb 7 2017, 10:43 AM

Feb 6 2017

mpickering awarded Contributor to recipient: arybczak.
Feb 6 2017, 1:46 PM
mpickering retitled D3057: Correct behaviour of push_bang_into_newtype when the pattern match has no arguments from Do not push bangs through newtypes when there are no arguments to Correct behaviour of push_bang_into_newtype when the pattern match has no arguments.
Feb 6 2017, 12:52 PM
mpickering requested changes to D2822: Allow type defaulting for multi-param type classes with ExtendedDefaultRules.

Also the differential description needs to be updated to describe the feature as this will become the commit message.

Feb 6 2017, 12:40 PM
mpickering added a comment to D2822: Allow type defaulting for multi-param type classes with ExtendedDefaultRules.

@amindfv Please can you upload you SSH key and update the diff (using arc diff --update D2822) so that the changes are pushed to the staging repo and built by harbormaster?

Feb 6 2017, 12:40 PM

Feb 4 2017

mpickering updated the diff for D3057: Correct behaviour of push_bang_into_newtype when the pattern match has no arguments.
  • Simon's comments and update existing test to test strictness properties
Feb 4 2017, 8:32 AM
mpickering awarded Contributor to recipient: anniecherkaev.
Feb 4 2017, 7:49 AM
mpickering added a comment to D3064: Don't return empty initial uncovered set for an unsat context.

I don't really think the pattern match checker is the right place to issue an inaccessibility warning so I think this is done for now.

Feb 4 2017, 5:41 AM
mpickering updated the diff for D3064: Don't return empty initial uncovered set for an unsat context.
  • Choose right base commit
Feb 4 2017, 5:37 AM
mpickering updated the diff for D3065: Don't panic when printing match with RecUpd context.
  • Update with new base commit
Feb 4 2017, 5:36 AM
mpickering updated the diff for D3064: Don't return empty initial uncovered set for an unsat context.
  • Accept new test output
Feb 4 2017, 5:35 AM
mpickering committed rGHC283acec1d730: Make split sections by default work again (authored by rwbarton).
Make split sections by default work again
Feb 4 2017, 4:34 AM
mpickering closed D3070: Make split sections by default work again by committing rGHC283acec1d730: Make split sections by default work again (authored by rwbarton).
Feb 4 2017, 4:34 AM
mpickering committed rGHC4d31880a0bfb: Fix comment (old filename '.lhs') in libraries/ (authored by takenobu).
Fix comment (old filename '.lhs') in libraries/
Feb 4 2017, 4:34 AM
mpickering closed D3060: Fix comment (old filename '.lhs') in libraries/ by committing rGHC4d31880a0bfb: Fix comment (old filename '.lhs') in libraries/ (authored by takenobu).
Feb 4 2017, 4:34 AM
mpickering committed rGHC8d60d739489b: Fix comment (old file names) in compiler/ (authored by takenobu).
Fix comment (old file names) in compiler/
Feb 4 2017, 4:34 AM
mpickering closed D3076: Fix comment (old file names) in compiler/ by committing rGHC8d60d739489b: Fix comment (old file names) in compiler/ (authored by takenobu).
Feb 4 2017, 4:34 AM
mpickering committed rGHC31bb85ffc4b6: Fix comment (old file names) in rts/ (authored by takenobu).
Fix comment (old file names) in rts/
Feb 4 2017, 4:34 AM
mpickering closed D3075: Fix comment (old file names) in rts/ by committing rGHC31bb85ffc4b6: Fix comment (old file names) in rts/ (authored by takenobu).
Feb 4 2017, 4:34 AM
mpickering committed rGHC9984024a58a2: Fix comment (old file names) in includes/ (authored by takenobu).
Fix comment (old file names) in includes/
Feb 4 2017, 4:34 AM
mpickering closed D3074: Fix comment (old file names) in includes/ by committing rGHC9984024a58a2: Fix comment (old file names) in includes/ (authored by takenobu).
Feb 4 2017, 4:34 AM
mpickering accepted D3070: Make split sections by default work again.

Good job you spotted this Reid.

Feb 4 2017, 4:18 AM
mpickering awarded Contributor to recipient: takenobu.
Feb 4 2017, 4:13 AM
mpickering requested changes to D3073: Do not drop dead code in the desugarer.

This solution feels very indirect. It is obtuse to keep dead bindings by making them live bindings in an unrelated part of the code upstream and potentially very confusing.

Feb 4 2017, 4:09 AM