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 (closed), 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 (closed).
• -Wincomplete-record-uses: warns about HasField constraints being solved for partial fields. These might arise from calls to getField/setField, or from RecordDotSyntax.

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 to setField 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 a HasField 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.

1. If -Wincomplete-record-selectors is enabled, any occurrence of x as a selector (in an expression) causes a warning. It is irrelevant whether or not it is applied. Thus f1 r = x r and g1 = x both warn, but h1 r = y r does not.

2. If -Wincomplete-record-updates is enabled and RecordDotSyntax is disabled, any occurrence of x in a record update expression causes a warning. Thus f2 r = r { x = 3 } warns, but h2 r = r { y = True } does not. See point 3 for what happens if RecordDotSyntax is enabled.

3. If -Wincomplete-record-uses is enabled, a warning is emitted whenever a constraint like HasField "x" T Int is solved automatically.

   • In particular this arises with getField @"x" @T and setField @"x" @T, but also with RecordDotSyntax in cases such as f3 (r :: T) = r.x or g3 (r :: T) = r { x = 3 }.
   • On the other hand h3 r = getField @"x" r and (with RecordDotSyntax) k3 r = r.x and f2 r = r { x = 3 } do not warn because their types are polymorphic in the record type, subject to a HasField constraint. (Note that f2 did warn under point 2 when RecordDotSyntax was disabled, but it meant something different, because it was not polymorphic in the record type.)
   • A later call to h3 at type T (such as l3 r = h3 (r :: T)) does trigger a warning, because this leads to the constraint HasField "x" T Int being solved.

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