Merge pull request #144 from asmundak/master

Fix 'append final' assignment.

Makefile

@@ -17,8 +17,14 @@ all: ckati ckati_tests
 include Makefile.kati
 include Makefile.ckati
 
-test: all ckati_tests
-	ruby runtest.rb -c -n
+test: run_tests
+
+test_quietly: run_tests
+test_quietly: RUN_TESTS_QUIETLY := -q
+
+run_tests: all ckati_tests
+	ruby runtest.rb -c -n $(RUN_TESTS_QUIETLY)
+
 
 clean: ckati_clean
 

eval.cc

@@ -64,67 +64,63 @@ Var* Evaluator::EvalRHS(Symbol lhs,
                         Value* rhs_v,
                         StringPiece orig_rhs,
                         AssignOp op,
-                        bool is_override) {
+                        bool is_override,
+                        bool *needs_assign) {
   VarOrigin origin =
       ((is_bootstrap_ ? VarOrigin::DEFAULT
                       : is_commandline_ ? VarOrigin::COMMAND_LINE
                                         : is_override ? VarOrigin::OVERRIDE
                                                       : VarOrigin::FILE));
 
-  Var* rhs = NULL;
+  Var* result = NULL;
   Var* prev = NULL;
-  bool needs_assign = true;
+  *needs_assign = true;
 
   switch (op) {
     case AssignOp::COLON_EQ: {
       prev = PeekVarInCurrentScope(lhs);
-      rhs = new SimpleVar(origin, this, rhs_v);
+      result = new SimpleVar(origin, this, rhs_v);
       break;
     }
     case AssignOp::EQ:
       prev = PeekVarInCurrentScope(lhs);
-      rhs = new RecursiveVar(rhs_v, origin, orig_rhs);
+      result = new RecursiveVar(rhs_v, origin, orig_rhs);
       break;
     case AssignOp::PLUS_EQ: {
       prev = LookupVarInCurrentScope(lhs);
       if (!prev->IsDefined()) {
-        rhs = new RecursiveVar(rhs_v, origin, orig_rhs);
+        result = new RecursiveVar(rhs_v, origin, orig_rhs);
       } else if (prev->ReadOnly()) {
         Error(StringPrintf("*** cannot assign to readonly variable: %s",
                            lhs.c_str()));
       } else {
-        prev->AppendVar(this, rhs_v);
-        rhs = prev;
-        needs_assign = false;
+        result = prev;
+        result->AppendVar(this, rhs_v);
+        *needs_assign = false;
       }
       break;
     }
     case AssignOp::QUESTION_EQ: {
       prev = LookupVarInCurrentScope(lhs);
       if (!prev->IsDefined()) {
-        rhs = new RecursiveVar(rhs_v, origin, orig_rhs);
+        result = new RecursiveVar(rhs_v, origin, orig_rhs);
       } else {
-        rhs = prev;
-        needs_assign = false;
+        result = prev;
+        *needs_assign = false;
       }
       break;
     }
   }
 
   if (prev != NULL) {
     prev->Used(this, lhs);
-    if (prev->Deprecated()) {
-      if (needs_assign) {
-        rhs->SetDeprecated(prev->DeprecatedMessage());
-      }
+    if (prev->Deprecated() && *needs_assign) {
+      result->SetDeprecated(prev->DeprecatedMessage());
     }
   }
 
-  LOG("Assign: %s=%s", lhs.c_str(), rhs->DebugString().c_str());
-  if (needs_assign) {
-    return rhs;
-  }
-  return NULL;
+  LOG("Assign: %s=%s", lhs.c_str(), result->DebugString().c_str());
+  return result;
 }
 
 void Evaluator::EvalAssign(const AssignStmt* stmt) {
@@ -148,19 +144,22 @@ void Evaluator::EvalAssign(const AssignStmt* stmt) {
     return;
   }
 
+  bool needs_assign;
   Var* var = EvalRHS(lhs, stmt->rhs, stmt->orig_rhs, stmt->op,
-                     stmt->directive == AssignDirective::OVERRIDE);
-  if (var) {
+                     stmt->directive == AssignDirective::OVERRIDE,
+                     &needs_assign);
+  if (needs_assign) {
     bool readonly;
     lhs.SetGlobalVar(var, stmt->directive == AssignDirective::OVERRIDE,
                      &readonly);
     if (readonly) {
       Error(StringPrintf("*** cannot assign to readonly variable: %s",
                          lhs.c_str()));
     }
-    if (stmt->is_final) {
-      var->SetReadOnly();
-    }
+  }
+
+  if (stmt->is_final) {
+    var->SetReadOnly();
   }
 }
 
@@ -238,18 +237,19 @@ void Evaluator::EvalRuleSpecificAssign(const vector<Symbol>& targets,
     if (var_sym == kKatiReadonlySym) {
       MarkVarsReadonly(rhs);
     } else {
-      Var* rhs_var = EvalRHS(var_sym, rhs, StringPiece("*TODO*"), assign_op, false);
-      if (rhs_var) {
+      bool needs_assign;
+      Var* rhs_var = EvalRHS(var_sym, rhs, StringPiece("*TODO*"), assign_op, false, &needs_assign);
+      if (needs_assign) {
         bool readonly;
         rhs_var->SetAssignOp(assign_op);
         current_scope_->Assign(var_sym, rhs_var, &readonly);
         if (readonly) {
           Error(StringPrintf("*** cannot assign to readonly variable: %s",
                              var_name));
         }
-        if (is_final) {
-          rhs_var->SetReadOnly();
-        }
+      }
+      if (is_final) {
+        rhs_var->SetReadOnly();
       }
     }
     current_scope_ = NULL;

eval.h

@@ -114,7 +114,8 @@ class Evaluator {
                Value* rhs,
                StringPiece orig_rhs,
                AssignOp op,
-               bool is_override);
+               bool is_override,
+               bool *needs_assign);
   void DoInclude(const string& fname);
 
   Var* LookupVarGlobal(Symbol name);

runtest.rb

@@ -39,6 +39,9 @@
   elsif ARGV[0] == '-v'
     show_failing = true
     ARGV.shift
+  elsif ARGV[0] == "-q"
+    hide_passing = true
+    ARGV.shift
   else
     break
   end
@@ -294,7 +297,9 @@ def normalize_kati_log(output)
 
     if expected != output
       if expected_failure
-        puts "#{name}: FAIL (expected)"
+        if !hide_passing
+          puts "#{name}: FAIL (expected)"
+        end
         expected_failures << name
       else
         puts "#{name}: FAIL"
@@ -308,7 +313,9 @@ def normalize_kati_log(output)
         puts "#{name}: PASS (unexpected)"
         unexpected_passes << name
       else
-        puts "#{name}: PASS"
+        if !hide_passing
+          puts "#{name}: PASS"
+        end
         passes << name
       end
     end
@@ -380,7 +387,9 @@ def normalize_kati_log(output)
       puts `diff -u out.make out.kati`
       failures << name
     else
-      puts "#{name}: PASS"
+      if !hide_passing
+        puts "#{name}: PASS"
+      end
       passes << name
     end
   end
@@ -398,7 +407,7 @@ def normalize_kati_log(output)
 
 puts
 
-if !expected_failures.empty?
+if !expected_failures.empty? && !hide_passing
   puts "=== Expected failures ==="
   expected_failures.each do |n|
     puts n

testcase/final_global.sh

@@ -41,3 +41,7 @@ build "=" "+="
 build ":=" ":="
 build ":=" "+="
 build ":=" "="
+
+build "+=" ":="
+build "+=" "+="
+build "+=" "="

testcase/final_rule2.sh

@@ -0,0 +1,33 @@
+#!/bin/bash
+#
+# Copyright 2018 Google Inc. All rights reserved
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http:#www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+set -u
+
+mk="$@"
+
+if echo "${mk}" | grep -q "^make"; then
+  # Make doesn't support final assignment
+  echo "Makefile:3: *** cannot assign to readonly variable: FOO"
+else
+  cat <<EOF > Makefile
+all: FOO +=$= bar
+FOO +=$= foo
+all: FOO +=$= baz
+all:
+EOF
+
+  ${mk} 2>&1 && echo "Clean exit"
+fi