[GIT PULL] patches for -rc2

[GIT PULL] patches for -rc2

[Luc Van Oostenryck]

Chris,

Please pull these patches for v0.5.1-rc2.
The patches are all about warnings or usability:
* warn for implicit type
* avoid warning on explicit 'bool <- restricted' casts
* finer control over error vs. warnings
* teach cgcc about OSX aka darwin
* add support for -Wmemcpy-max-count
* add more declarations of builtin functions
* add missing names for cgcc to filter out
* speedup the testsuite by avoiding unneeded fork+exec

The only recent changes are parts concerning
the missing names to filter out in cgcc,
thanks to Ramsay Jones to noticing it.

Thank you,
Luc

----------------------------------------------------------------
The following changes since commit 88578349140acf4405b768a60be05e10b7b8b158:

  Merge branches 'dump-macros-v2', 'fix-predefined-size', 'fix-bool-context', 'fix-missing-reload', 'fix-insert-branch', 'fix-NULL-type', 'testsuite-clean', 'fix-bitfield-init-v3' and 'fdump-linearize' into tip (2017-05-19 16:11:51 +0200)

are available in the git repository at:

  git://github.com/lucvoo/sparse.git tags/for-chris

for you to fetch changes up to 8f5e283dc0798ebfea8c019b3f4ea96a8ad00ca1:

  add support for -fmemcpy-max-count (2017-06-14 05:53:10 +0200)

----------------------------------------------------------------
Luc Van Oostenryck (22):
      more tests for implicit 'bool <- restricted' casts
      avoid warning on explicit 'bool <- restricted' casts
      testsuite: get all tags in once
      testsuite: grep the expected output only when needed
      testsuite: grep the output patterns only when needed
      testsuite: use shell arithmetic instead of fork-execing expr
      testsuite: remove unneeded './' before commands
      testsuite: avoid fork+execing basename
      teach cgcc about OSX aka darwin
      ret-void: add test case for toplevel asm
      ret-void: warn for implicit type
      use NULL instead of 0 in testcases.
      finer control over error vs. warnings
      Add more declarations for more builtin functions
      keep the warnings table alphabetically sorted
      cgcc: alphasort warning names in check_only_option()
      cgcc: add missing warning names to check_only_option()
      cgcc: filter-out '-fdump-linearize[=...]'
      memcpy()'s byte count is unsigned
      add support for -Wmemcpy-max-count
      add support for -fmemcpy-max-count

 cgcc                                  |   6 +-
 evaluate.c                            |  14 +++-
 lib.c                                 |  73 +++++++++++++++++-
 lib.h                                 |   6 ++
 parse.c                               |   9 +++
 sparse.1                              |  17 +++++
 sparse.c                              |   6 +-
 validation/alias-mixed.c              |   2 +-
 validation/asm-toplevel.c             |   7 ++
 validation/backend/arithmetic-ops.c   |   2 +-
 validation/backend/array.c            |   2 +-
 validation/backend/bitwise-ops.c      |   2 +-
 validation/backend/bool-test.c        |   2 +-
 validation/backend/cast.c             |   2 +-
 validation/backend/cmp-ops.c          |   2 +-
 validation/backend/extern.c           |   2 +-
 validation/backend/function-ptr.c     |   2 +-
 validation/backend/hello.c            |   2 +-
 validation/backend/int-cond.c         |   2 +-
 validation/backend/load-type.c        |   2 +-
 validation/backend/logical-ops.c      |   2 +-
 validation/backend/loop.c             |   2 +-
 validation/backend/loop2.c            |   2 +-
 validation/backend/ptrcast.c          |   2 +-
 validation/backend/store-type.c       |   2 +-
 validation/backend/struct-access.c    |   2 +-
 validation/backend/struct.c           |   2 +-
 validation/backend/sum.c              |   2 +-
 validation/backend/union.c            |   2 +-
 validation/backend/void-return-type.c |   2 +-
 validation/badtype2.c                 |   1 +
 validation/bool-cast-bad.c            |   1 -
 validation/bool-cast-explicit.c       |   4 -
 validation/bool-cast-restricted.c     |  20 ++++-
 validation/function-redecl.c          |   6 +-
 validation/implicit-ret-type.c        |  15 ++++
 validation/implicit-type.c            |  14 ++++
 validation/prototype.c                |   2 +-
 validation/test-suite                 | 137 +++++++++++++++++++---------------
 validation/typedef_shadow.c           |   1 +
 40 files changed, 279 insertions(+), 104 deletions(-)
 create mode 100644 validation/asm-toplevel.c
 create mode 100644 validation/implicit-ret-type.c
 create mode 100644 validation/implicit-type.c

