[Git][ghc/ghc][wip/warn-badly-staged-types] 2 commits: ci: Remove manually triggered test-ci job

Matthew Pickering (@mpickering) gitlab at gitlab.haskell.org
Tue Aug 22 07:58:16 UTC 2023



Matthew Pickering pushed to branch wip/warn-badly-staged-types at Glasgow Haskell Compiler / GHC


Commits:
7be4a272 by Matthew Pickering at 2023-08-22T08:55:20+01:00
ci: Remove manually triggered test-ci job

This doesn't work on slimmed down pipelines as the needed jobs don't
exist.

If you want to run test-primops then apply the label.

- - - - -
1b78388c by Oleg Grenrus at 2023-08-22T07:58:09+00:00
Add warning for badly staged types.

Resolves #23829.

The stage violation results in out-of-bound names in splices.
Technically this is an error, but someone might rely on this!?

Internal changes:
- we now track stages for TyVars.
- thLevel (RunSplice _) = 0, instead of panic, as reifyInstances does
  in fact rename its argument type, and it can contain variables.
- #20969 tests fails earlier, as the type annotation is in badly staged.
  Things like

    foo :: forall a. (Lift a, Num a) => a
    foo = $$(liftTyped 0 :: Code Q a)

  cannot work, as e.g. `Num` dictionary is true run time information.
  Therefore

    foo :: forall a. Num a => a
    foo = $$(liftTyped _ :: Code Q a)

  could report a type, but as stages are in fact checked before
  type-checking, it's impossible now. (For typed-TH they could be
  checked during type-checking, as is done for terms).

- - - - -


19 changed files:

- .gitlab-ci.yml
- compiler/GHC/Driver/Flags.hs
- compiler/GHC/Rename/HsType.hs
- compiler/GHC/Rename/Splice.hs
- compiler/GHC/Rename/Splice.hs-boot
- compiler/GHC/Tc/Errors/Ppr.hs
- compiler/GHC/Tc/Errors/Types.hs
- compiler/GHC/Tc/Types/TH.hs
- compiler/GHC/Tc/Utils/Env.hs
- compiler/GHC/Types/Error/Codes.hs
- docs/users_guide/using-warnings.rst
- + testsuite/tests/th/T23829_hasty.hs
- + testsuite/tests/th/T23829_hasty.stderr
- + testsuite/tests/th/T23829_tardy.ghc.stderr
- + testsuite/tests/th/T23829_tardy.hs
- + testsuite/tests/th/T23829_tardy.stdout
- + testsuite/tests/th/T23829_timely.hs
- testsuite/tests/th/all.T
- testsuite/tests/typecheck/no_skolem_info/T20969.stderr


Changes:

=====================================
.gitlab-ci.yml
=====================================
@@ -848,10 +848,6 @@ release-hackage-lint:
       artifacts: false
   extends: .test-primops
 
-test-primops-validate:
-  extends: .test-primops-validate-template
-  when: manual
-
 test-primops-label:
   extends: .test-primops-validate-template
   rules:


=====================================
compiler/GHC/Driver/Flags.hs
=====================================
@@ -693,6 +693,7 @@ data WarningFlag =
    | Opt_WarnImplicitRhsQuantification               -- Since 9.8
    | Opt_WarnIncompleteExportWarnings                -- Since 9.8
    | Opt_WarnIncompleteRecordSelectors                  -- Since 9.10
+   | Opt_WarnBadlyStagedTypes                        -- Since 9.10
    deriving (Eq, Ord, Show, Enum)
 
 -- | Return the names of a WarningFlag
@@ -804,6 +805,7 @@ warnFlagNames wflag = case wflag of
   Opt_WarnImplicitRhsQuantification               -> "implicit-rhs-quantification" :| []
   Opt_WarnIncompleteExportWarnings                -> "incomplete-export-warnings" :| []
   Opt_WarnIncompleteRecordSelectors                  -> "incomplete-record-selectors" :| []
