Super-monster patch implementing the new typechecker -- at last

[ghc-hetmet.git] / compiler / coreSyn / CoreUnfold.lhs

diff --git a/compiler/coreSyn/CoreUnfold.lhs b/compiler/coreSyn/CoreUnfold.lhs
index 798d94b..24d6330 100644 (file)
--- a/compiler/coreSyn/CoreUnfold.lhs
+++ b/compiler/coreSyn/CoreUnfold.lhs
@@ -40,9 +40,11 @@ import StaticFlags
 import DynFlags
 import CoreSyn
 import PprCore         ()      -- Instances
 import DynFlags
 import CoreSyn
 import PprCore         ()      -- Instances

+import TcType          ( tcSplitSigmaTy, tcSplitDFunHead )

 import OccurAnal
 import CoreSubst hiding( substTy )
 import CoreFVs         ( exprFreeVars )
 import OccurAnal
 import CoreSubst hiding( substTy )
 import CoreFVs         ( exprFreeVars )

+import CoreArity       ( manifestArity )

 import CoreUtils
 import Id
 import DataCon
 import CoreUtils
 import Id
 import DataCon

@@ -125,8 +127,16 @@ mkCoreUnfolding top_lvl src expr arity guidance
                    uf_expandable = exprIsExpandable expr,
                    uf_guidance   = guidance }
 
                    uf_expandable = exprIsExpandable expr,
                    uf_guidance   = guidance }
 

-mkDFunUnfolding :: DataCon -> [Id] -> Unfolding
-mkDFunUnfolding con ops = DFunUnfolding con (map Var ops)
+mkDFunUnfolding :: Type -> [CoreExpr] -> Unfolding
+mkDFunUnfolding dfun_ty ops 
+  = DFunUnfolding dfun_nargs data_con ops
+  where
+    (tvs, theta, head_ty) = tcSplitSigmaTy dfun_ty
+         -- NB: tcSplitSigmaTy: do not look through a newtype
+         --     when the dictionary type is a newtype
+    (cls, _)   = tcSplitDFunHead head_ty
+    dfun_nargs = length tvs + length theta
+    data_con   = classDataCon cls

 
 mkWwInlineRule :: Id -> CoreExpr -> Arity -> Unfolding
 mkWwInlineRule id expr arity
 
 mkWwInlineRule :: Id -> CoreExpr -> Arity -> Unfolding
 mkWwInlineRule id expr arity

@@ -140,13 +150,17 @@ mkCompulsoryUnfolding expr           -- Used for things that absolutely must be unfolde
                     expr 0    -- Arity of unfolding doesn't matter
                     (UnfWhen unSaturatedOk boringCxtOk)
 
                     expr 0    -- Arity of unfolding doesn't matter
                     (UnfWhen unSaturatedOk boringCxtOk)
 

-mkInlineRule :: Bool -> CoreExpr -> Arity -> Unfolding
-mkInlineRule unsat_ok expr arity 
+mkInlineRule :: CoreExpr -> Maybe Arity -> Unfolding
+mkInlineRule expr mb_arity 

   = mkCoreUnfolding True InlineRule     -- Note [Top-level flag on inline rules]
                    expr' arity 
                    (UnfWhen unsat_ok boring_ok)
   where
     expr' = simpleOptExpr expr
   = mkCoreUnfolding True InlineRule     -- Note [Top-level flag on inline rules]
                    expr' arity 
                    (UnfWhen unsat_ok boring_ok)
   where
     expr' = simpleOptExpr expr