Re: [GIT PULL] patches for -rc2

[Christopher Li]

On Wed, Jun 14, 2017 at 9:34 PM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> Chris,
>
> Please pull these patches for v0.5.1-rc2.

Thanks, I fetch your change already. Unfortunately I will not have a
chance to take
a closer look until tomorrow night. I will take a crash now.

Chris

Re: [GIT PULL] patches for -rc2

[Luc Van Oostenryck]

On Thu, Jun 15, 2017 at 9:11 AM, Christopher Li <sparse@chrisli.org> wrote:
> On Wed, Jun 14, 2017 at 9:34 PM, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
>> Chris,
>>
>> Please pull these patches for v0.5.1-rc2.
>
> Thanks, I fetch your change already. Unfortunately I will not have a
> chance to take
> a closer look until tomorrow night. I will take a crash now.

Thanks.
No worries.

-- Luc

Re: [GIT PULL] patches for -rc2

[Luc Van Oostenryck]

On Thu, Jun 15, 2017 at 12:11:55AM -0700, Christopher Li wrote:
> On Wed, Jun 14, 2017 at 9:34 PM, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
> > Chris,
> >
> > Please pull these patches for v0.5.1-rc2.
> 
> Thanks, I fetch your change already.

And of course, I had to mess it up by forgetting to add
my SoB to the new patches for cgcc. Sorry for that.
I have added them now, as well as adding missing credits,
and republished the updated version. The patches themselves
haven't changed, only the commit messages.

Please, drop what you have already pulled and take the
following instead.

Sorry again,
-- Luc

The following changes since commit 88578349140acf4405b768a60be05e10b7b8b158:

  Merge branches 'dump-macros-v2', 'fix-predefined-size', 'fix-bool-context', 'fix-missing-reload', 'fix-insert-branch', 'fix-NULL-type', 'testsuite-clean', 'fix-bitfield-init-v3' and 'fdump-linearize' into tip (2017-05-19 16:11:51 +0200)

are available in the git repository at:

  git://github.com/lucvoo/sparse.git tags/for-chris

for you to fetch changes up to bcfe020ed939fa1e8474efaf31a86d80d0e5c5fe:

  add support for -fmemcpy-max-count (2017-06-15 10:03:49 +0200)

