Warning for partial solutions to HasField constraints
Motivation
While discussing the planned changes to HasField
(#16232) in the context of a client project, a thought occurred to me: the current treatment of partial record fields in getField
(and the planned treatment in hasField
/setField
) is to throw a runtime exception, just as for traditional record selectors and updates, which is consistent but somewhat undesirable. (Alternatives are conceivable, but none made it through the proposals process.)
There really ought to be an (optional) warning at use sites of HasField
that involve partial fields, just as we have -Wincomplete-record-updates
. Seemingly we don't have a corresponding warning for selector use sites, per #7169 (closed)/#17100, but we really should.
The main use case for these warnings is for cases where -Wpartial-fields
is not quite right, because one wants to define partial fields and yet avoid using them in ways that might fail at runtime (i.e. pattern-matching instead of select/update). (See "Why do we need new warnings?" below for further justification.)
Proposal
I suggest defining two new flags (modulo name bikeshedding):
-
-Wincomplete-record-selectors
: warns about uses of traditional record selectors that are partial, just as-Wincomplete-record-updates
does for updates. This is #17100. -
-Wincomplete-record-uses
: warns aboutHasField
constraints being solved for partial fields. These might arise from calls togetField
/setField
, or fromRecordDotSyntax
.
Moreover I propose refining the specification of the existing -Wincomplete-record-updates
flag (as the interaction with RecordDotSyntax
is not yet specified AFAIK):
- When
RecordDotSyntax
is disabled, any occurrence of a partial field in a record update expression causes a warning, as at present. - When
RecordDotSyntax
is enabled, the flag has no effect, because record updates are desugared into calls tosetField
rather than being resolved to a concrete choice of datatype with possibly partial fields. Instead, the new-Wincomplete-record-uses
warning can be used to identify when aHasField
constraint arising from an update is solved for a partial field.
The new warnings would not (for now) be implied by -Wall
, just as -Wincomplete-record-updates
and -Wpartial-fields
are not.
Examples
Here are some examples, using
data T = T1 { x :: Int, y :: Bool }
| T2 { y :: Bool }
so that x
is a partial field and y
is a total field.
-
If
-Wincomplete-record-selectors
is enabled, any occurrence ofx
as a selector (in an expression) causes a warning. It is irrelevant whether or not it is applied. Thusf1 r = x r
andg1 = x
both warn, buth1 r = y r
does not. -
If
-Wincomplete-record-updates
is enabled andRecordDotSyntax
is disabled, any occurrence ofx
in a record update expression causes a warning. Thusf2 r = r { x = 3 }
warns, buth2 r = r { y = True }
does not. See point 3 for what happens ifRecordDotSyntax
is enabled. -
If
-Wincomplete-record-uses
is enabled, a warning is emitted whenever a constraint likeHasField "x" T Int
is solved automatically.- In particular this arises with
getField @"x" @T
andsetField @"x" @T
, but also withRecordDotSyntax
in cases such asf3 (r :: T) = r.x
org3 (r :: T) = r { x = 3 }
. - On the other hand
h3 r = getField @"x" r
and (withRecordDotSyntax
)k3 r = r.x
andf2 r = r { x = 3 }
do not warn because their types are polymorphic in the record type, subject to aHasField
constraint. (Note thatf2
did warn under point 2 whenRecordDotSyntax
was disabled, but it meant something different, because it was not polymorphic in the record type.) - A later call to
h3
at typeT
(such asl3 r = h3 (r :: T)
) does trigger a warning, because this leads to the constraintHasField "x" T Int
being solved.
- In particular this arises with
-
Uses of the field
x
in record construction or pattern-matching do not lead to a warning, so these are fine:h4 = T1 { x = 3, y = True } k4 T1{x=x'} = x' k4 T2{} = 0
Why do we need new warnings?
These warnings are intended to support a style of Haskell programming where uses of any partial functions are discouraged/minimized. This is a common (if not universal) approach. It is relatively easy to avoid uses of common partial functions such as head
, because (a) programmers preferring this style learn which Prelude
functions should be avoided anyway, and (b) explicit imports and/or a custom Prelude
can render them out of scope or mark them with WARNING
pragmas.
However record fields are different: selector functions are generated by the compiler, regardless of imports, and some of these functions are partial. A codebase typically contains many domain-specific record datatype definitions and hence many fields, of which some selectors are partial but many are not. It thus becomes difficult to keep track of which fields are potentially partial, and indeed as code changes over time, previously total fields may become partial.
Moreover, while it is relatively easy for tooling to check for uses of well-known functions such as head
, it becomes slightly harder to identify partial selector functions (since it requires analysis of many datatype definitions), and is yet harder still when HasField
gets involved (since r.x
may or may not refer to a partial selector x
depending on the types). Having GHC do it would be easy.
The -Wpartial-fields
warning gives one solution to this problem: rule out the definition of T
, and require all fields to be total. However this is unnecessarily restrictive, because it rules out ever giving field names to heterogeneous multi-constructor datatypes, even though these are sometimes useful, and it is perfectly possible to use them without any partiality (by construction/pattern-matching, avoiding selection/update).
I certainly wouldn't object to more control of warnings on partial functions such as head
, but I believe records are worth treating specially.
Status
Do new warning flags like this require a ghc-proposal? I hope not...
EDIT: originally the proposal had -Wincomplete-record-uses
imply -Wincomplete-record-selectors
and -Wincomplete-record-updates
, but this was removed following discussion.