Skip to content

Commit 503fd64

Browse files
barci2Marge Bot
authored andcommitted
This MR is an implementation of the proposal #516.
It adds a warning -Wincomplete-record-selectors for usages of a record field access function (either a record selector or getField @"rec"), while trying to silence the warning whenever it can be sure that a constructor without the record field would not be invoked (which would otherwise cause the program to fail). For example: data T = T1 | T2 {x :: Bool} f a = x a -- this would throw an error g T1 = True g a = x a -- this would not throw an error h :: HasField "x" r Bool => r -> Bool h = getField @"x" j :: T -> Bool j = h -- this would throw an error because of the `HasField` -- constraint being solved See the tests DsIncompleteRecSel* and TcIncompleteRecSel for more examples of the warning. See Note [Detecting incomplete record selectors] in GHC.HsToCore.Expr for implementation details
1 parent fac9e84 commit 503fd64

36 files changed

+579
-39
lines changed

compiler/GHC/Core/ConLike.hs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ import GHC.Utils.Outputable
4747

4848
import Data.Maybe( isJust )
4949
import qualified Data.Data as Data
50+
import qualified Data.List as List
5051

5152
{-
5253
************************************************************************
@@ -224,8 +225,10 @@ conLikeFieldType (RealDataCon dc) label = dataConFieldType dc label
224225

225226

226227
-- | The ConLikes that have *all* the given fields
227-
conLikesWithFields :: [ConLike] -> [FieldLabelString] -> [ConLike]
228-
conLikesWithFields con_likes lbls = filter has_flds con_likes
228+
conLikesWithFields :: [ConLike] -> [FieldLabelString]
229+
-> ( [ConLike] -- ConLikes containing the fields
230+
, [ConLike] ) -- ConLikes not containing the fields
231+
conLikesWithFields con_likes lbls = List.partition has_flds con_likes
229232
where has_flds dc = all (has_fld dc) lbls
230233
has_fld dc lbl = any (\ fl -> flLabel fl == lbl) (conLikeFieldLabels dc)
231234

compiler/GHC/Driver/Flags.hs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -684,6 +684,7 @@ data WarningFlag =
684684
| Opt_WarnMissingRoleAnnotations -- Since 9.8
685685
| Opt_WarnImplicitRhsQuantification -- Since 9.8
686686
| Opt_WarnIncompleteExportWarnings -- Since 9.8
687+
| Opt_WarnIncompleteRecordSelectors -- Since 9.10
687688
deriving (Eq, Ord, Show, Enum)
688689

689690
-- | Return the names of a WarningFlag
@@ -794,6 +795,7 @@ warnFlagNames wflag = case wflag of
794795
Opt_WarnMissingRoleAnnotations -> "missing-role-annotations" :| []
795796
Opt_WarnImplicitRhsQuantification -> "implicit-rhs-quantification" :| []
796797
Opt_WarnIncompleteExportWarnings -> "incomplete-export-warnings" :| []
798+
Opt_WarnIncompleteRecordSelectors -> "incomplete-record-selectors" :| []
797799

798800
-- -----------------------------------------------------------------------------
799801
-- Standard sets of warning options

compiler/GHC/Driver/Session.hs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2263,7 +2263,8 @@ wWarningFlagsDeps = mconcat [
22632263
warnSpec Opt_WarnTermVariableCapture,
22642264
warnSpec Opt_WarnMissingRoleAnnotations,
22652265
warnSpec Opt_WarnImplicitRhsQuantification,
2266-
warnSpec Opt_WarnIncompleteExportWarnings
2266+
warnSpec Opt_WarnIncompleteExportWarnings,
2267+
warnSpec Opt_WarnIncompleteRecordSelectors
22672268
]
22682269

22692270
warningGroupsDeps :: [(Deprecation, FlagSpec WarningGroup)]

compiler/GHC/HsToCore/Errors/Ppr.hs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,10 @@ instance Diagnostic DsMessage where
207207
<+> text "for"<+> quotes (ppr lhs_id)
208208
<+> text "might fire first")
209209
]
210+
DsIncompleteRecordSelector name cons_wo_field not_full_examples -> mkSimpleDecorated $
211+
text "The application of the record field" <+> quotes (ppr name)
212+
<+> text "may fail for the following constructors:"
213+
<+> vcat (map ppr cons_wo_field ++ [text "..." | not_full_examples])
210214