----------------------------------------------------------------
Luc Van Oostenryck (22):
      more tests for implicit 'bool <- restricted' casts
      avoid warning on explicit 'bool <- restricted' casts
      testsuite: get all tags in once
      testsuite: grep the expected output only when needed
      testsuite: grep the output patterns only when needed
      testsuite: use shell arithmetic instead of fork-execing expr
      testsuite: remove unneeded './' before commands
      testsuite: avoid fork+execing basename
      teach cgcc about OSX aka darwin
      ret-void: add test case for toplevel asm
      ret-void: warn for implicit type
      use NULL instead of 0 in testcases.
      finer control over error vs. warnings
      Add more declarations for more builtin functions
      keep the warnings table alphabetically sorted
      cgcc: alphasort warning names in check_only_option()
      cgcc: add missing warning names to check_only_option()
      cgcc: filter-out '-fdump-linearize[=...]'
      memcpy()'s byte count is unsigned
      add support for -Wmemcpy-max-count
      add support for -fmemcpy-max-count

 cgcc                                  |   6 +-
 evaluate.c                            |  14 +++-
 lib.c                                 |  73 +++++++++++++++++-
 lib.h                                 |   6 ++
 parse.c                               |   9 +++
 sparse.1                              |  17 +++++
 sparse.c                              |   6 +-
 validation/alias-mixed.c              |   2 +-
 validation/asm-toplevel.c             |   7 ++
 validation/backend/arithmetic-ops.c   |   2 +-
 validation/backend/array.c            |   2 +-
 validation/backend/bitwise-ops.c      |   2 +-
 validation/backend/bool-test.c        |   2 +-
 validation/backend/cast.c             |   2 +-
 validation/backend/cmp-ops.c          |   2 +-
 validation/backend/extern.c           |   2 +-
 validation/backend/function-ptr.c     |   2 +-
 validation/backend/hello.c            |   2 +-
 validation/backend/int-cond.c         |   2 +-
 validation/backend/load-type.c        |   2 +-
 validation/backend/logical-ops.c      |   2 +-
 validation/backend/loop.c             |   2 +-
 validation/backend/loop2.c            |   2 +-
 validation/backend/ptrcast.c          |   2 +-
 validation/backend/store-type.c       |   2 +-
 validation/backend/struct-access.c    |   2 +-
 validation/backend/struct.c           |   2 +-
 validation/backend/sum.c              |   2 +-
 validation/backend/union.c            |   2 +-
 validation/backend/void-return-type.c |   2 +-
 validation/badtype2.c                 |   1 +
 validation/bool-cast-bad.c            |   1 -
 validation/bool-cast-explicit.c       |   4 -
 validation/bool-cast-restricted.c     |  20 ++++-
 validation/function-redecl.c          |   6 +-
 validation/implicit-ret-type.c        |  15 ++++
 validation/implicit-type.c            |  14 ++++
 validation/prototype.c                |   2 +-
 validation/test-suite                 | 137 +++++++++++++++++++---------------
 validation/typedef_shadow.c           |   1 +
 40 files changed, 279 insertions(+), 104 deletions(-)
 create mode 100644 validation/asm-toplevel.c
 create mode 100644 validation/implicit-ret-type.c
 create mode 100644 validation/implicit-type.c

Re: [GIT PULL] patches for -rc2

[Christopher Li]

On Thu, Jun 15, 2017 at 1:20 AM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
>
> And of course, I had to mess it up by forgetting to add
> my SoB to the new patches for cgcc. Sorry for that.
> I have added them now, as well as adding missing credits,
> and republished the updated version. The patches themselves
> haven't changed, only the commit messages.
>
> Please, drop what you have already pulled and take the
> following instead.
>

No worries. I update git fetch already. Will take a look later today.

Chris

Re: [GIT PULL] patches for -rc2

[Ramsay Jones]

On 15/06/17 09:20, Luc Van Oostenryck wrote:
> On Thu, Jun 15, 2017 at 12:11:55AM -0700, Christopher Li wrote:
>> On Wed, Jun 14, 2017 at 9:34 PM, Luc Van Oostenryck
>> <luc.vanoostenryck@gmail.com> wrote:
>>> Chris,
>>>
>>> Please pull these patches for v0.5.1-rc2.
>>
>> Thanks, I fetch your change already.
> 
> And of course, I had to mess it up by forgetting to add
> my SoB to the new patches for cgcc. Sorry for that.
> I have added them now, as well as adding missing credits,
> and republished the updated version. The patches themselves
> haven't changed, only the commit messages.
> 
> Please, drop what you have already pulled and take the
> following instead.
> 
> Sorry again,
> -- Luc
> 
> The following changes since commit 88578349140acf4405b768a60be05e10b7b8b158:
> 
>   Merge branches 'dump-macros-v2', 'fix-predefined-size', 'fix-bool-context', 'fix-missing-reload', 'fix-insert-branch', 'fix-NULL-type', 'testsuite-clean', 'fix-bitfield-init-v3' and 'fdump-linearize' into tip (2017-05-19 16:11:51 +0200)
> 
> are available in the git repository at:
> 
>   git://github.com/lucvoo/sparse.git tags/for-chris
> 
> for you to fetch changes up to bcfe020ed939fa1e8474efaf31a86d80d0e5c5fe:
> 
>   add support for -fmemcpy-max-count (2017-06-15 10:03:49 +0200)
> 

Hi Chris, Luc,