+    (unsat_ok, arity) = case mb_arity of
+                          Nothing -> (unSaturatedOk, manifestArity expr')
+                          Just ar -> (needSaturated, ar)
+              

     boring_ok = case calcUnfoldingGuidance True    -- Treat as cheap
                                           False   -- But not bottoming
                                            (arity+1) expr' of
     boring_ok = case calcUnfoldingGuidance True    -- Treat as cheap
                                           False   -- But not bottoming
                                            (arity+1) expr' of

@@ -181,9 +195,9 @@ calcUnfoldingGuidance expr_is_cheap top_bot bOMB_OUT_SIZE expr
           = case (sizeExpr (iUnbox bOMB_OUT_SIZE) val_bndrs body) of
              TooBig -> UnfNever
              SizeIs size cased_bndrs scrut_discount
           = case (sizeExpr (iUnbox bOMB_OUT_SIZE) val_bndrs body) of
              TooBig -> UnfNever
              SizeIs size cased_bndrs scrut_discount

-               | uncondInline n_val_bndrs (iBox size) && expr_is_cheap
-               -> UnfWhen needSaturated boringCxtOk
-
+               | uncondInline n_val_bndrs (iBox size)
+                , expr_is_cheap
+               -> UnfWhen unSaturatedOk boringCxtOk   -- Note [INLINE for small functions]

                | top_bot  -- See Note [Do not inline top-level bottoming functions]
                -> UnfNever
 
                | top_bot  -- See Note [Do not inline top-level bottoming functions]
                -> UnfNever
 

@@ -239,24 +253,52 @@ Do not re-inline them!  But we *do* still inline if they are very small
 (the uncondInline stuff).
 
 
 (the uncondInline stuff).
 
 

-Note [Unconditional inlining]
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-We inline *unconditionally* if inlined thing is smaller (using sizeExpr)
-than the thing it's replacing.  Notice that
+Note [INLINE for small functions]
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+Consider       {-# INLINE f #-}
+                f x = Just x
+                g y = f y
+Then f's RHS is no larger than its LHS, so we should inline it into
+even the most boring context.  In general, f the function is
+sufficiently small that its body is as small as the call itself, the
+inline unconditionally, regardless of how boring the context is.
+
+Things to note:
+
+ * We inline *unconditionally* if inlined thing is smaller (using sizeExpr)
+   than the thing it's replacing.  Notice that

       (f x) --> (g 3)            -- YES, unconditionally
       (f x) --> x : []           -- YES, *even though* there are two
                                  --      arguments to the cons
       x     --> g 3              -- NO
       x            --> Just v            -- NO
 
       (f x) --> (g 3)            -- YES, unconditionally
       (f x) --> x : []           -- YES, *even though* there are two
                                  --      arguments to the cons
       x     --> g 3              -- NO
       x            --> Just v            -- NO
 

-It's very important not to unconditionally replace a variable by
-a non-atomic term.
+  It's very important not to unconditionally replace a variable by
+  a non-atomic term.
+
+* We do this even if the thing isn't saturated, else we end up with the
+  silly situation that
+     f x y = x
+     ...map (f 3)...
+  doesn't inline.  Even in a boring context, inlining without being
+  saturated will give a lambda instead of a PAP, and will be more
+  efficient at runtime.
+
+* However, when the function's arity > 0, we do insist that it 
+  has at least one value argument at the call site.  Otherwise we find this:
+       f = /\a \x:a. x
+       d = /\b. MkD (f b)
+  If we inline f here we get
+       d = /\b. MkD (\x:b. x)
+  and then prepareRhs floats out the argument, abstracting the type
+  variables, so we end up with the original again!
+

 
 \begin{code}
 uncondInline :: Arity -> Int -> Bool
 -- Inline unconditionally if there no size increase
 -- Size of call is arity (+1 for the function)
 
 \begin{code}
 uncondInline :: Arity -> Int -> Bool
 -- Inline unconditionally if there no size increase
 -- Size of call is arity (+1 for the function)

--- See Note [Unconditional inlining]
+-- See Note [INLINE for small functions]

 uncondInline arity size 
   | arity == 0 = size == 0
   | otherwise  = size <= arity + 1
 uncondInline arity size 
   | arity == 0 = size == 0
   | otherwise  = size <= arity + 1

@@ -444,21 +486,44 @@ funSize top_args fun n_val_args
 
 conSize :: DataCon -> Int -> ExprSize
 conSize dc n_val_args
 
 conSize :: DataCon -> Int -> ExprSize
 conSize dc n_val_args

-  | n_val_args == 0      = SizeIs (_ILIT(0)) emptyBag (_ILIT(1))       -- Like variables
+  | n_val_args == 0 = SizeIs (_ILIT(0)) emptyBag (_ILIT(1))    -- Like variables
+
+-- See Note [Constructor size]

   | isUnboxedTupleCon dc = SizeIs (_ILIT(0)) emptyBag (iUnbox n_val_args +# _ILIT(1))
   | isUnboxedTupleCon dc = SizeIs (_ILIT(0)) emptyBag (iUnbox n_val_args +# _ILIT(1))

-  | otherwise           = SizeIs (_ILIT(1)) emptyBag (iUnbox n_val_args +# _ILIT(1))
-       -- Treat a constructors application as size 1, regardless of how
-       -- many arguments it has; we are keen to expose them
-       -- (and we charge separately for their args).  We can't treat
-       -- them as size zero, else we find that (Just x) has size 0,
-       -- which is the same as a lone variable; and hence 'v' will 
-       -- always be replaced by (Just x), where v is bound to Just x.
-       --
-       -- However, unboxed tuples count as size zero
-       -- I found occasions where we had 
-       --      f x y z = case op# x y z of { s -> (# s, () #) }
-       -- and f wasn't getting inlined

 
 

+-- See Note [Unboxed tuple result discount]
+--  | isUnboxedTupleCon dc = SizeIs (_ILIT(0)) emptyBag (_ILIT(0))
+
+-- See Note [Constructor size]
+  | otherwise = SizeIs (_ILIT(1)) emptyBag (iUnbox n_val_args +# _ILIT(1))
+\end{code}
+
+Note [Constructor size]
+~~~~~~~~~~~~~~~~~~~~~~~
+Treat a constructors application as size 1, regardless of how many
+arguments it has; we are keen to expose them (and we charge separately
+for their args).  We can't treat them as size zero, else we find that
+(Just x) has size 0, which is the same as a lone variable; and hence
+'v' will always be replaced by (Just x), where v is bound to Just x.
+
+However, unboxed tuples count as size zero. I found occasions where we had 
+       f x y z = case op# x y z of { s -> (# s, () #) }
+and f wasn't getting inlined.
+
+Note [Unboxed tuple result discount]
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+I tried giving unboxed tuples a *result discount* of zero (see the
+commented-out line).  Why?  When returned as a result they do not
+allocate, so maybe we don't want to charge so much for them If you
+have a non-zero discount here, we find that workers often get inlined
+back into wrappers, because it look like
+    f x = case $wf x of (# a,b #) -> (a,b)
+and we are keener because of the case.  However while this change
+shrank binary sizes by 0.5% it also made spectral/boyer allocate 5%
+more. All other changes were very small. So it's not a big deal but I
+didn't adopt the idea.
+
+\begin{code}

 primOpSize :: PrimOp -> Int -> ExprSize
 primOpSize op n_val_args
  | not (primOpIsDupable op) = sizeN opt_UF_DearOp
 primOpSize :: PrimOp -> Int -> ExprSize
 primOpSize op n_val_args
  | not (primOpIsDupable op) = sizeN opt_UF_DearOp

@@ -597,9 +662,11 @@ actual arguments.
 \begin{code}
 couldBeSmallEnoughToInline :: Int -> CoreExpr -> Bool
 couldBeSmallEnoughToInline threshold rhs 
 \begin{code}
 couldBeSmallEnoughToInline :: Int -> CoreExpr -> Bool
 couldBeSmallEnoughToInline threshold rhs 

-  = case calcUnfoldingGuidance False False threshold rhs of
-       (_, UnfNever) -> False
-       _             -> True
+  = case sizeExpr (iUnbox threshold) [] body of
+       TooBig -> False
+       _      -> True
+  where
+    (_, body) = collectBinders rhs

 
 ----------------
 smallEnoughToInline :: Unfolding -> Bool
 
 ----------------
 smallEnoughToInline :: Unfolding -> Bool

@@ -687,7 +754,7 @@ callSiteInline dflags id unfolding lone_variable arg_infos cont_info
        NoUnfolding      -> Nothing ;
        OtherCon _       -> Nothing ;
        DFunUnfolding {} -> Nothing ;   -- Never unfold a DFun
        NoUnfolding      -> Nothing ;
        OtherCon _       -> Nothing ;
        DFunUnfolding {} -> Nothing ;   -- Never unfold a DFun

-       CoreUnfolding { uf_tmpl = unf_template, uf_is_top = is_top, uf_is_value = is_value,
+       CoreUnfolding { uf_tmpl = unf_template, uf_is_top = is_top, 

                        uf_is_cheap = is_cheap, uf_arity = uf_arity, uf_guidance = guidance } ->
                        -- uf_arity will typically be equal to (idArity id), 
                        -- but may be less for InlineRules
                        uf_is_cheap = is_cheap, uf_arity = uf_arity, uf_guidance = guidance } ->
                        -- uf_arity will typically be equal to (idArity id), 
                        -- but may be less for InlineRules

@@ -717,19 +784,19 @@ callSiteInline dflags id unfolding lone_variable arg_infos cont_info
 
        interesting_saturated_call 
          = case cont_info of
 
        interesting_saturated_call 
          = case cont_info of

-             BoringCtxt -> not is_top && uf_arity > 0          -- Note [Nested functions]
-             CaseCtxt   -> not (lone_variable && is_value)     -- Note [Lone variables]
-             ArgCtxt {} -> uf_arity > 0                        -- Note [Inlining in ArgCtxt]
-             ValAppCtxt -> True                                -- Note [Cast then apply]
+             BoringCtxt -> not is_top && uf_arity > 0        -- Note [Nested functions]
+             CaseCtxt   -> not (lone_variable && is_cheap)   -- Note [Lone variables]
+             ArgCtxt {} -> uf_arity > 0                      -- Note [Inlining in ArgCtxt]
+             ValAppCtxt -> True                              -- Note [Cast then apply]

 
        (yes_or_no, extra_doc)
          = case guidance of
              UnfNever -> (False, empty)
 
 
        (yes_or_no, extra_doc)
          = case guidance of
              UnfNever -> (False, empty)
 

-             UnfWhen unsat_ok boring_ok -> ( (unsat_ok  || saturated)
-                                           && (boring_ok || some_benefit)
-                                            , empty )
-                  -- For the boring_ok part see Note [INLINE for small functions]
+             UnfWhen unsat_ok boring_ok 
+                 -> (enough_args && (boring_ok || some_benefit), empty )
+                 where      -- See Note [INLINE for small functions]
+                   enough_args = saturated || (unsat_ok && n_val_args > 0)

 
              UnfIfGoodArgs { ug_args = arg_discounts, ug_res = res_discount, ug_size = size }
                 -> ( is_cheap && some_benefit && small_enough
 
              UnfIfGoodArgs { ug_args = arg_discounts, ug_res = res_discount, ug_size = size }
                 -> ( is_cheap && some_benefit && small_enough

@@ -741,13 +808,12 @@ callSiteInline dflags id unfolding lone_variable arg_infos cont_info
                                              res_discount arg_infos cont_info
                
     in    
                                              res_discount arg_infos cont_info
                
     in    

-    if dopt Opt_D_dump_inlinings dflags then
+    if (dopt Opt_D_dump_inlinings dflags && dopt Opt_D_verbose_core2core dflags) then

        pprTrace ("Considering inlining: " ++ showSDoc (ppr id))
                 (vcat [text "arg infos" <+> ppr arg_infos,
                        text "uf arity" <+> ppr uf_arity,
                        text "interesting continuation" <+> ppr cont_info,
                        text "some_benefit" <+> ppr some_benefit,
        pprTrace ("Considering inlining: " ++ showSDoc (ppr id))
                 (vcat [text "arg infos" <+> ppr arg_infos,
                        text "uf arity" <+> ppr uf_arity,
                        text "interesting continuation" <+> ppr cont_info,
                        text "some_benefit" <+> ppr some_benefit,

-                       text "is value:" <+> ppr is_value,

                         text "is cheap:" <+> ppr is_cheap,
                        text "guidance" <+> ppr guidance,
                        extra_doc,
                         text "is cheap:" <+> ppr is_cheap,
                        text "guidance" <+> ppr guidance,
                        extra_doc,

@@ -795,16 +861,6 @@ We *really* want to inline $dmmin, even though it has arity 3, in
 order to unravel the recursion.
 
 
 order to unravel the recursion.
 
 

-Note [INLINE for small functions]
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-Consider       {-# INLINE f #-}
-                f x = Just x
-                g y = f y
-Then f's RHS is no larger than its LHS, so we should inline it
-into even the most boring context.  (We do so if there is no INLINE
-pragma!)  
-
-

 Note [Things to watch]
 ~~~~~~~~~~~~~~~~~~~~~~
 *   { y = I# 3; x = y `cast` co; ...case (x `cast` co) of ... }
 Note [Things to watch]
 ~~~~~~~~~~~~~~~~~~~~~~
 *   { y = I# 3; x = y `cast` co; ...case (x `cast` co) of ... }

@@ -871,8 +927,8 @@ call is at least CONLIKE.  At least for the cases where we use ArgCtxt
 for the RHS of a 'let', we only profit from the inlining if we get a 
 CONLIKE thing (modulo lets).
 
 for the RHS of a 'let', we only profit from the inlining if we get a 
 CONLIKE thing (modulo lets).
 

-Note [Lone variables]
-~~~~~~~~~~~~~~~~~~~~~
+Note [Lone variables]  See also Note [Interaction of exprIsCheap and lone variables]
+~~~~~~~~~~~~~~~~~~~~~   which appears below

 The "lone-variable" case is important.  I spent ages messing about
 with unsatisfactory varaints, but this is nice.  The idea is that if a
 variable appears all alone
 The "lone-variable" case is important.  I spent ages messing about
 with unsatisfactory varaints, but this is nice.  The idea is that if a
 variable appears all alone

@@ -881,7 +937,7 @@ variable appears all alone
        as scrutinee of a case          CaseCtxt
        as arg of a fn                  ArgCtxt
 AND
        as scrutinee of a case          CaseCtxt
        as arg of a fn                  ArgCtxt
 AND

-       it is bound to a value
+       it is bound to a cheap expression

 
 then we should not inline it (unless there is some other reason,
 e.g. is is the sole occurrence).  That is what is happening at 
 
 then we should not inline it (unless there is some other reason,
 e.g. is is the sole occurrence).  That is what is happening at 

@@ -933,6 +989,27 @@ However, watch out:
    There's no advantage in inlining f here, and perhaps
    a significant disadvantage.  Hence some_val_args in the Stop case
 
    There's no advantage in inlining f here, and perhaps
    a significant disadvantage.  Hence some_val_args in the Stop case
 

+Note [Interaction of exprIsCheap and lone variables]
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+The lone-variable test says "don't inline if a case expression
+scrutines a lone variable whose unfolding is cheap".  It's very 
+important that, under these circumstances, exprIsConApp_maybe
+can spot a constructor application. So, for example, we don't
+consider
+       let x = e in (x,x)
+to be cheap, and that's good because exprIsConApp_maybe doesn't
+think that expression is a constructor application.
+
+I used to test is_value rather than is_cheap, which was utterly
+wrong, because the above expression responds True to exprIsHNF.
+
+This kind of thing can occur if you have
+
+       {-# INLINE foo #-}
+       foo = let x = e in (x,x)
+
+which Roman did.
+

 \begin{code}
 computeDiscount :: Int -> [Int] -> Int -> [ArgSummary] -> CallCtxt -> Int
 computeDiscount n_vals_wanted arg_discounts res_discount arg_infos cont_info
 \begin{code}
 computeDiscount :: Int -> [Int] -> Int -> [ArgSummary] -> CallCtxt -> Int
 computeDiscount n_vals_wanted arg_discounts res_discount arg_infos cont_info

@@ -1035,7 +1112,7 @@ interestingArg e = go e 0
     go (Note _ a)       n = go a n
     go (Cast e _)       n = go e n
     go (Lam v e)        n 
     go (Note _ a)       n = go a n
     go (Cast e _)       n = go e n
     go (Lam v e)        n 

-       | isTyVar v        = go e n
+       | isTyCoVar v      = go e n

        | n>0              = go e (n-1)
        | otherwise        = ValueArg
     go (Let _ e)        n = case go e n of { ValueArg -> ValueArg; _ -> NonTrivArg }
        | n>0              = go e (n-1)
        | otherwise        = ValueArg
     go (Let _ e)        n = case go e n of { ValueArg -> ValueArg; _ -> NonTrivArg }

@@ -1155,13 +1232,15 @@ exprIsConApp_maybe id_unf expr
 
     analyse (Var fun) args
        | Just con <- isDataConWorkId_maybe fun
 
     analyse (Var fun) args
        | Just con <- isDataConWorkId_maybe fun

-        , is_saturated
+        , count isValArg args == idArity fun

        , let (univ_ty_args, rest_args) = splitAtList (dataConUnivTyVars con) args
        = Just (con, stripTypeArgs univ_ty_args, rest_args)
 
        -- Look through dictionary functions; see Note [Unfolding DFuns]
        , let (univ_ty_args, rest_args) = splitAtList (dataConUnivTyVars con) args
        = Just (con, stripTypeArgs univ_ty_args, rest_args)
 
        -- Look through dictionary functions; see Note [Unfolding DFuns]

-        | DFunUnfolding con ops <- unfolding
-        , is_saturated
+        | DFunUnfolding dfun_nargs con ops <- unfolding
+        , let sat = length args == dfun_nargs    -- See Note [DFun arity check]
+          in if sat then True else 
+             pprTrace "Unsaturated dfun" (ppr fun <+> int dfun_nargs $$ ppr args) False   

         , let (dfun_tvs, _cls, dfun_res_tys) = tcSplitDFunTy (idType fun)
              subst = zipOpenTvSubst dfun_tvs (stripTypeArgs (takeList dfun_tvs args))
         = Just (con, substTys subst dfun_res_tys, 
         , let (dfun_tvs, _cls, dfun_res_tys) = tcSplitDFunTy (idType fun)
              subst = zipOpenTvSubst dfun_tvs (stripTypeArgs (takeList dfun_tvs args))
         = Just (con, substTys subst dfun_res_tys, 

@@ -1169,14 +1248,11 @@ exprIsConApp_maybe id_unf expr
 
        -- Look through unfoldings, but only cheap ones, because
        -- we are effectively duplicating the unfolding
 
        -- Look through unfoldings, but only cheap ones, because
        -- we are effectively duplicating the unfolding

-       | CoreUnfolding { uf_expandable = expand_me, uf_tmpl = rhs } <- unfolding
-       , expand_me = -- pprTrace "expanding" (ppr fun $$ ppr rhs) $
-                      analyse rhs args
+       | Just rhs <- expandUnfolding_maybe unfolding
+       = -- pprTrace "expanding" (ppr fun $$ ppr rhs) $
+          analyse rhs args

         where
         where

-         is_saturated = count isValArg args == idArity fun
-          unfolding = id_unf fun    -- Does not look through loop breakers
-                   -- ToDo: we *may* look through variables that are NOINLINE
-                   --       in this phase, and that is really not right
+         unfolding = id_unf fun

 
     analyse _ _ = Nothing
 
 
     analyse _ _ = Nothing
 

@@ -1189,11 +1265,7 @@ exprIsConApp_maybe id_unf expr
        = Nothing
 
     beta fun pairs args
        = Nothing
 
     beta fun pairs args

-        = case analyse (substExpr subst fun) args of
-           Nothing  -> -- pprTrace "Bale out! exprIsConApp_maybe" doc $
-                       Nothing
-           Just ans -> -- pprTrace "Woo-hoo! exprIsConApp_maybe" doc $
-                        Just ans
+        = analyse (substExpr (text "subst-expr-is-con-app") subst fun) args

         where
           subst = mkOpenSubst (mkInScopeSet (exprFreeVars fun)) pairs
          -- doc = vcat [ppr fun, ppr expr, ppr pairs, ppr args]
         where
           subst = mkOpenSubst (mkInScopeSet (exprFreeVars fun)) pairs
          -- doc = vcat [ppr fun, ppr expr, ppr pairs, ppr args]

@@ -1216,3 +1288,8 @@ So to split it up we just need to apply the ops $c1, $c2 etc
 to the very same args as the dfun.  It takes a little more work
 to compute the type arguments to the dictionary constructor.
 
 to the very same args as the dfun.  It takes a little more work
 to compute the type arguments to the dictionary constructor.
 

+Note [DFun arity check]
+~~~~~~~~~~~~~~~~~~~~~~~
+Here we check that the total number of supplied arguments (inclding 
+type args) matches what the dfun is expecting.  This may be *less*
+than the ordinary arity of the dfun: see Note [DFun unfoldings] in CoreSyn
\ No newline at end of file