211215
diagnosticReason = \case
212216
DsUnknownMessage m -> diagnosticReason m
@@ -237,6 +241,7 @@ instance Diagnostic DsMessage where
237241
DsRecBindsNotAllowedForUnliftedTys{} -> ErrorWithoutFlag
238242
DsRuleMightInlineFirst{} -> WarningWithFlag Opt_WarnInlineRuleShadowing
239243
DsAnotherRuleMightFireFirst{} -> WarningWithFlag Opt_WarnInlineRuleShadowing
244+
DsIncompleteRecordSelector{} -> WarningWithFlag Opt_WarnIncompleteRecordSelectors
240245

241246
diagnosticHints = \case
242247
DsUnknownMessage m -> diagnosticHints m
@@ -273,6 +278,7 @@ instance Diagnostic DsMessage where
273278
DsRecBindsNotAllowedForUnliftedTys{} -> noHints
274279
DsRuleMightInlineFirst _ lhs_id rule_act -> [SuggestAddInlineOrNoInlinePragma lhs_id rule_act]
275280
DsAnotherRuleMightFireFirst _ bad_rule _ -> [SuggestAddPhaseToCompetingRule bad_rule]
281+
DsIncompleteRecordSelector{} -> noHints
276282

277283
diagnosticCode = constructorCode
278284

compiler/GHC/HsToCore/Errors/Types.hs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import GHC.Prelude
88

99
import GHC.Core (CoreRule, CoreExpr, RuleName)
1010
import GHC.Core.DataCon
11+
import GHC.Core.ConLike
1112
import GHC.Core.Type
1213
import GHC.Driver.DynFlags (DynFlags, xopt)
1314
import GHC.Driver.Flags (WarningFlag)
@@ -147,6 +148,23 @@ data DsMessage
147148
!RuleName -- the \"bad\" rule
148149
!Var
149150

151+
{-| DsIncompleteRecordSelector is a warning triggered when we are not certain whether
152+
a record selector application will be successful. Currently, this means that
153+
the warning is triggered when there is a record selector of a data type that
154+
does not have that field in all its constructors.
155+
156+
Example(s):
157+
data T = T1 | T2 {x :: Bool}
158+
f :: T -> Bool
159+
f a = x a
160+
161+
Test cases:
162+
DsIncompleteRecSel1
163+
DsIncompleteRecSel2
164+
DsIncompleteRecSel3
165+
-}
166+
| DsIncompleteRecordSelector !Name ![ConLike] !Bool
167+
150168
deriving Generic
151169

152170
-- The positional number of the argument for an expression (first, second, third, etc)

compiler/GHC/HsToCore/Expr.hs