I decided to fetch this tonight and do a bit of light testing. At first,
things seemed to be going well. ;-)
Viz:

  $ cd git
  $ sparse --version
  v0.5.1-rc1-22-gbcfe020
  $ 
  $ rm pack-revindex.o
  $ make CC=cgcc CFLAGS='-Wall' pack-revindex.o
      * new build flags
      CC pack-revindex.o
  pack-revindex.c:64:23: warning: memset with byte count of 262144
  $ 
  $ rm pack-revindex.o
  $ make CC=cgcc CFLAGS='-Wall -Wmemcpy-max-count' pack-revindex.o
      * new build flags
      CC pack-revindex.o
  pack-revindex.c:64:23: warning: memset with byte count of 262144
  $ 
  $ rm pack-revindex.o
  $ make CC=cgcc CFLAGS='-Wall -Wno-memcpy-max-count' pack-revindex.o
      * new build flags
      CC pack-revindex.o
  $ 
  $ rm pack-revindex.o
  $ make CC=cgcc CFLAGS='-Wall -fmemcpy-max-count=0' pack-revindex.o
      * new build flags
      CC pack-revindex.o
  $ 
  $ rm pack-revindex.o
  $ make CC=cgcc CFLAGS='-Wall -fmemcpy-max-count=262145' pack-revindex.o
      * new build flags
      CC pack-revindex.o
  $ 
  $ rm pack-revindex.o
  $ make CC=cgcc CFLAGS='-Wall -fmemcpy-max-count=262144' pack-revindex.o
      * new build flags
      CC pack-revindex.o
  $ 
  $ rm pack-revindex.o
  $ make CC=cgcc CFLAGS='-Wall -fmemcpy-max-count=262143' pack-revindex.o
      * new build flags
      CC pack-revindex.o
  pack-revindex.c:64:23: warning: memset with byte count of 262144
  $ 

I then ran sparse over the entire git project, as I usually do, to see if
there were any new errors/warnings. Unfortunately, there were, which can
be seen as follows:
 
  $ make builtin/rev-parse.sp
      SP builtin/rev-parse.c
  ./git-compat-util.h:480:22: warning: crazy programmer
  ./git-compat-util.h:480:22: warning: crazy programmer
  ./git-compat-util.h:480:22: warning: crazy programmer
  $ 

I used git-bisect the find the commit responsible for the new warnings,
the end of that session was as follows:

  $ git bisect good
  5636cd5cbf816f30ee57d580ec4debd8e0bd7581 is the first bad commit
  commit 5636cd5cbf816f30ee57d580ec4debd8e0bd7581
  Author: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
  Date:   Wed Jan 4 04:03:21 2017 +0100
  
      missing load simplification
      
      In memops:find_dominating_parents(), the 'load-load optimization'
      (see commit cf07903a "Don't bother finding dominating loads if ...")
      cause some loads simplification to be missed.
      For example, with the following code:
              int foo(int *i, int *j)
              {
                      *i = 6;
                      *j = 1;
      
                      do {
                              if (*i != 6)
                                      (*i)++;
                              (*i)++;
                      } while (*i != *j);
      
                      return *j;
              }
      
      test-linearize returns something like:
              foo:
              .L0:
                      <entry-point>
                      store.32    $6 -> 0[%arg1]
                      store.32    $1 -> 0[%arg2]
                      br          .L1
      
              .L1:
                      load.32     %r4 <- 0[%arg1]
                      setne.32    %r5 <- %r4, $6
                      br          %r5, .L4, .L5
      
              .L4:
                      add.32      %r8 <- %r4, $1
                      store.32    %r8 -> 0[%arg1]
                      br          .L5
      
              .L5:
                      load.32     %r10 <- 0[%arg1]
                      add.32      %r11 <- %r10, $1
                      store.32    %r11 -> 0[%arg1]
                      load.32     %r15 <- 0[%arg2]
                      setne.32    %r16 <- %r11, %r15
                      br          %r16, .L1, .L6
      
              .L6:
                      ret.32      %r15
      
      where we can notice that the first load in .L5 is not needed,
      the value could be retrieved from %r4 and %r8, like:
              @@ -8,15 +8,17 @@
               .L1:
                      load.32     %r4 <- 0[%arg1]
                      setne.32    %r5 <- %r4, $6
              +       phisrc.32   %phi4 <- %r4
                      br          %r5, .L4, .L5
      
               .L4:
                      add.32      %r8 <- %r4, $1
                      store.32    %r8 -> 0[%arg1]
              +       phisrc.32   %phi5 <- %r8
                      br          .L5
      
               .L5:
              -       load.32     %r10 <- 0[%arg1]
              +       phi.32      %r10 <- %phi4, %phi5
                      add.32      %r11 <- %r10, $1
                      store.32    %r11 -> 0[%arg1]
                      load.32     %r15 <- 0[%arg2]
      
      The fix essentially consists in reverting commit cf07903a but on
      memops.c's version of find_dominating_parents().
      
      Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
      Signed-off-by: Christopher Li <sparse@chrisli.org>
  
  :100644 100644 6dac1f57833f1708cff72478de969e6b84726ef5 5efdd6f2dab4e475f02ffb9c02e88469662118d3 M	memops.c
  $ 
  
