Allow computed variables in depdb preamble similar to impure functions

5 files changed, 77 insertions, 15 deletions

diff --git a/libbuild2/build/script/parser.cxx b/libbuild2/build/script/parser.cxx
index 2112b5e..0614f20 100644
--- a/libbuild2/build/script/parser.cxx
+++ b/libbuild2/build/script/parser.cxx
@@ -93,6 +93,15 @@ namespace build2
             info << "consider using 'depdb' builtin to track its result "
                  << "changes";
 
+        // Diagnose computed variable exansions.
+        //
+        if (computed_var_)
+          fail (*computed_var_)
+            << "expansion of computed variable is only allowed in depdb "
+            << "preamble" <<
+            info << "consider using 'depdb' builtin to track its value "
+                 << "changes";
+
         // Diagnose absent/ambigous script name.
         //
         {
@@ -137,6 +146,7 @@ namespace build2
         // Save the custom dependency change tracking lines, if present.
         //
         s.depdb_clear = depdb_clear_.has_value ();
+        s.depdb_value = depdb_value_;
         if (depdb_dyndep_)
         {
           s.depdb_dyndep = depdb_dyndep_->second;
@@ -579,6 +589,8 @@ namespace build2
                     info (depdb_dyndep_->first) << "'depdb dyndep' call is here";
               }
 
+              depdb_value_ = depdb_value_ || (v == "string" || v == "hash");
+
               // Move the script body to the end of the depdb preamble.
               //
               // Note that at this (pre-parsing) stage we cannot evaluate if
@@ -603,10 +615,12 @@ namespace build2
                 script_->body_temp_dir = false;
               }
 
-              // Reset the impure function call info since it's valid for the
-              // depdb preamble.
+              // Reset the impure function call and computed variable
+              // expansion tracking since both are valid for the depdb
+              // preamble.
               //
               impure_func_ = nullopt;
+              computed_var_ = nullopt;
 
               // Instruct the parser to save the depdb builtin line separately
               // from the script lines, when it is fully parsed. Note that the
@@ -2299,6 +2313,9 @@ namespace build2
         // In the pre-parse mode collect the referenced variable names for the
         // script semantics change tracking.
         //
+        // Note that during pre-parse a computed (including qualified) name
+        // is signalled as an empty name.
+        //
         if (pre_parse_ || pre_parse_suspended_)
         {
           lookup r;
@@ -2332,12 +2349,27 @@ namespace build2
                 vars.push_back (move (name));
             }
           }
+          else
+          {
+            // What about pre_parse_suspended_? Don't think it makes sense to
+            // diagnose this since it can be indirect (that is, via an
+            // intermediate variable).
+            //
+            if (perform_update_ && file_based_ && !computed_var_)
+              computed_var_ = loc;
+          }
 
           return r;
         }
 
         if (!qual.empty ())
-          fail (loc) << "qualified variable name";
+        {
+          // Qualified variable is computed and we expect the user to track
+          // its changes manually.
+          //
+          return build2::script::parser::lookup_variable (
+            move (qual), move (name), loc);
+        }
 
         lookup r (environment_->lookup (name));
 
@@ -2348,13 +2380,13 @@ namespace build2
         // diag builtin argument change (which can be affected by such a
         // variable expansion) doesn't affect the script semantics and the
         // depdb argument is specifically used for the script semantics change
