mpickering (Matthew Pickering)Administrator
User

Projects

Today

  • Clear sailing ahead.

Tomorrow

  • Clear sailing ahead.

Tuesday

  • Clear sailing ahead.

User Details

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

Recent Activity

Today

mpickering planned changes to D3507: WIP: RnEnv refactoring.

This now validates on 8.0. https://phabricator.haskell.org/B15572

Sun, Apr 30, 3:42 PM
mpickering created D3515: Implement sequential name lookup properly.
Sun, Apr 30, 3:34 PM
mpickering updated the diff for D3507: WIP: RnEnv refactoring.
  • Use a flag instead of checking for NoParent
Sun, Apr 30, 2:16 PM
mpickering added a comment to D3514: Revert "Fix #10923 by fingerprinting optimization level.".

Why do you want to revert this? I think this is useful to have.

Sun, Apr 30, 1:57 PM

Yesterday

mpickering added a comment to D3507: WIP: RnEnv refactoring.

I think this will validate now but is it better than before? Maybe. It seemed that to fix the tests I had to progressively make things more ugly.

Sat, Apr 29, 5:27 PM
mpickering updated the diff for D3507: WIP: RnEnv refactoring.
  • Fail if there is no parent on lookup
  • Pass the right thing to be printed into IncorrectParent
Sat, Apr 29, 5:25 PM
mpickering updated the diff for D3507: WIP: RnEnv refactoring.
  • Remove tracing functions + comment
Sat, Apr 29, 2:39 PM
mpickering updated the diff for D3507: WIP: RnEnv refactoring.
  • Lookup using GHCi lookup function for overloaded case as well
Sat, Apr 29, 1:07 PM
mpickering updated the diff for D3507: WIP: RnEnv refactoring.
  • T13622 fix
  • Add LookupSub test
  • Prefer disambiguated occurences to ambiguous occurences
Sat, Apr 29, 9:38 AM
mpickering updated the diff for D3507: WIP: RnEnv refactoring.
  • Fix test failures
Sat, Apr 29, 7:29 AM

Fri, Apr 28

mpickering planned changes to D3507: WIP: RnEnv refactoring.
Fri, Apr 28, 5:01 AM
mpickering updated the diff for D3507: WIP: RnEnv refactoring.
  • Fix inverted logic in overloaded renamer
Fri, Apr 28, 5:00 AM
mpickering retitled D3507: WIP: RnEnv refactoring from WIP: RnEnv refactoring Replace explicit call to `newGlobalBndr` with `lookupOrig` to WIP: RnEnv refactoring.
Fri, Apr 28, 3:20 AM
mpickering created D3507: WIP: RnEnv refactoring.
Fri, Apr 28, 3:19 AM

Thu, Apr 13

mpickering requested changes to D3457: Introduce -dcore2core-summary flag.

Docs?

Thu, Apr 13, 2:28 PM
mpickering added inline comments to D3454: Fix space leak in sortBy.
Thu, Apr 13, 8:10 AM
mpickering updated the summary of D3454: Fix space leak in sortBy.
Thu, Apr 13, 8:05 AM
Herald added a reviewer for D3454: Fix space leak in sortBy: austin.
Thu, Apr 13, 8:03 AM
mpickering created D3453: WIP: Remove new name logic from renamer.
Thu, Apr 13, 6:31 AM

Wed, Apr 12

mpickering committed rGHCe07cd507ff87: Split up RnEnv into 4 modules, RnUnbound, RnUtils and RnFixity (authored by mpickering).
Split up RnEnv into 4 modules, RnUnbound, RnUtils and RnFixity
Wed, Apr 12, 2:05 PM
mpickering closed D3436: Split up RnEnv into 4 modules, RnUnbound, RnUtils and RnFixity by committing rGHCe07cd507ff87: Split up RnEnv into 4 modules, RnUnbound, RnUtils and RnFixity.
Wed, Apr 12, 2:05 PM
mpickering updated the diff for D3436: Split up RnEnv into 4 modules, RnUnbound, RnUtils and RnFixity.
  • New base commit
Wed, Apr 12, 4:06 AM
mpickering added a comment to D3435: RnEnv cleanup.

This is the first step in a larger refactoring I plan in this module.

Wed, Apr 12, 4:05 AM
mpickering updated the diff for D3435: RnEnv cleanup.
  • Fixup the broken tests, I think it will be better to fix this wrinkle about returning Just an arbritrary identifier
Wed, Apr 12, 4:03 AM

Mon, Apr 10

mpickering added reviewers for D3440: Caret diagnostics: Avoid decoding whole module if only specific line is needed: Rufflewind, trofi.
Mon, Apr 10, 3:45 PM
mpickering added a dependency for D3436: Split up RnEnv into 4 modules, RnUnbound, RnUtils and RnFixity: D3435: RnEnv cleanup.
Mon, Apr 10, 12:24 PM
mpickering added a dependent revision for D3435: RnEnv cleanup: D3436: Split up RnEnv into 4 modules, RnUnbound, RnUtils and RnFixity.
Mon, Apr 10, 12:24 PM
mpickering planned changes to D3436: Split up RnEnv into 4 modules, RnUnbound, RnUtils and RnFixity.

It is failing because the base commit is D3435 which I still need to sort out.

Mon, Apr 10, 12:24 PM

Sat, Apr 8

mpickering planned changes to D3435: RnEnv cleanup.

This is quite subtle.

