Skip to content

Commit 68f6f4d

Browse files
committed
Cache intermediate results in totality checking
This saves a lot of unnecessary exploring of size change graphs, which can get over the top quite quickly if there's complex mutual definitions, or even just a single function with an interesting variety of recursive calls. Fixes #1365 Fixes #1277 Fixes #645
1 parent b457d15 commit 68f6f4d

File tree

7 files changed

+67
-16
lines changed

7 files changed

+67
-16
lines changed

src/Core/Termination.idr

Lines changed: 30 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import Core.Value
1111
import Control.Monad.State
1212

1313
import Libraries.Data.NameMap
14+
import Libraries.Data.SortedMap
1415
import Data.List
1516

1617
%default covering
@@ -438,27 +439,41 @@ initArgs (S k)
438439
args' <- initArgs k
439440
pure (Just (arg, Same) :: args')
440441

442+
data Explored : Type where
443+
444+
-- Cached results of exploring the size change graph, so that if we visit a
445+
-- node again, we don't have to re-explore the whole thing
446+
SizeChanges : Type
447+
SizeChanges = SortedMap (Name, List (Maybe Arg)) Terminating
448+
441449
-- Traverse the size change graph. When we reach a point we've seen before,
442450
-- at least one of the arguments must have got smaller, otherwise it's
443451
-- potentially non-terminating
444452
checkSC : {auto a : Ref APos Arg} ->
445453
{auto c : Ref Ctxt Defs} ->
454+
{auto e : Ref Explored SizeChanges} ->
446455
Defs ->
447456
Name -> -- function we're checking
448457
List (Maybe (Arg, SizeChange)) -> -- functions arguments and change
449458
List (Name, List (Maybe Arg)) -> -- calls we've seen so far
450459
Core Terminating
451460
checkSC defs f args path
452-
= do log "totality.termination.sizechange" 7 $ "Checking Size Change Graph: " ++ show !(toFullNames f)
461+
= do exp <- get Explored
462+
log "totality.termination.sizechange" 7 $ "Checking Size Change Graph: " ++ show !(toFullNames f)
453463
let pos = (f, map (map Builtin.fst) args)
454-
if pos `elem` path
455-
then do log "totality.termination.sizechange.inPath" 8 $ "Checking arguments: " ++ show !(toFullNames f)
456-
toFullNames $ checkDesc (mapMaybe (map Builtin.snd) args) path
457-
else case !(lookupCtxtExact f (gamma defs)) of
458-
Nothing => do log "totality.termination.sizechange.isTerminating" 8 $ "Size Change Graph is Terminating for: " ++ show !(toFullNames f)
459-
pure IsTerminating
460-
Just def => do log "totality.termination.sizechange.needsChecking" 8 $ "Size Change Graph needs traversing: " ++ show !(toFullNames f)
461-
continue (sizeChange def) (pos :: path)
464+
case lookup pos exp of
465+
Just done => pure done -- already explored this bit of tree
466+
Nothing =>
467+
if pos `elem` path
468+
then do log "totality.termination.sizechange.inPath" 8 $ "Checking arguments: " ++ show !(toFullNames f)
469+
res <- toFullNames $ checkDesc (mapMaybe (map Builtin.snd) args) path
470+
put Explored (insert pos res exp)
471+
pure res
472+
else case !(lookupCtxtExact f (gamma defs)) of
473+
Nothing => do log "totality.termination.sizechange.isTerminating" 8 $ "Size Change Graph is Terminating for: " ++ show !(toFullNames f)
474+
pure IsTerminating
475+
Just def => do log "totality.termination.sizechange.needsChecking" 8 $ "Size Change Graph needs traversing: " ++ show !(toFullNames f)
476+
continue (sizeChange def) (pos :: path)
462477
where
463478
-- Look for something descending in the list of size changes
464479
checkDesc : List SizeChange -> List (Name, List (Maybe Arg)) -> Terminating
@@ -488,14 +503,14 @@ checkSC defs f args path
488503

489504
checkCall : List (Name, List (Maybe Arg)) -> SCCall -> Core Terminating
490505
checkCall path sc
491-
= do let inpath = fnCall sc `elem` map fst path
492-
Just gdef <- lookupCtxtExact (fnCall sc) (gamma defs)
506+
= do Just gdef <- lookupCtxtExact (fnCall sc) (gamma defs)
493507
| Nothing => pure IsTerminating -- nothing to check
494508
let Unchecked = isTerminating (totality gdef)
495509
| IsTerminating => pure IsTerminating
496510
| _ => pure (NotTerminating (BadCall [fnCall sc]))
497511
log "totality.termination.sizechange.checkCall" 8 $ "CheckCall Size Change Graph: " ++ show !(toFullNames (fnCall sc))
498512
term <- checkSC defs (fnCall sc) (mkArgs (fnArgs sc)) path
513+
let inpath = fnCall sc `elem` map fst path
499514
if not inpath
500515
then case term of
501516
NotTerminating (RecPath _) =>
@@ -505,7 +520,9 @@ checkSC defs f args path
505520
-- function was the top level thing we were checking)
506521
do log "totality.termination.sizechange.checkCall.inPathNot.restart" 9 $ "ReChecking Size Change Graph: " ++ show !(toFullNames (fnCall sc))
507522
args' <- initArgs (length (fnArgs sc))
508-
checkSC defs (fnCall sc) args' path
523+
t <- checkSC defs (fnCall sc) args' path
524+
setTerminating emptyFC (fnCall sc) t
525+
pure t
509526
t => do log "totality.termination.sizechange.checkCall.inPathNot.return" 9 $ "Have result: " ++ show !(toFullNames (fnCall sc))
510527
pure t
511528
else do log "totality.termination.sizechange.checkCall.inPath" 9 $ "Have Result: " ++ show !(toFullNames (fnCall sc))
@@ -535,6 +552,7 @@ calcTerminating loc n
535552
do let ty = type def
536553
a <- newRef APos firstArg
537554
args <- initArgs !(getArity defs [] ty)
555+
e <- newRef Explored empty
538556
checkSC defs n args []
539557
bad => pure bad
540558
where