-        // tracking. We also omit this check it the depdb builtin is used in
-        // the script, assuming that such variables are tracked manually, if
-        // required.
+        // tracking. We also omit this check if the depdb "value" (string,
+        // hash) builtin is used in the script, assuming that such variables
+        // are tracked manually, if required.
         //
         if (script_ != nullptr    &&
             !script_->depdb_clear &&
-            script_->depdb_preamble.empty ())
+            !script_->depdb_value)
         {
           if (r.defined () && !r.belongs (*environment_))
           {
diff --git a/libbuild2/build/script/parser.hxx b/libbuild2/build/script/parser.hxx
index a02e34a..932cbad 100644
--- a/libbuild2/build/script/parser.hxx
+++ b/libbuild2/build/script/parser.hxx
@@ -327,7 +327,8 @@ namespace build2
         //                         recipe should be provided.
         //
         //
-        optional<location> depdb_clear_;    // depdb-clear location.
+        optional<location> depdb_clear_;         // depdb-clear location.
+        bool               depdb_value_ = false; // depdb-{string,hash}
         optional<pair<location, size_t>>
                            depdb_dyndep_;   // depdb-dyndep location/position.
         bool               depdb_dyndep_byproduct_ = false; // --byproduct
@@ -344,6 +345,12 @@ namespace build2
         //
         optional<pair<string, location>> impure_func_;
 
+        // Similar to the impure function above but for a computed (e.g.,
+        // target-qualified) variable expansion. In this case we don't have a
+        // name (it's computed).
+        //
+        optional<location> computed_var_;
+
         // True during pre-parsing when the pre-parse mode is temporarily
         // suspended to perform expansion.
         //
diff --git a/libbuild2/build/script/script.hxx b/libbuild2/build/script/script.hxx
index 0619253..d0ab139 100644
--- a/libbuild2/build/script/script.hxx
+++ b/libbuild2/build/script/script.hxx
@@ -75,9 +75,10 @@ namespace build2
         // script parser for details).
         //
         bool             depdb_clear;
+        bool             depdb_value;                    // String or hash.
         optional<size_t> depdb_dyndep;                   // Pos of first dyndep.
         bool             depdb_dyndep_byproduct = false; // dyndep --byproduct
-        lines_type       depdb_preamble;
+        lines_type       depdb_preamble;                 // Note include vars.
         bool             depdb_preamble_temp_dir = false; // True if refs $~.
 
         location start_loc;
diff --git a/libbuild2/parser.hxx b/libbuild2/parser.hxx
index c59b90e..5c9d16a 100644
--- a/libbuild2/parser.hxx
+++ b/libbuild2/parser.hxx
@@ -562,7 +562,7 @@ namespace build2
     // Customization hooks.
     //
   protected:
-    // If qual is not empty, then firt element's pair member indicates the
+    // If qual is not empty, then first element's pair member indicates the
     // kind of qualification:
     //
     // '\0' -- target
diff --git a/tests/recipe/buildscript/testscript b/tests/recipe/buildscript/testscript
index 8632280..0bf752e 100644
--- a/tests/recipe/buildscript/testscript
+++ b/tests/recipe/buildscript/testscript
@@ -108,7 +108,7 @@ posix = ($cxx.target.class != 'windows')
     $* clean 2>-
   }
 
-  : untracked-var
+  : computed-var
   :
   {
     cat <<EOI >=buildfile;
@@ -121,9 +121,29 @@ posix = ($cxx.target.class != 'windows')
       }}
       EOI
 
+    $* 2>>EOE != 0
+      buildfile:6:10: error: expansion of computed variable is only allowed in depdb preamble
+        info: consider using 'depdb' builtin to track its value changes
+      EOE
+  }
+
+  : untracked-var
+  :
+  {
+    cat <<EOI >=buildfile;
+      a = a
+      b = b
+      foo:
+      {{
+        x = true
+        y = $($x ? a : b)
+        depdb env BOGUS
+        echo $y >$path($>)
+      }}
+      EOI
+
     $* 2>>~%EOE% != 0;
-      echo file{foo}
-      buildfile:6:10: error: use of untracked variable 'a'
+      buildfile:6:8: error: use of untracked variable 'a'
         info: use the 'depdb' builtin to manually track it
       %.+
       EOE
@@ -201,13 +221,15 @@ posix = ($cxx.target.class != 'windows')
         a = $process.run(cat baz)
         foo: bar
         {{
+          x = true
+          y = $($x ? a : b)
           depdb hash "$a"
+
           diag compose $>
 
           cp $path($<) $path($>)
 
-          x = true
-          echo "$($x ? a : b)" >>$path($>)
+          echo $y >>$path($>)
         }}
         EOI