I tried to create a minimal reproducer, but I just couldn't reduce it at all!

The warning points to the 'while' line of the skip_prefix() function in the
git-compat-util.h header file. This is used all over the place, but it seems
to be only complaining about its use in builtin/rev-parse.c. In particular,
there are 16 inlined calls in the function cmd_rev_parse() alone. So I don't
know which 3 of those inlined calls it is complaining about (the warning
message is not much help ...).

The skip_prefix() function looks like:

static inline int skip_prefix(const char *str, const char *prefix,
			      const char **out)
{
	do {
		if (!*prefix) {
			*out = str;
			return 1;
		}
	} while (*str++ == *prefix++);  /* <<<< this is line 480 */
	return 0;
}

That is as far as I have gone in trying to chase this down. I have to get
some sleep now, so I thought I should let you know what I found so far. :-D

ATB,
Ramsay Jones

Re: [GIT PULL] patches for -rc2

[Luc Van Oostenryck]

On Fri, Jun 16, 2017 at 01:03:00AM +0100, Ramsay Jones wrote:
> I decided to fetch this tonight and do a bit of light testing.

Thanks for that.

...
> I then ran sparse over the entire git project, as I usually do, to see if
> there were any new errors/warnings. Unfortunately, there were, which can
> be seen as follows:
>  
>   $ make builtin/rev-parse.sp
>       SP builtin/rev-parse.c
>   ./git-compat-util.h:480:22: warning: crazy programmer
>   ./git-compat-util.h:480:22: warning: crazy programmer
>   ./git-compat-util.h:480:22: warning: crazy programmer
>   $ 
> 
> I used git-bisect the find the commit responsible for the new warnings,
> the end of that session was as follows:
> 
>   $ git bisect good
>   5636cd5cbf816f30ee57d580ec4debd8e0bd7581 is the first bad commit

I'm not much surprised that you see a problem with this patch
but I would have hoped that these "crazy programmer" warnings
would have gone away after a later commit, like it did for the kernel:
	(51cfbc90a "fix: kill unreachable BBs after killing a child")
It visibly doesn't.

> I tried to create a minimal reproducer, but I just couldn't reduce it at all!
> 
> The warning points to the 'while' line of the skip_prefix() function in the
> git-compat-util.h header file. This is used all over the place, but it seems
> to be only complaining about its use in builtin/rev-parse.c. In particular,
> there are 16 inlined calls in the function cmd_rev_parse() alone. So I don't
> know which 3 of those inlined calls it is complaining about (the warning
> message is not much help ...).

Yes, the error message doesn't help much. It is emitted when a cycle is found
between loads, something a correct program should never have but can happen
when there is some unitilized variables that are optimized away. Here, it's
obviously a bug is sparse, though.

> That is as far as I have gone in trying to chase this down. I have to get
> some sleep now, so I thought I should let you know what I found so far. :-D

Thank you very much, it already help a lot. I'm looking at it now.

-- Luc

Re: [GIT PULL] patches for -rc2

[Christopher Li]

On Thu, Jun 15, 2017 at 1:20 AM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:

> Please, drop what you have already pulled and take the
> following instead.

I am reviewing the pull request, here is some of the feed back for the memcpy
patch.

For -fmemcpy-max-count and -Wmemcpy-max-count, do we actually need to
have two separate options here? -fmemcpy-max-count set to zero will disable
this warning any way.

                unsigned long long val = count->value;
-               if (Wmemcpy_max_count && val > 100000ULL)