tests/Main.idr

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,8 @@ idrisTestsPerformance : TestPool
121121
idrisTestsPerformance = MkTestPool "Performance" []
122122
-- Performance: things which have been slow in the past, or which
123123
-- pose interesting challenges for the elaborator
124-
["perf001", "perf002", "perf003", "perf004", "perf005", "perf006"]
124+
["perf001", "perf002", "perf003", "perf004", "perf005", "perf006",
125+
"perf007"]
125126

126127
idrisTestsRegression : TestPool
127128
idrisTestsRegression = MkTestPool "Various regressions" []

tests/idris2/error013/expected

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
1/1: Building Issue361 (Issue361.idr)
2-
Error: main is not total, possibly not terminating due to recursive path Main.main -> Main.Eq implementation at Issue361.idr:5:1--5:11 -> Main.== -> Main./= -> Main.==
2+
Error: main is not total, possibly not terminating due to call to Main./=
33

44
Issue361.idr:7:1--7:13
55
3 | data S = T | F
@@ -9,7 +9,7 @@ Issue361.idr:7:1--7:13
99
7 | main : IO ()
1010
^^^^^^^^^^^^
1111

12-
Error: /= is not total, possibly not terminating due to recursive path Main./= -> Main.== -> Main./= -> Main.==
12+
Error: /= is not total, possibly not terminating due to recursive path Main.main -> Main.Eq implementation at Issue361.idr:5:1--5:11 -> Main.== -> Main./= -> Main.==
1313

1414
Issue361.idr:5:1--5:11
1515
1 | %default total

tests/idris2/perf007/Slooow.idr

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
%default total
2+
3+
data TTy : Nat -> Nat -> Nat -> Nat -> Bool -> Type where
4+
5+
TA : TTy p1 r1 i1 i2 False ->
6+
TTy i1 i2 i1 u2 False ->
7+
TTy i1 i2 p2 b1 b2 ->
8+
TTy p1 r1 p1 i2 False
9+
10+
TB : TTy v1 r1 v2 r2 bt ->
11+
TTy v1 r1 v1 r2 (be && bt)
12+
13+
TC : TTy p1 r1 v2 r2 False ->
14+
TTy v2 r2 v3 r3 b2 ->
15+
TTy p1 r1 v3 r3 b2
16+
17+
n : Nat -> Nat
18+
n = (+ 2)
19+
20+
showInd : (indent : Nat) -> TTy p1 r1 v3 r3 br -> String
21+
showInd i (TA x1 x2 x3) = if False
22+
then (if False then show' i else show' i)
23+
else showInd (n i) x1 ++ show' (n i)
24+
where
25+
show' : Nat -> String
26+
show' i = showInd (n i) x3 ++ if False then "" else showInd (n i) x2
27+
showInd i (TB x) = showInd (n i) x ++ if False then "" else showInd (n i) x
28+
showInd i (TC x y) = if False then showInd i y else showInd i x ++ showInd i y

tests/idris2/perf007/expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
1/1: Building Slooow (Slooow.idr)

tests/idris2/perf007/run

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
$1 --check Slooow.idr
2+
3+
rm -rf build

tests/ttimp/total002/expected

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,5 +5,5 @@ Yaffle> Main.ack is total
55
Yaffle> Main.foo is total
66
Yaffle> Main.foo' is total
77
Yaffle> Main.foom is total
8-
Yaffle> Main.pfoom is possibly not terminating due to recursive path Main.pfoom -> Main.pfoom -> Main.pfoom
8+
Yaffle> Main.pfoom is possibly not terminating due to recursive path Main.pfoom -> Main.pfoom
99
Yaffle> Bye for now!

0 commit comments

Comments
 (0)