darcs

Patch 1900 restrict the defaults for fromPrim/fromPrims

Title restrict the defaults for fromPrim/fromPrims
Superseder Nosy List ganesh
Related Issues
Status accepted Assigned To
Milestone

Created on 2019-08-30.11:38:14 by ganesh, last changed 2019-09-16.15:49:33 by ganesh.

Files
File name Status Uploaded Type Edit Remove
patch-preview.txt ganesh, 2019-08-30.11:38:13 text/x-darcs-patch
restrict-the-defaults-for-fromprim_fromprims.dpatch ganesh, 2019-08-30.11:38:13 application/x-darcs-patch
unnamed ganesh, 2019-08-30.11:38:13 text/plain
See mailing list archives for discussion on individual patches.
Messages
msg21283 (view) Author: ganesh Date: 2019-08-30.11:38:13
This is an incremental improvement for the
fromPrim/fromPrims default methods, that I think
removes their inherent unsafety. The idea is
simply to add a DefaultSignatures-based constraint
PatchId p ~ ().

Incidentally I was surprised to discover that we
were getting away without type instance PatchId for
both V1 and V2 patches, even though fromPrim
mentions them in its signature.

I'd still like to do more to refactor the FromPrim class
but this makes it less urgent.

1 patch for repository darcs-unstable@darcs.net:screened:

patch 6d2cf4a3e07e4da98221ffc47be502d00d2388b0
Author: Ganesh Sittampalam <ganesh@earth.li>
Date:   Wed Aug 28 12:39:36 BST 2019
  * restrict the defaults for fromPrim/fromPrims
  
  Now they can only be used when the patch type identifier
  is (), so there's no risk of accidentally instantiating
  a real patch identifier type with a dummy value.
Attachments
msg21296 (view) Author: bf Date: 2019-08-31.08:28:32
> This is an incremental improvement for the
> fromPrim/fromPrims default methods, that I think
> removes their inherent unsafety. The idea is
> simply to add a DefaultSignatures-based constraint
> PatchId p ~ ().

Good idea.

> Incidentally I was surprised to discover that we
> were getting away without type instance PatchId for
> both V1 and V2 patches, even though fromPrim
> mentions them in its signature.

Yes, quite strange. I was wondering about that, but didn't investigate.
I hope this is not some sort of defaulting.
msg21297 (view) Author: ganesh Date: 2019-08-31.08:41:52
>> Incidentally I was surprised to discover that we
>> were getting away without type instance PatchId for
>> both V1 and V2 patches, even though fromPrim
>> mentions them in its signature.
> 
> Yes, quite strange. I was wondering about that, but didn't investigate.
> I hope this is not some sort of defaulting.

I think it's because the type checker is happy to leave type family
applications unreduced if it doesn't have a rule for them. For something
like 'PatchId a' this makes sense as it doesn't know what 'a' is yet.
For things like 'PatchId (RepoPatchV1 prim)' it's a bit weird and often
it leaks into error messages that say "Can't unify PatchId (RepoPatchV1
prim) with Int" when really your mistake was just to not define the type
instance. In this case the argument is being ignored so probably it's
just getting unified with a free type variable which always succeeds.
msg21417 (view) Author: bf Date: 2019-09-14.20:01:22
It's a temporary measure as we probably want to remove these defaults
at some point, but okay for now.
History
Date User Action Args
2019-08-30 11:38:14ganeshcreate
2019-08-30 11:41:34ganeshsetstatus: needs-screening -> needs-review
2019-08-31 08:28:32bfsetmessages: + msg21296
2019-08-31 08:41:53ganeshsetmessages: + msg21297
2019-09-14 20:01:22bfsetstatus: needs-review -> accepted
messages: + msg21417
2019-09-14 20:04:02bfsetstatus: accepted -> accepted-pending-tests
2019-09-16 15:49:33ganeshsetstatus: accepted-pending-tests -> accepted