I'm aware of substTys being currently unchecked, I didn't want to introduce substTysUnchecked just yet, as there already are more than 30 occurrences of substTyUnchecked. But maybe I should push uncheckedness as far up as possible, do you think it's worth to do it now?
Well, it’s just as much a source of potential bugs as substTy. So yes, I’d make all the calls to substTysUnchecked, and then work through getting rid of them. Many of them will be simple.
Do you want me to do the renaming?
yes please
Pls yell specifically when you need help; don’t rely on me seeing Phab patches
S
From: Bartosz Nitka [mailto:niteria@gmail.com]
Sent: 26 January 2016 11:15
To: Simon Peyton Jones
Cc: ghc-devs@haskell.org
Subject: Re: Substitutions
Hello Simon,
I was about to send an improved version of D1801, I will do that once I rebase past your patch (there's a small overlap with what I've done). D1820 is ready for review.
I have a patch coming for piResultTys [1] where the FVs of ty are not in the in_scope set. I've also looked at a similar problem in lint_app [2], but the fix I have is a bit awkward, so I will probably need some input from you. Both of them are blocking #11360 and I want to get it unblocked as soon as possible to continue determinism work.
I'm aware of substTys being currently unchecked, I didn't want to introduce substTysUnchecked just yet, as there already are more than 30 occurrences of substTyUnchecked. But maybe I should push uncheckedness as far up as possible, do you think it's worth to do it now?
Do you want me to do the renaming?
Cheers,
Bartosz
[1] https://phabricator.haskell.org/diffusion/GHC/browse/master/compiler/types/T...
[2] https://phabricator.haskell.org/diffusion/GHC/browse/master/compiler/coreSyn...
2016-01-26 10:55 GMT+00:00 Simon Peyton Jones mailto:simonpj@microsoft.com>:
Bartosz
How are you getting on with tidying up substitutions.
I’ve done a bit in that direction myself:
commit 1c6d70c2121fd1126fcc2458bdbcc856e19598c2
Author: Simon Peyton Jones mailto:simonpj@microsoft.com>
Date: Tue Jan 26 09:37:06 2016 +0000
Kill off zipTopTCvSubst in favour of zipOpenTCvSubst
But there is more to do.
In particular, I note that substTys is not checked – but neither is it called substTysUnchecked. It should be! That will show up a bunch more cases, I think.
Another particular thing, arising from my patch, is that we can now do some renaming
• zipOpenTCvSubst -> zipTvSubst (It only deals with tyvars)
• zipOpenTCvSubstCoVars -> zipCvSubst (it only deals with covars)
• zipOpenTCvSubstBinders -> zipTyBinderSubst (it only deals with TyBinders, not covars)
I guess you’ll want to do similar things for the “mk” variants.
Anyway: good for you! Let me know if you need help.
Simon
