Bug 277122 - XUL preprocessor #else-type conditions are evaluated relative to the result of the previous condition only. Fixed the bug with the original patch (unknowing use of a Perl 5.8.0 feature) and relanding... r=bsmedberg

config/preprocessor.pl

@@ -183,6 +183,18 @@ sub fatal {
 
 package stack;
 
+# condition evaluated just prior to this context was false
+use constant COND_FALSE => 0;
+
+# condition evaluated just prior to this context was true
+use constant COND_TRUE => 1;
+
+# some prior condition at this level already evaluated to true (or a
+# parent condition evaluated to false or must be ignored), so we're
+# ignoring all remaining conditions at current level (and nested
+# conditions, too)
+use constant COND_COMPLETED => 2;
+
 sub new {
     return bless {
         'variables' => {
@@ -196,8 +208,8 @@ sub new {
             # filters
         },
         'values' => [], # the value of the last condition evaluated at the nth level
-        'lastPrinting' => [], # whether we were printing at the n-1th level
-        'printing' => 1, # whether we are currently printing at the Nth level
+        'lastConditionState' => [], # whether the condition in the nth-level context was true, false, or not applicable
+        'conditionState' => COND_TRUE,
         'dependencies' => 0, # whether we are showing dependencies
         'lineEndings' => "\n", # default to platform conventions
     };
@@ -242,28 +254,42 @@ sub get {
     }
 }
 
+sub replace {
+    my $self = shift;
+    my ($value) = @_;
+
+    ${$self->{'values'}}[-1] = $value;
+    $self->{'conditionState'} = $self->{'conditionState'} != COND_FALSE
+                              ? COND_COMPLETED
+                              : $value ? COND_TRUE : COND_FALSE;
+}
+
 sub push {
     my $self = shift;
     my($value) = @_;
+
     push(@{$self->{'values'}}, $value);
-    push(@{$self->{'lastPrinting'}}, $self->{'printing'});
-    $self->{'printing'} = $value && $self->{'printing'};
+    my $lastCondition = $self->{'conditionState'};
+    push(@{$self->{'lastConditionState'}}, $lastCondition);
+    $self->{'conditionState'} = $lastCondition != COND_TRUE
+                              ? COND_COMPLETED
+                              : $value ? COND_TRUE : COND_FALSE;
 }
 
 sub pop {
     my $self = shift;
-    $self->{'printing'} = pop(@{$self->{'lastPrinting'}});
+    $self->{'conditionState'} = pop(@{$self->{'lastConditionState'}});
     return pop(@{$self->{'values'}});
 }
 
 sub enabled {
     my $self = shift;
-    return $self->{'printing'};
+    return $self->{'conditionState'} == COND_TRUE;
 }
 
 sub disabled {
     my $self = shift;
-    return not $self->{'printing'};
+    return $self->{'conditionState'} != COND_TRUE;
 }
 
 sub filter {
@@ -329,36 +355,65 @@ sub undef {
 
 sub ifdef {
     my $stack = shift;
-    die "argument expected\n" unless @_;
-    $stack->push($stack->defined(@_));
+    my $variable = shift;
+    my $replace = defined(shift);
+    die "argument expected\n" unless defined($variable);
+    if ($replace) {
+        $stack->replace($stack->defined($variable));
+    } else {
+        $stack->push($stack->defined($variable));
+    }
 }
 
 sub ifndef {
     my $stack = shift;
-    die "argument expected\n" unless @_;
-    $stack->push(not $stack->defined(@_));
+    my $variable = shift;
+    my $replace = defined(shift);
+    die "argument expected\n" unless defined($variable);
+    if ($replace) {
+        $stack->replace(not $stack->defined($variable));
+    } else {
+        $stack->push(not $stack->defined($variable));
+    }
 }
 
 sub if {
     my $stack = shift;
     die "argument expected\n" unless @_;
     my $argument = shift;
+    my $replace = defined(shift);
     for ($argument) {
         /^(\w+)==(.*)$/os && do {
             # equality
-            return $stack->push($stack->get($1) eq $2);
+            if ($replace) {
+                return $stack->replace($stack->get($1) eq $2);
+            } else {
+                return $stack->push($stack->get($1) eq $2);
+            }
         };
         /^(\w+)!=(.*)$/os && do {
             # inequality
-            return $stack->push($stack->get($1) ne $2);
+            if ($replace) {
+                return $stack->replace($stack->get($1) ne $2);
+            } else {
+                return $stack->push($stack->get($1) ne $2);
+            }
         };
         /^(\w+)$/os && do {
             # true value
-            return $stack->push($stack->get($1));
+            if ($replace) {
+                return $stack->replace($stack->get($1));
+            } else {
+                return $stack->push($stack->get($1));
+            }
         };
         /^!(\w+)$/os && do {
             # false value
-            return $stack->push(not $stack->get($1));
+            if ($replace) {
+                return $stack->replace(not $stack->get($1));
+            } else {
+                return $stack->push(not $stack->get($1));
+            }
         };
         die "invalid argument: '$_'\n";
     }
@@ -367,37 +422,25 @@ sub if {
 sub else {
     my $stack = shift;
     die "argument unexpected\n" if @_;
-    $stack->push(not $stack->pop);
+    $stack->replace(1);
 }
 
 sub elif {
     my $stack = shift;
     die "argument expected\n" unless @_;
-    if ($stack->pop) {
-        $stack->push(0);
-    } else {
-        &if($stack, @_);
-    }
+    &if($stack, @_, 1);
 }
 
 sub elifdef {
     my $stack = shift;
     die "argument expected\n" unless @_;
-    if ($stack->pop) {
-        $stack->push(0);
-    } else {
-        &ifdef($stack, @_);
-    }
+    &ifdef($stack, @_, 1);
 }
 
 sub elifndef {
     my $stack = shift;
     die "argument expected\n" unless @_;
-    if ($stack->pop) {
-        $stack->push(0);
-    } else {
-        &ifndef($stack, @_);
-    }
+    &ifndef($stack, @_, 1);
 }
 
 sub endif {

config/preprocessor.txt

@@ -46,20 +46,8 @@ character string, the second defines it to be four characters long.
 The conditionals (#ifdef, #ifndef, #if, #else, #elifdef, #elifndef,
 #elif, #endif) can be nested to arbitrary depth.
 
-An #else section can be followed by an #else section, as in:
-
-   #if 1
-      used
-   #else
-      not used
-   #else
-      used again
-   #endif
-
-Whether this is wise or not is left up to the reader to decide.
-
 The #elifdef, #elifndef, and #elif instructions are equivalent to
-#else instructions combined with the relevant conditional. For
+#else instructions combined with the relevant conditional.  For
 example,
 
    #ifdef foo
@@ -78,33 +66,37 @@ example,
    #endif
    #endif
 
-#else blocks need not come last, which can lead to some odd
-constructs.
+An #else block is included if all previous conditions were false, and
+is equivalent to #elif 1, i.e.:
 
    #ifdef foo
-      included if foo is defined
+      foo is defined
    #else
-      included if foo is not defined
-   #elifdef bar
-      included if foo is defined and bar is defined
-   #else
-      included if either foo or bar are not defined
+      foo is not defined
    #endif
 
-Note in particular the following holds:
-
-   #if 1
-      always included
+...is equivalent to:
+   
+   #ifdef foo
+      foo is defined
    #elif 1
+      foo is not defined
+   #endif
+
+#else is not required to be the last condition in an if/el*/endif
+series.  In particular, along with #else's equivalence to #elif 1
+this means that the following holds:
+
+   #if 0
       never included
    #else
       always included
+   #else
+      never included
+   #elif 1
+      never included
    #endif
 
-That is to say, #else is relative to whether the previous conditional
-was included _only_. It isn't an "and" relationship with previous
-conditionals. This is arguably a bug.
-
 The #error instruction stops execution at this point with a fatal
 error. The error message will include the given STRING.