+  Opt_WarnBadlyStagedTypes                        -> "badly-staged-types" :| []
 
 -- -----------------------------------------------------------------------------
 -- Standard sets of warning options
@@ -942,6 +944,7 @@ standardWarnings -- see Note [Documenting warning flags]
         Opt_WarnUnicodeBidirectionalFormatCharacters,
         Opt_WarnGADTMonoLocalBinds,
         Opt_WarnLoopySuperclassSolve,
+        Opt_WarnBadlyStagedTypes,
         Opt_WarnTypeEqualityRequiresOperators
       ]
 


=====================================
compiler/GHC/Rename/HsType.hs
=====================================
@@ -44,7 +44,7 @@ module GHC.Rename.HsType (
 
 import GHC.Prelude
 
-import {-# SOURCE #-} GHC.Rename.Splice( rnSpliceType )
+import {-# SOURCE #-} GHC.Rename.Splice( rnSpliceType, checkThLocalTyName )
 
 import GHC.Core.TyCo.FVs ( tyCoVarsOfTypeList )
 import GHC.Hs
@@ -59,6 +59,7 @@ import GHC.Rename.Unbound ( notInScopeErr, WhereLooking(WL_LocalOnly) )
 import GHC.Tc.Errors.Types
 import GHC.Tc.Errors.Ppr ( pprHsDocContext )
 import GHC.Tc.Utils.Monad
+import GHC.Unit.Module ( getModule )
 import GHC.Types.Name.Reader
 import GHC.Builtin.Names
 import GHC.Types.Hint ( UntickedPromotedThing(..) )
@@ -535,6 +536,9 @@ rnHsTyKi env (HsTyVar _ ip (L loc rdr_name))
            -- Any type variable at the kind level is illegal without the use
            -- of PolyKinds (see #14710)
        ; name <- rnTyVar env rdr_name
+       ; this_mod <- getModule
+       ; when (nameIsLocalOrFrom this_mod name) $
+         checkThLocalTyName name
        ; when (isDataConName name && not (isPromoted ip)) $
          -- NB: a prefix symbolic operator such as (:) is represented as HsTyVar.
             addDiagnostic (TcRnUntickedPromotedThing $ UntickedConstructor Prefix name)


=====================================
compiler/GHC/Rename/Splice.hs
=====================================
@@ -1,5 +1,6 @@
 {-# LANGUAGE FlexibleContexts #-}
 {-# LANGUAGE TypeFamilies #-}
+{-# LANGUAGE MultiWayIf #-}
 
 module GHC.Rename.Splice (
         rnTopSpliceDecls,
@@ -12,7 +13,8 @@ module GHC.Rename.Splice (
         -- Brackets
         rnTypedBracket, rnUntypedBracket,
 
-        checkThLocalName, traceSplice, SpliceInfo(..)
+        checkThLocalName, traceSplice, SpliceInfo(..),
+        checkThLocalTyName,
   ) where
 
 import GHC.Prelude
@@ -903,6 +905,24 @@ traceSplice (SpliceInfo { spliceDescription = sd, spliceSource = mb_src
       = vcat [ text "--" <+> ppr loc <> colon <+> text "Splicing" <+> text sd
              , gen ]
 
+checkThLocalTyName :: Name -> RnM ()
+checkThLocalTyName name
+  | isUnboundName name   -- Do not report two errors for
+  = return ()            --   $(not_in_scope args)
+
+  | otherwise
+  = do  { traceRn "checkThLocalTyName" (ppr name)
+        ; mb_local_use <- getStageAndBindLevel name
+        ; case mb_local_use of {
+             Nothing -> return () ;  -- Not a locally-bound thing
+             Just (top_lvl, bind_lvl, use_stage) ->
+    do  { let use_lvl = thLevel use_stage
+        ; checkWellStaged (StageCheckSplice name) bind_lvl use_lvl
+        ; traceRn "checkThLocalTyName" (ppr name <+> ppr bind_lvl
+                                                 <+> ppr use_stage
+                                                 <+> ppr use_lvl)
+        ; checkCrossStageLiftingTy top_lvl bind_lvl use_stage use_lvl name } } }
+
 checkThLocalName :: Name -> RnM ()
 checkThLocalName name
   | isUnboundName name   -- Do not report two errors for
@@ -975,6 +995,20 @@ check_cross_stage_lifting top_lvl name ps_var
         ; ps <- readMutVar ps_var
         ; writeMutVar ps_var (pend_splice : ps) }
 
+checkCrossStageLiftingTy :: TopLevelFlag -> ThLevel -> ThStage -> ThLevel -> Name -> TcM ()
+checkCrossStageLiftingTy top_lvl bind_lvl _use_stage use_lvl name
+  | isTopLevel top_lvl
+  = return ()
+
+  -- There is no liftType (yet), so we could error, or more conservatively, just warn.
+  --
+  -- For now, we check here for both untyped and typed splices, as we don't create splices.
+  | use_lvl > bind_lvl
+  = addDiagnostic $ TcRnBadlyStagedType name bind_lvl use_lvl
+
+  | otherwise
+  = return ()
+
 {-
 Note [Keeping things alive for Template Haskell]
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


=====================================
compiler/GHC/Rename/Splice.hs-boot
=====================================
@@ -2,6 +2,7 @@ module GHC.Rename.Splice where
 
 import GHC.Hs
 import GHC.Tc.Utils.Monad
+import GHC.Types.Name (Name)
 import GHC.Types.Name.Set
 
 
@@ -13,3 +14,5 @@ rnSpliceTyPat  :: HsUntypedSplice GhcPs -> RnM ( (HsUntypedSplice GhcRn, HsUntyp
 rnSpliceDecl :: SpliceDecl GhcPs -> RnM (SpliceDecl GhcRn, FreeVars)
 
 rnTopSpliceDecls :: HsUntypedSplice GhcPs -> RnM ([LHsDecl GhcPs], FreeVars)
+
+checkThLocalTyName :: Name -> RnM ()


=====================================
compiler/GHC/Tc/Errors/Ppr.hs
=====================================
@@ -1417,6 +1417,11 @@ instance Diagnostic TcRnMessage where
          text "Stage error:" <+> pprStageCheckReason reason <+>
          hsep [text "is bound at stage" <+> ppr bind_lvl,
                text "but used at stage" <+> ppr use_lvl]
+    TcRnBadlyStagedType name bind_lvl use_lvl
+      -> mkSimpleDecorated $
+         text "Badly staged type:" <+> ppr name <+>
+         hsep [text "is bound at stage" <+> ppr bind_lvl,
+               text "but used at stage" <+> ppr use_lvl]
     TcRnStageRestriction reason
       -> mkSimpleDecorated $
          sep [ text "GHC stage restriction:"
@@ -2279,6 +2284,8 @@ instance Diagnostic TcRnMessage where
       -> ErrorWithoutFlag
     TcRnBadlyStaged{}
       -> ErrorWithoutFlag
+    TcRnBadlyStagedType{}
+      -> WarningWithFlag Opt_WarnBadlyStagedTypes
     TcRnStageRestriction{}
       -> ErrorWithoutFlag
     TcRnTyThingUsedWrong{}
@@ -2916,6 +2923,8 @@ instance Diagnostic TcRnMessage where
       -> noHints
     TcRnBadlyStaged{}
       -> noHints
+    TcRnBadlyStagedType{}
+      -> noHints
     TcRnStageRestriction{}
       -> noHints
     TcRnTyThingUsedWrong{}


=====================================
compiler/GHC/Tc/Errors/Types.hs
=====================================
@@ -3296,6 +3296,21 @@ data TcRnMessage where
     :: !StageCheckReason -- ^ The binding being spliced.
     -> TcRnMessage
 
+  {-| TcRnBadlyStagedWarn is a warning that occurs when a TH type binding is
+    used in an invalid stage.
+
+    Controlled by flags:
+       - Wbadly-staged-type
+
+    Test cases:
+      T23829_timely T23829_tardy T23829_hasty
+  -}
+  TcRnBadlyStagedType
+    :: !Name  -- ^ The type bidning being spliced.
+    -> !Int -- ^ The binding stage.
+    -> !Int -- ^ The sstage at which the binding is used.
+    -> TcRnMessage
+
   {-| TcRnTyThingUsedWrong is an error that occurs when a thing is used where another
     thing was expected.
 


=====================================
compiler/GHC/Tc/Types/TH.hs
=====================================
@@ -17,7 +17,6 @@ import qualified Language.Haskell.TH as TH
 import GHC.Tc.Types.Evidence
 import GHC.Utils.Outputable
 import GHC.Prelude
-import GHC.Utils.Panic
 import GHC.Tc.Types.TcRef
 import GHC.Tc.Types.Constraint
 import GHC.Hs.Expr ( PendingTcSplice, PendingRnSplice )
@@ -105,7 +104,7 @@ thLevel :: ThStage -> ThLevel
 thLevel (Splice _)    = 0
 thLevel Comp          = 1
 thLevel (Brack s _)   = thLevel s + 1
-thLevel (RunSplice _) = panic "thLevel: called when running a splice"
+thLevel (RunSplice _) = 0 -- previously: panic "thLevel: called when running a splice"
                         -- See Note [RunSplice ThLevel].
 
 {- Note [RunSplice ThLevel]
@@ -113,6 +112,12 @@ thLevel (RunSplice _) = panic "thLevel: called when running a splice"
 The 'RunSplice' stage is set when executing a splice, and only when running a
 splice. In particular it is not set when the splice is renamed or typechecked.
 
+However, this is not true. `reifyInstances` for example does rename the given type,
+and these types may contain varialbes (#9262 allow free variables in reifyInstances).
+Therefore here we assume that thLevel (RunSplice _) = 0.
+Proper fix would probably require renaming argument `reifyInstances` separately prior
+to evaluation of the overall splice.
+
 'RunSplice' is needed to provide a reference where 'addModFinalizer' can insert
 the finalizer (see Note [Delaying modFinalizers in untyped splices]), and
 'addModFinalizer' runs when doing Q things. Therefore, It doesn't make sense to


=====================================
compiler/GHC/Tc/Utils/Env.hs
=====================================
@@ -644,8 +644,7 @@ tc_extend_local_env top_lvl extra_env thing_inside
                           -- (GlobalRdrEnv handles the top level)
 
               , tcl_th_bndrs = extendNameEnvList th_bndrs
-                               [(n, thlvl) | (n, ATcId {}) <- extra_env]
-                               -- We only track Ids in tcl_th_bndrs
+                               [(n, thlvl) | (n, _) <- extra_env]
 
               , tcl_env = extendNameEnvList lcl_type_env extra_env }
               -- tcl_rdr and tcl_th_bndrs: extend the local LocalRdrEnv and


=====================================
compiler/GHC/Types/Error/Codes.hs
=====================================
@@ -532,6 +532,7 @@ type family GhcDiagnosticCode c = n | n -> c where
   GhcDiagnosticCode "TcRnIncorrectTyVarOnLhsOfInjCond"              = 88333
   GhcDiagnosticCode "TcRnUnknownTyVarsOnRhsOfInjCond"               = 48254
   GhcDiagnosticCode "TcRnBadlyStaged"                               = 28914
+  GhcDiagnosticCode "TcRnBadlyStagedType"                           = 86357
   GhcDiagnosticCode "TcRnStageRestriction"                          = 18157
   GhcDiagnosticCode "TcRnTyThingUsedWrong"                          = 10969
   GhcDiagnosticCode "TcRnCannotDefaultKindVar"                      = 79924


=====================================
docs/users_guide/using-warnings.rst
=====================================
@@ -78,6 +78,7 @@ as ``-Wno-...`` for every individual warning in the group.
         * :ghc-flag:`-Wforall-identifier`
         * :ghc-flag:`-Wgadt-mono-local-binds`
         * :ghc-flag:`-Wtype-equality-requires-operators`
+        * :ghc-flag:`-Wbadly-staged-types"
 
 .. ghc-flag:: -W
     :shortdesc: enable normal warnings
@@ -2531,4 +2532,21 @@ sanity, not yours.)
         import A
 
      When :ghc-flag:`-Wincomplete-export-warnings` is enabled, GHC warns about exports
-     that are not deprecating a name that is deprecated with another export in that module.
\ No newline at end of file
+     that are not deprecating a name that is deprecated with another export in that module.
+
+.. ghc-flag:: -Wbadly-staged-types
+    :shortdesc: warn when type binding is used at the wrong TH stage.
+    :type: dynamic
+    :reverse: -Wno-badly-staged-types
+
+    :since: 9.10.1
+
+    Consider an example: ::
+
+        tardy :: forall a. Proxy a -> IO Type
+        tardy _ = [t| a |]
+
+    The type binding ``a`` is bound at stage 1 but used on stage 2.
+
+    This is badly staged program, and the ``tardy (Proxy @Int)`` won't produce
+    a type representation of ``Int``, but rather a local name ``a``.


=====================================
testsuite/tests/th/T23829_hasty.hs
=====================================
@@ -0,0 +1,11 @@
+{-# LANGUAGE TemplateHaskellQuotes, TypeApplications #-}
+module T99999_hasty (main) where
+
+import Language.Haskell.TH
+import Data.Proxy
+
+hasty :: Q Type -> Int
+hasty ty = const @Int @($ty) 42
+
+main :: IO ()
+main = print $ hasty [| Char |]


=====================================
testsuite/tests/th/T23829_hasty.stderr
=====================================
@@ -0,0 +1,6 @@
+
+T23829_hasty.hs:8:26: error: [GHC-18157]
+    • GHC stage restriction:
+        ‘ty’ is used in a top-level splice, quasi-quote, or annotation,
+        and must be imported, not defined locally
+    • In the untyped splice: $ty


=====================================
testsuite/tests/th/T23829_tardy.ghc.stderr
=====================================
@@ -0,0 +1,11 @@
+[1 of 2] Compiling Main             ( T23829_tardy.hs, T23829_tardy.o )
+
+T23829_tardy.hs:9:15: warning: [GHC-86357] [-Wbadly-staged-types (in -Wdefault)]
+    Badly staged type: a is bound at stage 1 but used at stage 2
+
+T23829_tardy.hs:12:19: warning: [GHC-86357] [-Wbadly-staged-types (in -Wdefault)]
+    Badly staged type: a is bound at stage 1 but used at stage 2
+
+T23829_tardy.hs:15:20: warning: [GHC-86357] [-Wbadly-staged-types (in -Wdefault)]
+    Badly staged type: a is bound at stage 1 but used at stage 2
+[2 of 2] Linking T23829_tardy


=====================================
testsuite/tests/th/T23829_tardy.hs
=====================================
@@ -0,0 +1,28 @@
+{-# LANGUAGE TemplateHaskellQuotes, TypeApplications #-}
+module Main (main) where
+
+import Language.Haskell.TH
+import Data.Char
+import Data.Proxy
+
+tardy :: forall a. Proxy a -> IO Type
+tardy _ = [t| a |]
+
+tardy2 :: forall a. Proxy a -> IO Exp
+tardy2 _ = [| id @a |]
+
+tardy3 :: forall a. Proxy a -> Code IO (a -> a)
+tardy3 _ = [|| id @a ||]
+
+main :: IO ()
+main = do
+    tardy (Proxy @Int) >>= putStrLn . filt . show
+    tardy2 (Proxy @Int) >>= putStrLn . filt . show
+    examineCode (tardy3 (Proxy @Int)) >>= putStrLn . filt . show . unType
+
+-- ad-hoc filter uniques, a_12313 -> a
+filt :: String -> String
+filt = go where
+    go []           = []
+    go ('_' : rest) = go (dropWhile isDigit rest)
+    go (c:cs)       = c : go cs


=====================================
testsuite/tests/th/T23829_tardy.stdout
=====================================
@@ -0,0 +1,3 @@
+VarT a
+AppTypeE (VarE GHC.Base.id) (VarT a)
+AppTypeE (VarE GHC.Base.id) (VarT a)


=====================================
testsuite/tests/th/T23829_timely.hs
=====================================
@@ -0,0 +1,18 @@
+{-# LANGUAGE TemplateHaskellQuotes, TypeApplications #-}
+module T99999_timely (main) where
+
+import Language.Haskell.TH
+import Data.Proxy
+
+timely :: IO Type
+timely = [t| forall a. a |]
+
+type Foo = Int
+
+timely_top :: IO Type
+timely_top = [t| Foo |]
+
+main :: IO ()
+main = do
+    timely >>= print
+    timely_top >>= print
\ No newline at end of file


=====================================
testsuite/tests/th/all.T
=====================================
@@ -583,3 +583,6 @@ test('T23525', normal, compile, [''])
 test('CodeQ_HKD', normal, compile, [''])
 test('T23748', normal, compile, [''])
 test('T23796', normal, compile, [''])
+test('T23829_timely', normal, compile, [''])
+test('T23829_tardy', normal, warn_and_run, [''])
+test('T23829_hasty', normal, compile_fail, [''])


=====================================
testsuite/tests/typecheck/no_skolem_info/T20969.stderr
=====================================
@@ -1,23 +1,8 @@
 
-T20969.hs:10:40: error: [GHC-39999]
-    • No instance for ‘TH.Lift a’ arising from a use of ‘TH.liftTyped’
-    • In the expression: TH.liftTyped _ :: TH.Code TH.Q a
-      In the first argument of ‘fromList’, namely
-        ‘[TH.liftTyped _ :: TH.Code TH.Q a, [|| x ||]]’
-      In the first argument of ‘sequenceCode’, namely
-        ‘(fromList [TH.liftTyped _ :: TH.Code TH.Q a, [|| x ||]])’
-
-T20969.hs:10:53: error: [GHC-88464]
-    • Found hole: _ :: a
-      Where: ‘a’ is a rigid type variable bound by
-               the type signature for:
-                 glumber :: forall a. Num a => a -> Seq a
-               at T20969.hs:9:1-40
-    • In the first argument of ‘TH.liftTyped’, namely ‘_’
-      In the expression: TH.liftTyped _ :: TH.Code TH.Q a
-      In the first argument of ‘fromList’, namely
-        ‘[TH.liftTyped _ :: TH.Code TH.Q a, [|| x ||]]’
-    • Relevant bindings include
-        x :: a (bound at T20969.hs:10:9)
-        glumber :: a -> Seq a (bound at T20969.hs:10:1)
-      Valid hole fits include x :: a (bound at T20969.hs:10:9)
+T20969.hs:10:71: error: [GHC-18157]
+    • GHC stage restriction:
+        ‘a’ is used in a top-level splice, quasi-quote, or annotation,
+        and must be imported, not defined locally
+    • In the typed splice:
+        $$(sequenceCode
+             (fromList [TH.liftTyped _ :: TH.Code TH.Q a, [|| x ||]]))



View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/6ada2234c9b3accb833b929580efd2d8dd3f67a0...1b78388c94f55df426a8a668a87c8916d89e8ed6

-- 
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/6ada2234c9b3accb833b929580efd2d8dd3f67a0...1b78388c94f55df426a8a668a87c8916d89e8ed6
You're receiving this email because of your account on gitlab.haskell.org.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.haskell.org/pipermail/ghc-commits/attachments/20230822/447b6fcb/attachment-0001.html>


More information about the ghc-commits mailing list