[Git][ghc/ghc][master] simplifier: Zap Id unfoldings before constructing InScopeSet in simpleOptExpr

[Marge Bot (@marge-bot)]

Marge Bot pushed to branch master at Glasgow Haskell Compiler / GHC


Commits:
8273d7d1 by Matthew Pickering at 2025-03-05T04:50:30-05:00
simplifier: Zap Id unfoldings before constructing InScopeSet in simpleOptExpr

Care must be taken to remove unfoldings from `Var`s collected by exprFreeVars
before using them to construct an in-scope set hence `zapIdUnfolding` in `init_subst`.
Consider calling `simpleOptExpr` on an expression like

```
 case x of (a,b) -> (x,a)
```

* One of those two occurrences of x has an unfolding (the one in (x,a), with
unfolding x = (a,b)) and the other does not. (Inside a case GHC adds
unfolding-info to the scrutinee's Id.)
* But exprFreeVars just builds a set, so it's a bit random which occurrence is collected.
* Then simpleOptExpr replaces each occurrence of x with the one in the in-scope set.
* Bad bad bad: then the x in  case x of ... may be replaced with a version that has an unfolding.

Fixes #25790

- - - - -


4 changed files:

- compiler/GHC/Core/SimpleOpt.hs
- + testsuite/tests/ghci/should_run/T25790.hs
- + testsuite/tests/ghci/should_run/T25790.script
- testsuite/tests/ghci/should_run/all.T


Changes:

=====================================
compiler/GHC/Core/SimpleOpt.hs
=====================================
@@ -89,6 +89,24 @@ functions called precisely once, without repeatedly optimising the same
 expression.  In fact, the simple optimiser is a good example of this
 little dance in action; the full Simplifier is a lot more complicated.
 
+Note [The InScopeSet for simpleOptExpr]
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+Care must be taken to remove unfoldings from `Var`s collected by exprFreeVars
+before using them to construct an in-scope set hence `zapIdUnfolding` in `init_subst`.
+Consider calling `simpleOptExpr` on an expression like
+
+```
+ case x of (a,b) -> (x,a)
+```
+
+* One of those two occurrences of x has an unfolding (the one in (x,a), with
+unfolding x = (a,b)) and the other does not. (Inside a case GHC adds
+unfolding-info to the scrutinee's Id.)
+* But exprFreeVars just builds a set, so it's a bit random which occurrence is collected.
+* Then simpleOptExpr replaces each occurrence of x with the one in the in-scope set.
+* Bad bad bad: then the x in  case x of ... may be replaced with a version that has an unfolding.
+
+See ticket #25790
 -}
 
 -- | Simple optimiser options
@@ -135,14 +153,9 @@ simpleOptExpr opts expr
   = -- pprTrace "simpleOptExpr" (ppr init_subst $$ ppr expr)
     simpleOptExprWith opts init_subst expr
   where
-    init_subst = mkEmptySubst (mkInScopeSet (exprFreeVars expr))
-        -- It's potentially important to make a proper in-scope set
-        -- Consider  let x = ..y.. in \y. ...x...
-        -- Then we should remember to clone y before substituting
-        -- for x.  It's very unlikely to occur, because we probably
-        -- won't *be* substituting for x if it occurs inside a
-        -- lambda.
-        --
+    init_subst = mkEmptySubst (mkInScopeSet (mapVarSet zapIdUnfolding (exprFreeVars expr)))
+        -- zapIdUnfolding: see Note [The InScopeSet for simpleOptExpr]
+
         -- It's a bit painful to call exprFreeVars, because it makes
         -- three passes instead of two (occ-anal, and go)
 


=====================================
testsuite/tests/ghci/should_run/T25790.hs
=====================================
@@ -0,0 +1,10 @@
+module T25790
+    ( nest
+    ) where
+
+import Control.Monad.Reader
+
+data RunS = RunS { depth :: Int }
+
+nest :: ReaderT RunS IO a -> ReaderT RunS IO a
+nest = local (\s -> s { depth = depth s })


=====================================
testsuite/tests/ghci/should_run/T25790.script
=====================================
@@ -0,0 +1 @@
+:l T25790.hs


=====================================
testsuite/tests/ghci/should_run/all.T
=====================================
@@ -97,3 +97,4 @@ test('T24115', just_ghci + [extra_run_opts("-e ':add T24115.hs'")], ghci_script,
 
 test('T10920', [only_ways(ghci_ways), extra_files(['LocalPrelude/Prelude.hs'])], ghci_script, ['T10920.script'])
 test('TopEnvIface', [only_ways(ghci_ways)], makefile_test, [])
+test('T25790', [only_ways(ghci_ways), extra_ways(["ghci-opt"])], ghci_script, ['T25790.script'])



View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/8273d7d16ddc1b16096dbab6ad7208dded4ad1b2

-- 
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/8273d7d16ddc1b16096dbab6ad7208dded4ad1b2
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/20250305/413fbdc8/attachment-0001.html>