Lines changed: 55 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
{-# LANGUAGE ViewPatterns #-}
44

55
{-# OPTIONS_GHC -Wno-incomplete-uni-patterns #-}
6+
{-# LANGUAGE LambdaCase #-}
67

78
{-
89
(c) The University of Glasgow 2006
@@ -31,7 +32,7 @@ import GHC.HsToCore.Monad
3132
import GHC.HsToCore.Pmc
3233
import GHC.HsToCore.Errors.Types
3334
import GHC.Types.SourceText
34-
import GHC.Types.Name
35+
import GHC.Types.Name hiding (varName)
3536
import GHC.Core.FamInstEnv( topNormaliseType )
3637
import GHC.HsToCore.Quote
3738
import GHC.HsToCore.Ticks (stripTicksTopHsExpr)
@@ -51,6 +52,7 @@ import GHC.Core.Make
5152
import GHC.Driver.Session
5253
import GHC.Types.CostCentre
5354
import GHC.Types.Id
55+
import GHC.Types.Id.Info
5456
import GHC.Types.Id.Make
5557
import GHC.Unit.Module
5658
import GHC.Core.ConLike
@@ -67,6 +69,7 @@ import GHC.Utils.Panic
6769
import GHC.Utils.Panic.Plain
6870
import GHC.Core.PatSyn
6971
import Control.Monad
72+
import GHC.Types.Error
7073

7174
{-
7275
************************************************************************
@@ -230,7 +233,38 @@ dsLExpr (L loc e) = putSrcSpanDsA loc $ dsExpr e
230233
-- | Desugar a typechecked expression.
231234
dsExpr :: HsExpr GhcTc -> DsM CoreExpr
232235
dsExpr (HsVar _ (L _ id)) = dsHsVar id
233-
dsExpr (HsRecSel _ (FieldOcc id _)) = dsHsVar id
236+
237+
{- Record selectors are warned about if they are not
238+
present in all of the parent data type's constructor,
239+
or always in case of pattern synonym record selectors
240+
(regulated by a flag). However, this only produces
241+
a warning if it's not a part of a record selector
242+
application. For example:
243+
244+
data T = T1 | T2 {s :: Bool}
245+
f x = s x -- the warning from this case will be supressed
246+
247+
See the `HsApp` case for where it is filtered out
248+
-}
249+
dsExpr (HsRecSel _ (FieldOcc id _))
250+
= do { let name = getName id
251+
RecSelId {sel_cons = (_, cons_wo_field)}
252+
= idDetails id
253+
; cons_trimmed <- trim_cons cons_wo_field
254+
; unless (null cons_wo_field) $ diagnosticDs
255+
$ DsIncompleteRecordSelector name cons_trimmed (cons_trimmed /= cons_wo_field)
256+
-- This only produces a warning if it's not a part of a
257+
-- record selector application (e.g. `s a` where `s` is a selector)
258+
-- See the `HsApp` case for where it is filtered out
259+
; dsHsVar id }
260+
where
261+
trim_cons :: [ConLike] -> DsM [ConLike]
262+
trim_cons cons_wo_field = do
263+
dflags <- getDynFlags
264+
let maxConstructors = maxUncoveredPatterns dflags
265+
return $ take maxConstructors cons_wo_field
266+
267+
234268
dsExpr (HsUnboundVar (HER ref _ _) _) = dsEvTerm =<< readMutVar ref
235269
-- See Note [Holes] in GHC.Tc.Types.Constraint
236270

@@ -297,9 +331,27 @@ dsExpr (HsLamCase _ lc_variant matches)
297331
= uncurry mkCoreLams <$> matchWrapper (LamCaseAlt lc_variant) Nothing matches
298332

299333
dsExpr e@(HsApp _ fun arg)
300-
= do { fun' <- dsLExpr fun
334+
-- We want to have a special case that uses the PMC information to filter
335+
-- out some of the incomplete record selectors warnings and not trigger
336+
-- the warning emitted during the desugaring of dsExpr(HsRecSel)
337+
-- See Note [Detecting incomplete record selectors] in GHC.HsToCore.Pmc
338+
= do { (msgs, fun') <- captureMessagesDs $ dsLExpr fun
339+
-- Make sure to filter out the generic incomplete record selector warning
340+
-- if it's a raw record selector
301341
; arg' <- dsLExpr arg
342+
; case getIdFromTrivialExpr_maybe fun' of
343+
Just fun_id | isRecordSelector fun_id
344+
-> do { let msgs' = filterMessages is_incomplete_rec_sel_msg msgs
345+
; addMessagesDs msgs'
346+
; pmcRecSel fun_id arg' }
347+
_ -> addMessagesDs msgs
302348
; return $ mkCoreAppDs (text "HsApp" <+> ppr e) fun' arg' }
349+
where
350+
is_incomplete_rec_sel_msg :: MsgEnvelope DsMessage -> Bool
351+
is_incomplete_rec_sel_msg (MsgEnvelope {errMsgDiagnostic = DsIncompleteRecordSelector{}})
352+
= False
353+
is_incomplete_rec_sel_msg _ = True
354+
303355

304356
dsExpr e@(HsAppType {}) = dsHsWrapped e
305357

compiler/GHC/HsToCore/Monad.hs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ module GHC.HsToCore.Monad (
4545
-- Warnings and errors
4646
DsWarning, diagnosticDs, errDsCoreExpr,
4747
failWithDs, failDs, discardWarningsDs,
48+
addMessagesDs, captureMessagesDs,
4849

4950
-- Data types
5051
DsMatchContext(..),
@@ -443,6 +444,12 @@ diagnosticDs dsMessage
443444
; let msg = mkMsgEnvelope diag_opts loc (ds_name_ppr_ctx env) dsMessage
444445
; updMutVar (ds_msgs env) (\ msgs -> msg `addMessage` msgs) }
445446

447+
addMessagesDs :: Messages DsMessage -> DsM ()
448+
addMessagesDs msgs1
449+
= do { msg_var <- ds_msgs <$> getGblEnv
450+
; msgs0 <- liftIO $ readIORef msg_var
451+
; liftIO $ writeIORef msg_var (msgs0 `unionMessages` msgs1) }
452+
446453
-- | Issue an error, but return the expression for (), so that we can continue
447454
-- reporting errors.
448455
errDsCoreExpr :: DsMessage -> DsM CoreExpr
@@ -458,6 +465,13 @@ failWithDs msg
458465
failDs :: DsM a
459466
failDs = failM
460467

468+
captureMessagesDs :: DsM a -> DsM (Messages DsMessage, a)
469+
captureMessagesDs thing_inside
470+
= do { msg_var <- liftIO $ newIORef emptyMessages
471+
; res <- updGblEnv (\gbl -> gbl {ds_msgs = msg_var}) thing_inside
472+
; msgs <- liftIO $ readIORef msg_var
473+
; return (msgs, res) }
474+
461475
mkNamePprCtxDs :: DsM NamePprCtx
462476
mkNamePprCtxDs = ds_name_ppr_ctx <$> getGblEnv
463477

compiler/GHC/HsToCore/Pmc.hs

Lines changed: 90 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,12 @@
3535
-- 'ldiMatch'. See Section 4.1 of the paper.
3636
module GHC.HsToCore.Pmc (
3737
-- Checking and printing
38-
pmcPatBind, pmcMatches, pmcGRHSs,
38+
pmcPatBind, pmcMatches, pmcGRHSs, pmcRecSel,
3939
isMatchContextPmChecked, isMatchContextPmChecked_SinglePat,
4040

4141
-- See Note [Long-distance information]
42-
addTyCs, addCoreScrutTmCs, addHsScrutTmCs, getLdiNablas
42+
addTyCs, addCoreScrutTmCs, addHsScrutTmCs, getLdiNablas,
43+
getNFirstUncovered
4344
) where
4445

4546
import GHC.Prelude
@@ -51,29 +52,28 @@ import GHC.HsToCore.Pmc.Desugar
5152
import GHC.HsToCore.Pmc.Check
5253
import GHC.HsToCore.Pmc.Solver
5354
import GHC.Types.Basic (Origin(..))
54-
import GHC.Core (CoreExpr)
55+
import GHC.Core
5556
import GHC.Driver.DynFlags
5657
import GHC.Hs
5758
import GHC.Types.Id
5859
import GHC.Types.SrcLoc
5960
import GHC.Utils.Misc
6061
import GHC.Utils.Outputable
6162
import GHC.Utils.Panic
62-
import GHC.Types.Var (EvVar)
63+
import GHC.Types.Var (EvVar, Var (..))
64+
import GHC.Types.Id.Info
6365
import GHC.Tc.Utils.TcType (evVarPred)
64-
import GHC.Tc.Utils.Monad (updTopFlags)
6566
import {-# SOURCE #-} GHC.HsToCore.Expr (dsLExpr)
6667
import GHC.HsToCore.Monad
6768
import GHC.Data.Bag
68-
import GHC.Data.IOEnv (unsafeInterleaveM)
6969
import GHC.Data.OrdList
70-
import GHC.Utils.Monad (mapMaybeM)
7170

7271
import Control.Monad (when, forM_)
7372
import qualified Data.Semigroup as Semi
7473
import Data.List.NonEmpty ( NonEmpty(..) )
7574
import qualified Data.List.NonEmpty as NE
7675
import Data.Coerce
76+
import GHC.Tc.Utils.Monad
7777

7878
--
7979
-- * Exported entry points to the checker
@@ -193,9 +193,92 @@ pmcMatches ctxt vars matches = {-# SCC "pmcMatches" #-} do
193193
{-# SCC "formatReportWarnings" #-} formatReportWarnings ReportMatchGroup ctxt vars result
194194
return (NE.toList (ldiMatchGroup (cr_ret result)))
195195

196+
{-
197+
Note [Detecting incomplete record selectors]
198+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
199+
A record selector occurence is incomplete iff. it could fail due to
200+
being applied to a data type constructor not present for this record field.
201+
202+
e.g.
203+
data T = T1 | T2 {x :: Int}
204+
d = x someComputation -- `d` may fail
205+
206+
There are 4 parts to detecting and warning about
207+
incomplete record selectors to consider:
208+
209+
- Computing which constructors a general application of a record field will succeed on,
210+
and which ones it will fail on. This is stored in the `sel_cons` field of
211+
`IdDetails` datatype, which is a part of an `Id` and calculated when renaming a
212+
record selector in `mkOneRecordSelector`
213+
214+
- Emitting a warning whenever a `HasField` constraint is solved.
215+
This is checked in `matchHasField` and emitted only for when
216+
the constraint is resolved with an implicit instance rather than a
217+
custom one (since otherwise the warning will be emitted in
218+
the custom implementation anyways)
219+
220+
e.g.
221+
g :: HasField "x" t Int => t -> Int
222+
g = getField @"x"
223+
224+
f :: T -> Int
225+
f = g -- warning will be emitted here
226+
227+
- Emitting a warning for a general occurence of the record selector
228+
This is done during the renaming of a `HsRecSel` expression in `dsExpr`
229+
and simply pulls the information about incompleteness from the `Id`
230+
231+
e.g.
232+
l :: T -> Int
233+
l a = x a -- warning will be emitted here
234+
235+
- Emitting a warning for a record selector `sel` applied to a variable `y`.
236+
In that case we want to use the long-distance information from the
237+
pattern match checker to rule out impossible constructors
238+
(See Note [Long-distance information]). We first add constraints to
239+
the long-distance `Nablas` that `y` cannot be one of the constructors that
240+
contain `sel` (function `checkRecSel` in GHC.HsToCore.Pmc.Check). If the
241+
`Nablas` are still inhabited, we emit a warning with the inhabiting constructors
242+
as examples of where `sel` may fail.
243+
244+
e.g.
245+
z :: T -> Int
246+
z T1 = 0
247+
z a = x a -- warning will not be emitted here since `a` can only be `T2`
248+
-}
249+
250+
pmcRecSel :: Id -- ^ Id of the selector
251+
-> CoreExpr -- ^ Core expression of the argument to the selector
252+
-> DsM ()
253+
pmcRecSel sel_id arg
254+
| RecSelId{ sel_cons = (cons_w_field, _ : _) } <- idDetails sel_id = do
255+
!missing <- getLdiNablas
256+
257+
tracePm "pmcRecSel {" (ppr sel_id)
258+
CheckResult{ cr_ret = PmRecSel{ pr_arg_var = arg_id }, cr_uncov = uncov_nablas }
259+
<- unCA (checkRecSel (PmRecSel () arg cons_w_field)) missing
260+
tracePm "}: " $ ppr uncov_nablas
261+
262+
inhabited <- isInhabited uncov_nablas
263+
when inhabited $ warn_incomplete arg_id uncov_nablas
264+
where
265+
sel_name = varName sel_id
266+
warn_incomplete arg_id uncov_nablas = do
267+
dflags <- getDynFlags
268+
let maxConstructors = maxUncoveredPatterns dflags
269+
unc_examples <- getNFirstUncovered MinimalCover [arg_id] (maxConstructors + 1) uncov_nablas
270+
let cons = [con | unc_example <- unc_examples
271+
, Just (PACA (PmAltConLike con) _ _) <- [lookupSolution unc_example arg_id]]
272+
not_full_examples = length cons == (maxConstructors + 1)
273+
cons' = take maxConstructors cons
274+
diagnosticDs $ DsIncompleteRecordSelector sel_name cons' not_full_examples
275+
276+
pmcRecSel _ _ = return ()
277+
196278
{- Note [pmcPatBind doesn't warn on pattern guards]
197279
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
198280
@pmcPatBind@'s main purpose is to check vanilla pattern bindings, like
281+
>>>>>>> 8760510af3 (This MR is an implementation of the proposal #516.)
199282
@x :: Int; Just x = e@, which is in a @PatBindRhs@ context.
200283
But its caller is also called for individual pattern guards in a @StmtCtxt@.
201284
For example, both pattern guards in @f x y | True <- x, False <- y = ...@ will

0 commit comments

Comments
 (0)