Re: [GIT PULL] patches for -rc2

[Christopher Li]

On Thu, Jun 15, 2017 at 1:20 AM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
>
> Please, drop what you have already pulled and take the
> following instead.

Here is the feed back for "finer control over error vs. warnings"


+#define        ERROR_CURR_PHASE        (1 << 0)
+#define        ERROR_PREV_PHASE        (1 << 1)
+extern int has_error;

The current phase vs previous phase define is a bit messy to
maintain when you advance to next phase. Currently your
patch does:

+
+       if (has_error & ERROR_CURR_PHASE)
+               has_error = ERROR_PREV_PHASE;

I purpose each different phase/stage we have a dedicate bits.
#define PHASE_PARSING (1 << 0)
#define PHASE_EVALUATE (1<<1)

Have one variable to track which phase we are currently in,
initialized to parsing.

int current_phase = PHASE_PARSING;

When move to evaluation:

current_phase = PHASE_EVALUATE;

The following code:
-       max_warnings = 0;
+       has_error |= ERROR_CURR_PHASE;

will change to:
        has_error |= current_phase;

Then you don't need to move bits between different phase:

+
+       if (has_error & ERROR_CURR_PHASE)
+               has_error = ERROR_PREV_PHASE;

That will be gone. The rest of the patch should be very similar.

This has the advantage of:
- The code clearly show which stage it is in.
- No need to move error bits from current phase to previous phase.
- more friendly if we have more than two phase.

Chris

Re: [GIT PULL] patches for -rc2

[Luc Van Oostenryck]

On Fri, Jun 16, 2017 at 10:08:18AM -0700, Christopher Li wrote:
> On Thu, Jun 15, 2017 at 1:20 AM, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
> 
> > Please, drop what you have already pulled and take the
> > following instead.
> 
> I am reviewing the pull request, here is some of the feed back for the memcpy
> patch.

Mmmm. 10 days ago you said for this series:
	"This V2 version of the series looks perfectly fine to me."
 
> For -fmemcpy-max-count and -Wmemcpy-max-count, do we actually need to
> have two separate options here? -fmemcpy-max-count set to zero will disable
> this warning any way.

We don't need to, but what does it cost us to have both?
What it would bring us if we don't?

I'm seeking coherence, here. In sparse, like in gcc or clang,
warnings are enabled and disabled with '-WXXX' and '-Wno-XXX'
options. The other flag is used to tune some settings.
It's all pretty simple  and without surprises. And using '0'
as special value meaning 'unlimited' is pretty common too.

-- Luc

Re: [GIT PULL] patches for -rc2

[Luc Van Oostenryck]

On Fri, Jun 16, 2017 at 10:27:44AM -0700, Christopher Li wrote:
> On Thu, Jun 15, 2017 at 1:20 AM, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
> >
> > Please, drop what you have already pulled and take the
> > following instead.
> 
> Here is the feed back for "finer control over error vs. warnings"
> 
> 
> +#define        ERROR_CURR_PHASE        (1 << 0)
> +#define        ERROR_PREV_PHASE        (1 << 1)
> +extern int has_error;
> 
> The current phase vs previous phase define is a bit messy to
> maintain when you advance to next phase. Currently your
> patch does:
> 
> +
> +       if (has_error & ERROR_CURR_PHASE)
> +               has_error = ERROR_PREV_PHASE;
> 
> I purpose each different phase/stage we have a dedicate bits.
> #define PHASE_PARSING (1 << 0)
> #define PHASE_EVALUATE (1<<1)

It's what I did first because it was obvious to do so but then
I changed my mind because I found that theer was no real
advantages and a few complications too.

But I don't really mind, I can change it.

....
> That will be gone. The rest of the patch should be very similar.
> 
> This has the advantage of:
> - The code clearly show which stage it is in.
But what is less clear is in what is that an advantage.
> - No need to move error bits from current phase to previous phase.
Indeed, one assignment less ...
> - more friendly if we have more than two phase.
In fact no, you expose something that don't need to. With the
one-def-per-phase, it's 'easier' to set the new state (but again,
it's just an assignement) but when you need to use what you really
need is 'what what the status in the previous phase(s)', whatever
the previous state was.

But then again, I really don't care.

-- Luc