Sat, Apr 8, 4:51 PM
Herald added a reviewer for D3436: Split up RnEnv into 4 modules, RnUnbound, RnUtils and RnFixity: austin.
Sat, Apr 8, 2:39 PM
mpickering created D3435: RnEnv cleanup.
Sat, Apr 8, 2:34 PM

Fri, Apr 7

mpickering updated the summary of D3434: Allow qualified names to be children in export lists.
Fri, Apr 7, 1:49 PM
mpickering created D3434: Allow qualified names to be children in export lists.
Fri, Apr 7, 1:45 PM

Tue, Apr 4

mpickering edited this Badge.
Tue, Apr 4, 12:10 PM

Mon, Apr 3

mpickering added a comment to D2424: UNPACK support for sums.

Is this patch still waiting to be reviewed @osa1 ?

Mon, Apr 3, 4:59 PM
mpickering updated the diff for D3414: Replace Digraph's Node type synonym with a data type.
  • Update determinism001
Mon, Apr 3, 4:35 PM
mpickering created D3414: Replace Digraph's Node type synonym with a data type.
Mon, Apr 3, 4:29 AM

Mar 29 2017

mpickering added inline comments to D3392: Optimise common cases of GHC.setProgramDynFlags.
Mar 29 2017, 3:07 PM

Mar 27 2017

mpickering committed rGHC5025fe2435d0: -fspec-constr-keen docs typos [skip ci] (authored by mpickering).
-fspec-constr-keen docs typos [skip ci]
Mar 27 2017, 11:44 AM
mpickering committed rGHCd819e416a2a5: Only use locally bound variables in pattern synonym declarations (authored by mpickering).
Only use locally bound variables in pattern synonym declarations
Mar 27 2017, 5:28 AM
mpickering closed D3377: Only use locally bound variables in pattern synonym declarations by committing rGHCd819e416a2a5: Only use locally bound variables in pattern synonym declarations.
Mar 27 2017, 5:28 AM
mpickering committed rGHCa6ce7f338c88: Remove unused argument from importSuggestions (authored by mpickering).
Remove unused argument from importSuggestions
Mar 27 2017, 5:28 AM
mpickering closed D3376: Remove unused argument from importSuggestions by committing rGHCa6ce7f338c88: Remove unused argument from importSuggestions.
Mar 27 2017, 5:28 AM
mpickering updated the diff for D3377: Only use locally bound variables in pattern synonym declarations.

Rebase

Mar 27 2017, 5:25 AM

Mar 25 2017

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
Mar 25 2017, 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?

Mar 25 2017, 3:52 AM

Mar 24 2017

mpickering retitled D3378: Print module when dumping rules from Print module when dumping rule to Print module when dumping rules.
Mar 24 2017, 5:50 PM
mpickering created D3379: Allow operators as record pattern synonym fields.
Mar 24 2017, 5:47 PM
mpickering created D3378: Print module when dumping rules.
Mar 24 2017, 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.

Mar 24 2017, 9:58 AM

Mar 23 2017

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

Mar 21 2017

mpickering accepted D3361: Show valid substitutions for typed holes.

Good job!

Mar 21 2017, 3:51 PM
mpickering added inline comments to D3361: Show valid substitutions for typed holes.
Mar 21 2017, 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.

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

Mar 20 2017

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)

Mar 20 2017, 12:52 PM
mpickering edited this Badge.
Mar 20 2017, 10:34 AM
mpickering requested changes to D3361: Show valid substitutions for typed holes.
Mar 20 2017, 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?

Mar 20 2017, 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.

Mar 20 2017, 5:25 AM

Mar 19 2017

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).

Mar 19 2017, 4:59 PM

Mar 17 2017

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.

Mar 17 2017, 5:42 AM

Mar 16 2017

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

Mar 14 2017

mpickering accepted D3341: Reimplement minusList using Set.
Mar 14 2017, 5:36 PM
mpickering added inline comments to D3341: Reimplement minusList using Set.
Mar 14 2017, 4:04 PM
mpickering added inline comments to D3341: Reimplement minusList using Set.
Mar 14 2017, 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.

Mar 14 2017, 2:34 PM

Mar 12 2017

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.

Mar 12 2017, 3:22 AM

Mar 11 2017

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.
Mar 11 2017, 3:50 PM
mpickering created D3326: Replace debugging trace with a proper WARN.
Mar 11 2017, 3:40 PM
mpickering created D3322: WIP: Reduce stderr output by passing -v0 to ghc-pkg.
Mar 11 2017, 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
Mar 11 2017, 1:47 PM

Mar 8 2017

mpickering added inline comments to D3299: Fixed QuasiQuotation test and output.
Mar 8 2017, 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.

Mar 8 2017, 1:40 PM

Mar 6 2017

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

Mar 3 2017

mpickering abandoned D3270: Fix completesig04 test properly.

D3266 was merged.

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

Mar 1 2017

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

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

Thanks Reid

Mar 1 2017, 3:13 AM

Feb 26 2017

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!

Feb 26 2017, 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.
Feb 26 2017, 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.

Feb 26 2017, 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.

Feb 26 2017, 5:21 AM

Feb 25 2017

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.

Feb 25 2017, 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.

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

Put the flag in the right place..

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

Redundant imports

Feb 25 2017, 2:21 PM

Feb 24 2017

mpickering updated the diff for D3113: Eliminate ListSetOps from imp_trust_pkgs.
  • Fix build
Feb 24 2017, 4:00 PM
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 edited this Badge.
Feb 17 2017, 3:43 AM