Sasha Bogicevic pushed to branch wip/21101 at Glasgow Haskell Compiler / GHC Commits: 6f1d6da8 by Sasha Bogicevic at 2026-07-15T18:24:05+02:00 Record wildcard hints: show them in more contexts, and include arity (#21101) Address review feedback on !8673: * Emit SuggestExplicitConstructorArguments for record patterns as well as record construction, so `f (C {..}) = ...` also suggests rewriting to positional form. SuggestEmptyRecordBraces remains pattern-only, since `C {}` as an expression would construct a value with all fields unset. * Store the constructor's visible arity in ConHasPositionalArgs so the hint can say how many arguments to write, e.g. Apply ‘D’ to its two arguments instead The arity is threaded from the parsed declaration through LConsWithFields: the per-constructor payload changes from Maybe [Located Int] to Either VisArity [Located Int], where Left n means the constructor has n unlabelled arguments. Updates test baselines: T21101, T9815, T9815b, T9815ghci, T9815bghci. - - - - - 14 changed files: - compiler/GHC/Hs/Utils.hs - compiler/GHC/Rename/Env.hs - compiler/GHC/Rename/Names.hs - compiler/GHC/Rename/Pat.hs - compiler/GHC/Tc/Errors/Ppr.hs - compiler/GHC/Tc/Errors/Types.hs - compiler/GHC/Types/GREInfo.hs - compiler/GHC/Types/Hint.hs - compiler/GHC/Types/Hint/Ppr.hs - testsuite/tests/rename/should_fail/T21101.stderr - testsuite/tests/rename/should_fail/T9815.stderr - testsuite/tests/rename/should_fail/T9815b.stderr - testsuite/tests/rename/should_fail/T9815bghci.stderr - testsuite/tests/rename/should_fail/T9815ghci.stderr Changes: ===================================== compiler/GHC/Hs/Utils.hs ===================================== @@ -1589,8 +1589,8 @@ hsConDeclsBinders in the following format: with its record fields, in the form of a list of Int indices into... - IntMap FieldOcc, an IntMap of record fields. -(In actual fact, we use [(ConRdrName, Maybe [Located Int])], with Nothing indicating -that the constructor has unlabelled fields: see Note [Local constructor info in the renamer] +(In actual fact, we use [(ConRdrName, Either VisArity [Located Int])], with Left n indicating +that the constructor has n unlabelled arguments: see Note [Local constructor info in the renamer] in GHC.Types.GREInfo.) This allows us to do the following (see GHC.Rename.Names.getLocalNonValBinders.new_tc): @@ -1611,7 +1611,7 @@ Other relevant test cases: rnfail015. -- See Note [Collecting record fields in data declarations]. data LConsWithFields p = LConsWithFields - { consWithFieldIndices :: [(LocatedA (IdP (GhcPass p)), Maybe [Located Int])] + { consWithFieldIndices :: [(LocatedA (IdP (GhcPass p)), Either VisArity [Located Int])] , consFields :: IntMap (LFieldOcc (GhcPass p)) } @@ -1651,16 +1651,15 @@ hsConDeclsBinders cons = go emptyFieldIndices cons LConsWithFields ns fs = go seen' rs get_flds_h98 :: FieldIndices p -> HsConDeclH98Details (GhcPass p) - -> (Maybe [Located Int], FieldIndices p) - get_flds_h98 seen (RecCon _ flds) = first Just $ get_flds seen flds - get_flds_h98 seen (PrefixCon _ []) = (Just [], seen) - get_flds_h98 seen _ = (Nothing, seen) + -> (Either VisArity [Located Int], FieldIndices p) + get_flds_h98 seen (RecCon _ flds) = first Right $ get_flds seen flds + get_flds_h98 seen (PrefixCon _ args) = (Left (length args), seen) + get_flds_h98 seen (InfixCon {}) = (Left 2, seen) get_flds_gadt :: FieldIndices p -> HsConDeclGADTDetails (GhcPass p) - -> (Maybe [Located Int], FieldIndices p) - get_flds_gadt seen (RecConGADT _ flds) = first Just $ get_flds seen flds - get_flds_gadt seen (PrefixConGADT _ []) = (Just [], seen) - get_flds_gadt seen _ = (Nothing, seen) + -> (Either VisArity [Located Int], FieldIndices p) + get_flds_gadt seen (RecConGADT _ flds) = first Right $ get_flds seen flds + get_flds_gadt seen (PrefixConGADT _ args) = (Left (length args), seen) get_flds :: FieldIndices p -> LocatedA [LHsConDeclRecField (GhcPass p)] -> ([Located Int], FieldIndices p) ===================================== compiler/GHC/Rename/Env.hs ===================================== @@ -423,7 +423,10 @@ lookupConstructorInfo qcon@(WithUserRdr _ con_name) = do { info <- lookupGREInfo_GRE con_name ; case info of IAmConLike con_info -> return con_info - UnboundGRE -> return $ ConInfo (ConIsData []) ConHasPositionalArgs + UnboundGRE -> return $ ConInfo (ConIsData []) (ConHasPositionalArgs 0) + -- The arity is a dummy: an unbound constructor never reaches the + -- code that consults it (see the isUnboundName guard in + -- GHC.Rename.Pat.rn_dotdot). IAmTyCon {} -> failIllegalTyCon WL_ConLike qcon _ -> pprPanic "lookupConstructorInfo: not a ConLike" $ vcat [ text "name:" <+> ppr con_name ] ===================================== compiler/GHC/Rename/Names.hs ===================================== @@ -71,7 +71,7 @@ import GHC.Types.FieldLabel import GHC.Types.Hint import GHC.Types.SourceFile import GHC.Types.SrcLoc as SrcLoc -import GHC.Types.Basic ( TyConFlavour (..), convImportLevel ) +import GHC.Types.Basic (TyConFlavour (..), convImportLevel, VisArity) import GHC.Types.Id import GHC.Types.PkgQual import GHC.Types.GREInfo (ConInfo(..), ConFieldInfo (..), ConLikeInfo (ConIsData)) @@ -875,15 +875,16 @@ getLocalNonValBinders fixity_env -- -- The information we needed was all set up for us: -- see Note [Collecting record fields in data declarations] in GHC.Hs.Utils. - mk_fld_env :: [(Name, Maybe [Located Int])] -> IntMap FieldLabel + mk_fld_env :: [(Name, Either VisArity [Located Int])] -> IntMap FieldLabel -> [(ConLikeName, ConInfo)] mk_fld_env names flds = [ (DataConName con, ConInfo (ConIsData (map fst names)) fld_info) - | (con, mb_fl_indxs) <- names - , let fld_info = case fmap (map ((flds IntMap.!) . unLoc)) mb_fl_indxs of - Nothing -> ConHasPositionalArgs - Just [] -> ConIsNullary - Just (fld:flds) -> ConHasRecordFields $ fld NE.:| flds ] + | (con, con_fl_indxs) <- names + , let fld_info = case fmap (map ((flds IntMap.!) . unLoc)) con_fl_indxs of + Left 0 -> ConIsNullary + Left arity -> ConHasPositionalArgs arity + Right [] -> ConIsNullary + Right (fld:flds) -> ConHasRecordFields $ fld NE.:| flds ] new_assoc :: DuplicateRecordFields -> FieldSelectors -> LInstDecl GhcPs -> RnM [GlobalRdrElt] @@ -939,10 +940,10 @@ getLocalNonValBinders fixity_env -- Add errors if a constructor has a duplicate record field. add_dup_fld_errs :: IntMap FieldLabel - -> (Name, Maybe [Located Int]) + -> (Name, Either VisArity [Located Int]) -> IOEnv (Env TcGblEnv TcLclEnv) () - add_dup_fld_errs all_flds (con, mb_con_flds) - | Just con_flds <- mb_con_flds + add_dup_fld_errs all_flds (con, con_flds_or_arity) + | Right con_flds <- con_flds_or_arity , let (_, dups) = removeDups (comparing unLoc) con_flds = for_ dups $ \ dup_flds -> -- Report the error at the location of the second occurrence ===================================== compiler/GHC/Rename/Pat.hs ===================================== @@ -874,7 +874,11 @@ rnHsRecFields ctxt mk_arg (HsRecFields { rec_flds = flds, rec_dotdot = dotdot }) ; checkErr dd_flag (needFlagDotDot ctxt) ; (rdr_env, lcl_env) <- getRdrEnvs ; conInfo <- lookupConstructorInfo qcon - ; when (conFieldInfo conInfo == ConHasPositionalArgs) (addErr (TcRnIllegalWildcardsInConstructor (toRecordFieldPart ctxt) con)) + + ; case conFieldInfo conInfo of + ConHasPositionalArgs nbArgs -> + addErr $ TcRnIllegalWildcardsInConstructor (toRecordFieldPart ctxt) con nbArgs + _ -> return () ; let present_flds = mkOccSet $ map rdrNameOcc (getFieldRdrs flds) -- For constructor uses (but not patterns) ===================================== compiler/GHC/Tc/Errors/Ppr.hs ===================================== @@ -357,7 +357,7 @@ instance Diagnostic TcRnMessage where -> mkSimpleDecorated $ vcat [text "Illegal view pattern: " <+> ppr pat] TcRnCharLiteralOutOfRange c -> mkSimpleDecorated $ text "character literal out of range: '\\" <> char c <> char '\'' - TcRnIllegalWildcardsInConstructor ctx con + TcRnIllegalWildcardsInConstructor ctx con _ -> mkSimpleDecorated $ text "The data constructor" <+> quotes (ppr con) <+> text "does not have named record fields, so the record" @@ -2791,10 +2791,12 @@ instance Diagnostic TcRnMessage where -> [suggestExtension LangExt.ViewPatterns] TcRnCharLiteralOutOfRange{} -> noHints - TcRnIllegalWildcardsInConstructor ctx con + TcRnIllegalWildcardsInConstructor ctx con arity -> case ctx of - RecordFieldPattern{} -> [SuggestEmptyRecordBraces con] - _ -> [SuggestExplicitConstructorArguments con] + RecordFieldPattern{} -> [ SuggestEmptyRecordBraces con + , SuggestExplicitConstructorArguments con arity + ] + _ -> [SuggestExplicitConstructorArguments con arity] TcRnIgnoringAnnotations{} -> noHints TcRnAnnotationInSafeHaskell ===================================== compiler/GHC/Tc/Errors/Types.hs ===================================== @@ -824,6 +824,8 @@ data TcRnMessage where worded accordingly. Constructors with no fields at all do not trigger this error: since GHC proposal 496 ("Nullary record wildcards"), @C {..}@ is legal for nullary constructors. + The 'VisArity' field records the constructor's number of positional arguments + which the suggested fix mentions. Example(s): @@ -842,7 +844,7 @@ data TcRnMessage where rename/should_fail/T9815bghci.hs rename/should_fail/T21101.hs -} - TcRnIllegalWildcardsInConstructor :: !RecordFieldPart -> !Name -> TcRnMessage + TcRnIllegalWildcardsInConstructor :: !RecordFieldPart -> !Name -> !VisArity -> TcRnMessage {-| TcRnIgnoringAnnotations is a warning that occurs when the source code contains annotation pragmas but the platform in use does not support an ===================================== compiler/GHC/Types/GREInfo.hs ===================================== @@ -244,14 +244,14 @@ instance NFData ConLikeInfo where -- See Note [Local constructor info in the renamer] data ConFieldInfo = ConHasRecordFields (NonEmpty FieldLabel) - | ConHasPositionalArgs + | ConHasPositionalArgs !VisArity | ConIsNullary deriving stock Eq deriving Data instance NFData ConFieldInfo where rnf ConIsNullary = () - rnf ConHasPositionalArgs = () + rnf (ConHasPositionalArgs arity) = rnf arity rnf (ConHasRecordFields flds) = rnf flds mkConInfo :: ConLikeInfo -> VisArity -> [FieldLabel] -> ConInfo @@ -259,9 +259,9 @@ mkConInfo con_ty n flds = ConInfo { conLikeInfo = con_ty , conFieldInfo = mkConFieldInfo n flds } -mkConFieldInfo :: Arity -> [FieldLabel] -> ConFieldInfo +mkConFieldInfo :: VisArity -> [FieldLabel] -> ConFieldInfo mkConFieldInfo 0 _ = ConIsNullary -mkConFieldInfo _ fields = maybe ConHasPositionalArgs ConHasRecordFields +mkConFieldInfo arity fields = maybe (ConHasPositionalArgs arity) ConHasRecordFields $ NonEmpty.nonEmpty fields conInfoFields :: ConInfo -> [FieldLabel] @@ -269,7 +269,7 @@ conInfoFields = conFieldInfoFields . conFieldInfo conFieldInfoFields :: ConFieldInfo -> [FieldLabel] conFieldInfoFields (ConHasRecordFields fields) = NonEmpty.toList fields -conFieldInfoFields ConHasPositionalArgs = [] +conFieldInfoFields (ConHasPositionalArgs _) = [] conFieldInfoFields ConIsNullary = [] instance Outputable ConInfo where @@ -284,7 +284,7 @@ instance Outputable ConLikeInfo where instance Outputable ConFieldInfo where ppr ConIsNullary = text "ConIsNullary" - ppr ConHasPositionalArgs = text "ConHasPositionalArgs" + ppr (ConHasPositionalArgs arity) = text "ConHasPositionalArgs" <+> braces (ppr arity) ppr (ConHasRecordFields fieldLabels) = text "ConHasRecordFields" <+> braces (ppr fieldLabels) ===================================== compiler/GHC/Types/Hint.hs ===================================== @@ -45,7 +45,7 @@ import GHC.Types.InlinePragma (ActivationGhc) import GHC.Types.Name (Name, NameSpace, OccName (occNameFS), isSymOcc, nameOccName) import GHC.Types.Name.Reader (RdrName (Unqual), ImpDeclSpec, GlobalRdrElt) import GHC.Types.SrcLoc (SrcSpan) -import GHC.Types.Basic (RuleName) +import GHC.Types.Basic (RuleName, VisArity) import GHC.Parser.Errors.Basic import GHC.Utils.Outputable import GHC.Data.FastString (fsLit) @@ -560,9 +560,10 @@ data GhcHint of record syntax, for constructors without labelled fields. Triggered by 'GHC.Tc.Errors.Types.TcRnIllegalWildcardsInConstructor' - in a record construction. + in a record construction and record patterns. + The 'VisArity' is the number of positional arguments of the constructor. -} - | SuggestExplicitConstructorArguments !Name + | SuggestExplicitConstructorArguments !Name !VisArity -- | What the user should upgrade to resolve an @-jsem@ semaphore -- protocol version mismatch. ===================================== compiler/GHC/Types/Hint/Ppr.hs ===================================== @@ -348,8 +348,9 @@ instance Outputable GhcHint where SuggestEmptyRecordBraces con -> text "Use" <+> quotes (ppr con <> text "{}") <+> text "instead," <+> text "which matches" <+> quotes (ppr con) <+> text "regardless of its fields" - SuggestExplicitConstructorArguments con - -> text "Apply" <+> quotes (ppr con) <+> text "to its arguments instead" + SuggestExplicitConstructorArguments con nbArgs + -> text "Apply" <+> quotes (ppr con) <+> text "to its" + <+> speakNOf nbArgs (text "argument") <+> text "instead" perhapsAsPat :: SDoc perhapsAsPat = text "Perhaps you meant an as-pattern, which must not be surrounded by whitespace" ===================================== testsuite/tests/rename/should_fail/T21101.stderr ===================================== @@ -1,5 +1,6 @@ T21101.hs:7:3: error: [GHC-47217] The data constructor ‘D’ does not have named record fields, so the record pattern ‘D{..}’ is invalid. - Suggested fix: - Use ‘D{}’ instead, which matches ‘D’ regardless of its fields + Suggested fixes: + • Use ‘D{}’ instead, which matches ‘D’ regardless of its fields + • Apply ‘D’ to its two arguments instead ===================================== testsuite/tests/rename/should_fail/T9815.stderr ===================================== @@ -1,4 +1,4 @@ T9815.hs:6:13: error: [GHC-47217] The data constructor ‘N’ does not have named record fields, so the record construction ‘N{..}’ is invalid. - Suggested fix: Apply ‘N’ to its arguments instead + Suggested fix: Apply ‘N’ to its one argument instead ===================================== testsuite/tests/rename/should_fail/T9815b.stderr ===================================== @@ -1,4 +1,4 @@ T9815.hs:6:13: error: [GHC-47217] The data constructor ‘N’ does not have named record fields, so the record construction ‘N{..}’ is invalid. - Suggested fix: Apply ‘N’ to its arguments instead + Suggested fix: Apply ‘N’ to its one argument instead ===================================== testsuite/tests/rename/should_fail/T9815bghci.stderr ===================================== @@ -1,4 +1,4 @@ <interactive>:5:7: error: [GHC-47217] The data constructor ‘Arg’ does not have named record fields, so the record construction ‘Arg{..}’ is invalid. - Suggested fix: Apply ‘Arg’ to its arguments instead + Suggested fix: Apply ‘Arg’ to its two arguments instead ===================================== testsuite/tests/rename/should_fail/T9815ghci.stderr ===================================== @@ -1,4 +1,5 @@ <interactive>:3:7: error: [GHC-47217] The data constructor ‘Data.Semigroup.Arg’ does not have named record fields, so the record construction ‘Data.Semigroup.Arg{..}’ is invalid. - Suggested fix: Apply ‘Data.Semigroup.Arg’ to its arguments instead + Suggested fix: + Apply ‘Data.Semigroup.Arg’ to its two arguments instead View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/6f1d6da89bde72a30c975f84383f908c... -- View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/6f1d6da89bde72a30c975f84383f908c... You're receiving this email because of your account on gitlab.haskell.org. Manage all notifications: https://gitlab.haskell.org/-/profile/notifications | Help: https://gitlab.haskell.org/help