Make noinline more reliable

This patch makes the desugarer rewrite
   noinline (f d)  -->   noinline f d

This makes 'noinline' much more reliable: see #18995

It's explained in the improved Note [noinlineId magic]
in GHC.Types.Id.Make

3 files changed, 39 insertions, 11 deletions

diff --git a/compiler/GHC/CoreToStg/Prep.hs b/compiler/GHC/CoreToStg/Prep.hs
index 460f8ad9ea..f8955ae977 100644
--- a/compiler/GHC/CoreToStg/Prep.hs
+++ b/compiler/GHC/CoreToStg/Prep.hs
@@ -768,7 +768,10 @@ cpeApp top_env expr
             -> UniqSM (Floats, CpeRhs)
     cpe_app env (Var f) (CpeApp Type{} : CpeApp arg : args) depth
         | f `hasKey` lazyIdKey          -- Replace (lazy a) with a, and
+            -- See Note [lazyId magic] in GHC.Types.Id.Make
        || f `hasKey` noinlineIdKey      -- Replace (noinline a) with a
+            -- See Note [noinlineId magic] in GHC.Types.Id.Make
+
         -- Consider the code:
         --
         --      lazy (f x) y
diff --git a/compiler/GHC/HsToCore/Utils.hs b/compiler/GHC/HsToCore/Utils.hs
index 01085b3270..ac66b00813 100644
--- a/compiler/GHC/HsToCore/Utils.hs
+++ b/compiler/GHC/HsToCore/Utils.hs
@@ -494,6 +494,13 @@ mkCoreAppDs _ (Var f `App` Type _r `App` Type ty1 `App` Type ty2 `App` arg1) arg
                           -> v1        -- Note [Desugaring seq], points (2) and (3)
                    _      -> mkWildValBinder Many ty1
 
+mkCoreAppDs _ (Var f `App` Type _r) arg
+  | f `hasKey` noinlineIdKey   -- See Note [noinlineId magic] in GHC.Types.Id.Make
+  , (fun, args) <- collectArgs arg
+  , not (null args)
+  = (Var f `App` Type (exprType fun) `App` fun)
+    `mkCoreApps` args
+
 mkCoreAppDs s fun arg = mkCoreApp s fun arg  -- The rest is done in GHC.Core.Make
 
 -- NB: No argument can be levity polymorphic
diff --git a/compiler/GHC/Types/Id/Make.hs b/compiler/GHC/Types/Id/Make.hs
index 9aa91e3017..dd5657f419 100644
--- a/compiler/GHC/Types/Id/Make.hs
+++ b/compiler/GHC/Types/Id/Make.hs
@@ -1657,18 +1657,36 @@ Implementing 'lazy' is a bit tricky:
 
 Note [noinlineId magic]
 ~~~~~~~~~~~~~~~~~~~~~~~
-noinline :: forall a. a -> a
-
 'noinline' is used to make sure that a function f is never inlined,
-e.g., as in 'noinline f x'.  Ordinarily, the identity function with NOINLINE
-could be used to achieve this effect; however, this has the unfortunate
-result of leaving a (useless) call to noinline at runtime.  So we have
-a little bit of magic to optimize away 'noinline' after we are done
-running the simplifier.
-
-'noinline' needs to be wired-in because it gets inserted automatically
-when we serialize an expression to the interface format. See
-Note [Inlining and hs-boot files] in GHC.CoreToIface
+e.g., as in 'noinline f x'.  We won't inline f because we never inline
+lone variables (see Note [Lone variables] in GHC.Core.Unfold
+
+You might think that we could implement noinline like this:
+   {-# NOINLINE #-}
+   noinline :: forall a. a -> a
+   noinline x = x
+
+But actually we give 'noinline' a wired-in name for three distinct reasons:
+
+1. We don't want to leave a (useless) call to noinline in the final program,
+   to be executed at runtime. So we have a little bit of magic to
+   optimize away 'noinline' after we are done running the simplifier.
+   This is done in GHC.CoreToStg.Prep.cpeApp.
+
+2. 'noinline' sometimes gets inserted automatically when we serialize an
+   expression to the interface format, in GHC.CoreToIface.toIfaceVar.
+   See Note [Inlining and hs-boot files] in GHC.CoreToIface
+
+3. Given foo :: Eq a => [a] -> Bool, the expression
+     noinline foo x xs
+   where x::Int, will naturally desugar to
+      noinline @Int (foo @Int dEqInt) x xs
+   But now it's entirely possible htat (foo @Int dEqInt) will inline foo,
+   since 'foo' is no longer a lone variable -- see #18995
+
+   Solution: in the desugarer, rewrite
+      noinline (f x y)  ==>  noinline f x y
+   This is done in GHC.HsToCore.Utils.mkCoreAppDs.
 
 Note that noinline as currently implemented can hide some simplifications since
 it hides strictness from the demand analyser. Specifically, the demand analyser