From d13c6a94a26637c3a6b9a73496dad7947627cf40 Mon Sep 17 00:00:00 2001 From: Chris Pressey Date: Tue, 11 Dec 2018 19:27:40 +0000 Subject: [PATCH 1/3] Drop the check for "consistent initialization" inside `if` blocks. --- HISTORY.md | 3 +++ TODO.md | 9 ------- eg/c64/demo-game/demo-game.60p | 15 ------------ src/sixtypical/analyzer.py | 26 ++++---------------- tests/SixtyPical Analysis.md | 44 ++++++++++++++++++++++++++++++---- 5 files changed, 46 insertions(+), 51 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index 24db63e..34e6b0d 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -4,6 +4,9 @@ History of SixtyPical 0.18 ---- +* The "consistent initialization" check inside `if` blocks has + been dropped. If a location is initialized inside one block + but not the other, it is treated as uninitialized afterwards. * Syntactically, `goto` may only appear at the end of a block. It need no longer be the final instruction in a routine, as long as the type context is consistent at every exit. diff --git a/TODO.md b/TODO.md index 69227f8..6e7857e 100644 --- a/TODO.md +++ b/TODO.md @@ -44,15 +44,6 @@ buffer; and the ones you establish must be disjoint. An alternative would be `static` pointers, which are currently not possible because pointers must be zero-page, thus `@`, thus uninitialized. -### Question "consistent initialization" - -Question the value of the "consistent initialization" principle for `if` statement analysis. - -Part of this is the trashes at the end; I think what it should be is that the trashes -after the `if` is the union of the trashes in each of the branches; this would obviate the -need to `trash` values explicitly, but if you tried to access them afterwards, it would still -error. - ### Tail-call optimization If a block ends in a `call` can that be converted to end in a `goto`? Why not? I think it can, diff --git a/eg/c64/demo-game/demo-game.60p b/eg/c64/demo-game/demo-game.60p index 4097f70..1e6fbd6 100644 --- a/eg/c64/demo-game/demo-game.60p +++ b/eg/c64/demo-game/demo-game.60p @@ -311,14 +311,6 @@ define player_logic logic_routine ld a, 1 st a, player_died } - - // FIXME these trashes, strictly speaking, probably shouldn't be needed, - // but currently the compiler cares a little too much about values that are - // initialized in one branch of an `if`, but not the other, but are trashed - // at the end of the routine anyway. - trash ptr - trash y - trash v } } @@ -355,13 +347,6 @@ define enemy_logic logic_routine add ptr, pos copy 82, [ptr] + y } - - // FIXME these trashes, strictly speaking, probably shouldn't be needed, - // but currently the compiler cares too much about values that are - // initialized in one branch of an `if`, but not the other, but trashed - // at the end of the routine anyway. - trash ptr - trash y } else { copy delta, compare_target st on, c diff --git a/src/sixtypical/analyzer.py b/src/sixtypical/analyzer.py index 9837424..d1006c6 100644 --- a/src/sixtypical/analyzer.py +++ b/src/sixtypical/analyzer.py @@ -414,11 +414,11 @@ class Analyzer(object): if self.debug: print("at end of routine `{}`:".format(routine.name)) print(context) - print("trashed: ", LocationRef.format_set(trashed)) + #print("trashed: ", LocationRef.format_set(trashed)) print("outputs: ", LocationRef.format_set(type_.outputs)) - trashed_outputs = type_.outputs & trashed - if trashed_outputs: - print("TRASHED OUTPUTS: ", LocationRef.format_set(trashed_outputs)) + #trashed_outputs = type_.outputs & trashed + #if trashed_outputs: + # print("TRASHED OUTPUTS: ", LocationRef.format_set(trashed_outputs)) print('') print('-' * 79) print('') @@ -801,24 +801,6 @@ class Analyzer(object): outgoing_meaningful = set(context1.each_meaningful()) & set(context2.each_meaningful()) outgoing_trashes = incoming_meaningful - outgoing_meaningful - # TODO may we need to deal with touched separately here too? - # probably not; if it wasn't meaningful in the first place, it - # doesn't really matter if you modified it or not, coming out. - for ref in context1.each_meaningful(): - if ref in outgoing_trashes: - continue - context2.assert_meaningful( - ref, exception_class=InconsistentInitializationError, - message='initialized in block 1 but not in block 2 of `if {}`'.format(instr.src) - ) - for ref in context2.each_meaningful(): - if ref in outgoing_trashes: - continue - context1.assert_meaningful( - ref, exception_class=InconsistentInitializationError, - message='initialized in block 2 but not in block 1 of `if {}`'.format(instr.src) - ) - # merge the contexts. # first, the easy case: if one of the contexts has terminated, just use the other one. diff --git a/tests/SixtyPical Analysis.md b/tests/SixtyPical Analysis.md index 0331e7e..968521f 100644 --- a/tests/SixtyPical Analysis.md +++ b/tests/SixtyPical Analysis.md @@ -1607,7 +1607,9 @@ Both blocks of an `if` are analyzed. | } = ok -If a location is initialized in one block, is must be initialized in the other as well. +If a location is initialized in one block, is must be initialized in the other as well +in order to be considered to be initialized after the block. If it is not consistent, +it will be considered uninitialized. | define foo routine | inputs a @@ -1621,7 +1623,7 @@ If a location is initialized in one block, is must be initialized in the other a | ld a, 23 | } | } - ? InconsistentInitializationError: x + ? UnmeaningfulOutputError: x | define foo routine | inputs a @@ -1635,7 +1637,7 @@ If a location is initialized in one block, is must be initialized in the other a | ld x, 7 | } | } - ? InconsistentInitializationError: x + ? UnmeaningfulOutputError: x | define foo routine | inputs a @@ -1649,7 +1651,39 @@ If a location is initialized in one block, is must be initialized in the other a | ld x, 7 | } | } - ? InconsistentInitializationError: x + ? UnmeaningfulOutputError: x + +If we don't care if it's uninitialized after the `if`, that's okay then. + + | define foo routine + | inputs a + | trashes a, x, z, n, c + | { + | cmp a, 42 + | if not z { + | ld a, 6 + | } else { + | ld x, 7 + | } + | } + = ok + +Or, if it does get initialized on both branches, that's okay then. + + | define foo routine + | inputs a + | outputs x + | trashes a, z, n, c + | { + | cmp a, 42 + | if not z { + | ld x, 0 + | ld a, 6 + | } else { + | ld x, 7 + | } + | } + = ok However, this only pertains to initialization. If a value is already initialized, either because it was set previous to the `if`, or is an @@ -1698,7 +1732,7 @@ An `if` with a single block is analyzed as if it had an empty `else` block. | ld x, 7 | } | } - ? InconsistentInitializationError: x + ? UnmeaningfulOutputError: x | define foo routine | inputs a From 547b7c960a3af1b10b3ae9108a5311a6cfd2ed12 Mon Sep 17 00:00:00 2001 From: Chris Pressey Date: Tue, 11 Dec 2018 22:40:23 +0000 Subject: [PATCH 2/3] Restore the debugging (which should be rethought, anyway). --- src/sixtypical/analyzer.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/sixtypical/analyzer.py b/src/sixtypical/analyzer.py index d1006c6..2298e53 100644 --- a/src/sixtypical/analyzer.py +++ b/src/sixtypical/analyzer.py @@ -411,14 +411,16 @@ class Analyzer(object): self.analyze_block(routine.block, context) + trashed = set(context.each_touched()) - set(context.each_meaningful()) + if self.debug: print("at end of routine `{}`:".format(routine.name)) print(context) - #print("trashed: ", LocationRef.format_set(trashed)) + print("trashed: ", LocationRef.format_set(trashed)) print("outputs: ", LocationRef.format_set(type_.outputs)) - #trashed_outputs = type_.outputs & trashed - #if trashed_outputs: - # print("TRASHED OUTPUTS: ", LocationRef.format_set(trashed_outputs)) + trashed_outputs = type_.outputs & trashed + if trashed_outputs: + print("TRASHED OUTPUTS: ", LocationRef.format_set(trashed_outputs)) print('') print('-' * 79) print('') @@ -438,8 +440,6 @@ class Analyzer(object): raise InconsistentExitError("Exit contexts are not consistent") context.update_from(exit_context) - trashed = set(context.each_touched()) - set(context.each_meaningful()) - # these all apply whether we encountered goto(s) in this routine, or not...: # can't trash an output. From 538365f1e199140ccf729238a45e1c6da95d49a3 Mon Sep 17 00:00:00 2001 From: Chris Pressey Date: Tue, 11 Dec 2018 22:42:59 +0000 Subject: [PATCH 3/3] Add one more test case, to demonstrate that it's not just output. --- tests/SixtyPical Analysis.md | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/tests/SixtyPical Analysis.md b/tests/SixtyPical Analysis.md index 968521f..a86d548 100644 --- a/tests/SixtyPical Analysis.md +++ b/tests/SixtyPical Analysis.md @@ -1607,7 +1607,7 @@ Both blocks of an `if` are analyzed. | } = ok -If a location is initialized in one block, is must be initialized in the other as well +If a location is initialized in one block, it must be initialized in the other as well in order to be considered to be initialized after the block. If it is not consistent, it will be considered uninitialized. @@ -1653,6 +1653,20 @@ it will be considered uninitialized. | } ? UnmeaningfulOutputError: x + | define foo routine + | inputs a + | trashes a, x, z, n, c + | { + | cmp a, 42 + | if not z { + | ld a, 6 + | } else { + | ld x, 7 + | } + | ld a, x + | } + ? UnmeaningfulReadError: x + If we don't care if it's uninitialized after the `if`, that's okay then. | define foo routine