From af5fa9e744acf6da12f2eab7f44810195c0d3ecd Mon Sep 17 00:00:00 2001
From: Boris Kolpackov <boris@codesynthesis.com>
Date: Mon, 24 May 2021 16:51:24 +0200
Subject: Ban conversion of patterns to values

Also improve conversion diagnostic.
---
 libbuild2/function+call.test.testscript |   2 +-
 libbuild2/name.cxx                      |   2 +
 libbuild2/name.hxx                      |   3 +-
 libbuild2/variable.cxx                  | 101 +++++++++++++-----------
 libbuild2/variable.hxx                  |   2 +
 libbuild2/variable.txx                  | 133 +++++++++++++++++---------------
 tests/directive/assert.testscript       |   2 +-
 tests/directive/config.testscript       |   3 +
 tests/expansion/subscript.testscript    |   2 +-
 9 files changed, 137 insertions(+), 113 deletions(-)

diff --git a/libbuild2/function+call.test.testscript b/libbuild2/function+call.test.testscript
index 8469ab4..03255f2 100644
--- a/libbuild2/function+call.test.testscript
+++ b/libbuild2/function+call.test.testscript
@@ -36,7 +36,7 @@ EOE
 : fail-invalid-arg
 :
 $* <'$fail_arg(abc)' 2>>EOE != 0
-error: invalid argument: invalid uint64 value: 'abc'
+error: invalid argument: invalid uint64 value 'abc'
   buildfile:1:2: info: while calling fail_arg(<untyped>)
 EOE
 
diff --git a/libbuild2/name.cxx b/libbuild2/name.cxx
index 06bf9b9..6a33a63 100644
--- a/libbuild2/name.cxx
+++ b/libbuild2/name.cxx
@@ -34,6 +34,8 @@ namespace build2
   string
   to_string (const name& n)
   {
+    assert (!n.pattern);
+
     string r;
 
     // Note: similar to to_stream() below.
diff --git a/libbuild2/name.hxx b/libbuild2/name.hxx
index 3d14976..5c76d07 100644
--- a/libbuild2/name.hxx
+++ b/libbuild2/name.hxx
@@ -146,7 +146,8 @@ namespace build2
   // Return string representation of a name.
   //
   // Note that this function does not quote special characters and you should
-  // use the to_stream() function below if this is necessary.
+  // use the to_stream() function below if this is necessary. It also cannot
+  // be used on a name pattern.
   //
   LIBBUILD2_SYMEXPORT string
   to_string (const name&);
diff --git a/libbuild2/variable.cxx b/libbuild2/variable.cxx
index 6de0cfb..3e3bf05 100644
--- a/libbuild2/variable.cxx
+++ b/libbuild2/variable.cxx
@@ -432,23 +432,30 @@ namespace build2
   // Throw invalid_argument for an invalid simple value.
   //
   [[noreturn]] static void
-  throw_invalid_argument (const name& n, const name* r, const char* type)
+  throw_invalid_argument (const name& n,
+                          const name* r,
+                          const char* type,
+                          bool pair_ok = false)
   {
     string m;
     string t (type);
 
-    if (r != nullptr)
+    // Note that the message should be suitable for appending "in variable X".
+    //
+    if (!pair_ok && r != nullptr)
       m = "pair in " + t + " value";
+    else if (n.pattern || (r != nullptr && r->pattern))
+      m = "pattern in " + t + " value";
     else
     {
-      m = "invalid " + t + " value: ";
+      m = "invalid " + t + " value ";
 
       if (n.simple ())
         m += "'" + n.value + "'";
       else if (n.directory ())
         m += "'" + n.dir.representation () + "'";
       else
-        m += "complex name";
+        m += "name '" + to_string (n) + "'";
     }
 
     throw invalid_argument (m);
@@ -463,7 +470,7 @@ namespace build2
   bool value_traits<bool>::
   convert (const name& n, const name* r)
   {
-    if (r == nullptr && n.simple ())
+    if (r == nullptr && !n.pattern && n.simple () )
     {
       const string& s (n.value);
 
@@ -504,7 +511,7 @@ namespace build2
   int64_t value_traits<int64_t>::
   convert (const name& n, const name* r)
   {
-    if (r == nullptr && n.simple ())
+    if (r == nullptr && !n.pattern && n.simple ())
     {
       try
       {
@@ -552,7 +559,7 @@ namespace build2
   uint64_t value_traits<uint64_t>::
   convert (const name& n, const name* r)
   {
-    if (r == nullptr && n.simple ())
+    if (r == nullptr && !n.pattern && n.simple ())
     {
       try
       {
@@ -607,9 +614,16 @@ namespace build2
 
     // We can only convert project-qualified simple and directory names.
     //
-    if (!(n.simple (true) || n.directory (true)) ||
-        !(r == nullptr || r->simple (true) || r->directory (true)))
-      throw_invalid_argument (n, r, "string");
+    if (n.pattern ||
+        !(n.simple (true) || n.directory (true)))
+      throw_invalid_argument (n, nullptr, "string");
+
+    if (r != nullptr)
+    {
+      if (r->pattern ||
+          !(r->simple (true) || r->directory (true)))
+        throw_invalid_argument (*r, nullptr, "string");
+    }
 
     string s;
 
@@ -679,7 +693,7 @@ namespace build2
   path value_traits<path>::
   convert (name&& n, name* r)
   {
-    if (r == nullptr)
+    if (r == nullptr && !n.pattern)
     {
       // A directory path is a path.
       //
@@ -746,7 +760,7 @@ namespace build2
   dir_path value_traits<dir_path>::
   convert (name&& n, name* r)
   {
-    if (r == nullptr)
+    if (r == nullptr && !n.pattern)
     {
       if (n.directory ())
         return move (n.dir);
@@ -813,7 +827,7 @@ namespace build2
   abs_dir_path value_traits<abs_dir_path>::
   convert (name&& n, name* r)
   {
-    if (r == nullptr && (n.simple () || n.directory ()))
+    if (r == nullptr && !n.pattern && (n.simple () || n.directory ()))
     {
       try
       {
@@ -861,7 +875,7 @@ namespace build2
   name value_traits<name>::
   convert (name&& n, name* r)
   {
-    if (r == nullptr)
+    if (r == nullptr && !n.pattern)
       return move (n);
 
     throw_invalid_argument (n, r, "name");
@@ -899,6 +913,9 @@ namespace build2
   name_pair value_traits<name_pair>::
   convert (name&& n, name* r)
   {
+    if (n.pattern || (r != nullptr && r->pattern))
+      throw_invalid_argument (n, r, "name_pair", true /* pair_ok */);
+
     n.pair = '\0'; // Keep "unpaired" in case r is empty.
     return name_pair (move (n), r != nullptr ? move (*r) : name ());
   }
@@ -979,8 +996,8 @@ namespace build2
   static T
   process_path_convert (name&& n, name* r, const char* what)
   {
-    if (                   n.untyped () &&  n.unqualified () &&  !n.empty () &&
-        (r == nullptr || (r->untyped () && r->unqualified () && !r->empty ())))
+    if (                   !n.pattern &&  n.untyped () &&  n.unqualified () &&  !n.empty () &&
+        (r == nullptr || (!r->pattern && r->untyped () && r->unqualified () && !r->empty ())))
     {
       path rp (move (n.dir));
       if (rp.empty ())
@@ -1003,7 +1020,7 @@ namespace build2
       return pp;
     }
 
-    throw_invalid_argument (n, r, what);
+    throw_invalid_argument (n, r, what, true /* pair_ok */);
   }
 
   process_path value_traits<process_path>::
@@ -1142,7 +1159,7 @@ namespace build2
       if (!i->pair)
         throw invalid_argument ("non-pair in process_path_ex value");
 
-      if (!i->simple ())
+      if (i->pattern || !i->simple ())
         throw_invalid_argument (*i, nullptr, "process_path_ex");
 
       const string& k ((i++)->value);
@@ -1151,14 +1168,14 @@ namespace build2
       //
       if (k == "name")
       {
-        if (!i->simple ())
+        if (i->pattern || !i->simple ())
           throw_invalid_argument (*i, nullptr, "process_path_ex name");
 
         pp.name = move (i->value);
       }
       else if (k == "checksum")
       {
-        if (!i->simple ())
+        if (i->pattern || !i->simple ())
           throw_invalid_argument (
             *i, nullptr, "process_path_ex executable checksum");
 
@@ -1166,7 +1183,7 @@ namespace build2
       }
       else if (k == "env-checksum")
       {
-        if (!i->simple ())
+        if (i->pattern || !i->simple ())
           throw_invalid_argument (
             *i, nullptr, "process_path_ex environment checksum");
 
@@ -1323,22 +1340,17 @@ namespace build2
   target_triplet value_traits<target_triplet>::
   convert (name&& n, name* r)
   {
-    if (r == nullptr)
+    if (r == nullptr && !n.pattern && n.simple ())
     {
-      if (n.simple ())
+      try
       {
-        try
-        {
-          return n.empty () ? target_triplet () : target_triplet (n.value);
-        }
-        catch (const invalid_argument& e)
-        {
-          throw invalid_argument (
-            string ("invalid target_triplet value: ") + e.what ());
-        }
+        return n.empty () ? target_triplet () : target_triplet (n.value);
+      }
+      catch (const invalid_argument& e)
+      {
+        throw invalid_argument (
+          string ("invalid target_triplet value: ") + e.what ());
       }
-
-      // Fall through.
     }
 
     throw_invalid_argument (n, r, "target_triplet");
@@ -1369,22 +1381,17 @@ namespace build2
   project_name value_traits<project_name>::
   convert (name&& n, name* r)
   {
-    if (r == nullptr)
+    if (r == nullptr && !n.pattern && n.simple ())
     {
-      if (n.simple ())
+      try
       {
-        try
-        {
-          return n.empty () ? project_name () : project_name (move (n.value));
-        }
-        catch (const invalid_argument& e)
-        {
-          throw invalid_argument (
-            string ("invalid project_name value: ") + e.what ());
-        }
+        return n.empty () ? project_name () : project_name (move (n.value));
+      }
+      catch (const invalid_argument& e)
+      {
+        throw invalid_argument (
+          string ("invalid project_name value: ") + e.what ());
       }
-
-      // Fall through.
     }
 
     throw_invalid_argument (n, r, "project_name");
diff --git a/libbuild2/variable.hxx b/libbuild2/variable.hxx
index a40da92..a272013 100644
--- a/libbuild2/variable.hxx
+++ b/libbuild2/variable.hxx
@@ -882,6 +882,8 @@ namespace build2
 
   // name
   //
+  // Note that a typed name is never a pattern.
+  //
   template <>
   struct LIBBUILD2_SYMEXPORT value_traits<name>
   {
diff --git a/libbuild2/variable.txx b/libbuild2/variable.txx
index 3e4a9f3..b1c4112 100644
--- a/libbuild2/variable.txx
+++ b/libbuild2/variable.txx
@@ -128,6 +128,7 @@ namespace build2
   {
     size_t n (ns.size ());
 
+    diag_record dr;
     if (value_traits<T>::empty_value ? n <= 1 : n == 1)
     {
       try
@@ -137,19 +138,23 @@ namespace build2
           (n == 0
            ? T ()
            : value_traits<T>::convert (move (ns.front ()), nullptr)));
-
-        return;
       }
-      catch (const invalid_argument&) {} // Fall through.
+      catch (const invalid_argument& e)
+      {
+        dr << fail << e;
+      }
     }
+    else
+      dr << fail << "invalid " << value_traits<T>::value_type.name
+         << " value: " << (n == 0 ? "empty" : "multiple names");
 
-    diag_record dr (fail);
-
-    dr << "invalid " << value_traits<T>::value_type.name
-       << " value '" << ns << "'";
+    if (!dr.empty ())
+    {
+      if (var != nullptr)
+        dr << " in variable " << var->name;
 
-    if (var != nullptr)
-      dr << " in variable " << var->name;
+      dr << info << "while converting '" << ns << "'";
+    }
   }
 
   template <typename T>
@@ -158,6 +163,7 @@ namespace build2
   {
     size_t n (ns.size ());
 
+    diag_record dr;
     if (value_traits<T>::empty_value ? n <= 1 : n == 1)
     {
       try
@@ -167,19 +173,23 @@ namespace build2
           (n == 0
            ? T ()
            : value_traits<T>::convert (move (ns.front ()), nullptr)));
-
-        return;
       }
-      catch (const invalid_argument&) {} // Fall through.
+      catch (const invalid_argument& e)
+      {
+        dr << fail << e;
+      }
     }
+    else
+      dr << fail << "invalid " << value_traits<T>::value_type.name
+         << " value: " << (n == 0 ? "empty" : "multiple names");
 
-    diag_record dr (fail);
-
-    dr << "invalid " << value_traits<T>::value_type.name
-       << " value '" << ns << "'";
+    if (!dr.empty ())
+    {
+      if (var != nullptr)
+        dr << " in variable " << var->name;
 
-    if (var != nullptr)
-      dr << " in variable " << var->name;
+      dr << info << "while converting '" << ns << "'";
+    }
   }
 
   template <typename T>
@@ -188,6 +198,7 @@ namespace build2
   {
     size_t n (ns.size ());
 
+    diag_record dr;
     if (value_traits<T>::empty_value ? n <= 1 : n == 1)
     {
       try
@@ -197,19 +208,23 @@ namespace build2
           (n == 0
            ? T ()
            : value_traits<T>::convert (move (ns.front ()), nullptr)));
-
-        return;
       }
-      catch (const invalid_argument&) {} // Fall through.
+      catch (const invalid_argument& e)
+      {
+        dr << fail << e;
+      }
     }
+    else
+      dr << fail << "invalid " << value_traits<T>::value_type.name
+         << " value: " << (n == 0 ? "empty" : "multiple names");
 
-    diag_record dr (fail);
-
-    dr << "invalid " << value_traits<T>::value_type.name
-       << " value '" << ns << "'";
+    if (!dr.empty ())
+    {
+      if (var != nullptr)
+        dr << " in variable " << var->name;
 
-    if (var != nullptr)
-      dr << " in variable " << var->name;
+      dr << info << "while converting '" << ns << "'";
+    }
   }
 
   template <typename T>
@@ -282,30 +297,28 @@ namespace build2
 
         return pair<F, S>  (move (f), move (s));
       }
-      catch (const invalid_argument&)
+      catch (const invalid_argument& e)
       {
         diag_record dr (fail);
 
-        dr << "invalid " << value_traits<S>::value_type.name
-           << " second have of pair '" << *r << "'";
-
+        dr << e;
         if (var != nullptr)
           dr << " in variable " << var->name;
 
-        dr << endf;
+        dr << info << "while converting second have of pair '" << *r << "'"
+           << endf;
       }
     }
-    catch (const invalid_argument&)
+    catch (const invalid_argument& e)
     {
       diag_record dr (fail);
 
-      dr << "invalid " << value_traits<F>::value_type.name
-         << " first have of pair '" << l << "'";
-
+      dr << e;
       if (var != nullptr)
         dr << " in variable " << var->name;
 
-      dr << endf;
+      dr << info << "while converting first have of pair '" << l << "'"
+         << endf;
     }
   }
 
@@ -340,30 +353,28 @@ namespace build2
 
         return pair<F, optional<S>>  (move (f), move (s));
       }
-      catch (const invalid_argument&)
+      catch (const invalid_argument& e)
       {
         diag_record dr (fail);
 
-        dr << "invalid " << value_traits<S>::value_type.name
-           << " second have of pair '" << *r << "'";
-
+        dr << e;
         if (var != nullptr)
           dr << " in variable " << var->name;
 
-        dr << endf;
+        dr << info << "while converting second have of pair '" << *r << "'"
+           << endf;
       }
     }
-    catch (const invalid_argument&)
+    catch (const invalid_argument& e)
     {
       diag_record dr (fail);
 
-      dr << "invalid " << value_traits<F>::value_type.name
-         << " first have of pair '" << l << "'";
-
+      dr << e;
       if (var != nullptr)
         dr << " in variable " << var->name;
 
-      dr << endf;
+      dr << info << "while converting first have of pair '" << l << "'"
+         << endf;
     }
   }
 
@@ -401,30 +412,28 @@ namespace build2
 
         return pair<optional<F>, S>  (move (f), move (s));
       }
-      catch (const invalid_argument&)
+      catch (const invalid_argument& e)
       {
         diag_record dr (fail);
 
-        dr << "invalid " << value_traits<S>::value_type.name
-           << " second have of pair '" << l << "'";
-
+        dr << e;
         if (var != nullptr)
           dr << " in variable " << var->name;
 
-        dr << endf;
+        dr << info << "while converting second have of pair '" << *r << "'"
+           << endf;
       }
     }
-    catch (const invalid_argument&)
+    catch (const invalid_argument& e)
     {
       diag_record dr (fail);
 
-      dr << "invalid " << value_traits<F>::value_type.name
-         << " first have of pair '" << l << "'";
-
+      dr << e;
       if (var != nullptr)
         dr << " in variable " << var->name;
 
-      dr << endf;
+      dr << info << "while converting first have of pair '" << l << "'"
+         << endf;
     }
   }
 
@@ -528,19 +537,19 @@ namespace build2
       {
         p.push_back (value_traits<T>::convert (move (n), r));
       }
-      catch (const invalid_argument&)
+      catch (const invalid_argument& e)
       {
         diag_record dr (fail);
 
-        dr << "invalid " << value_traits<T>::value_type.name;
+        dr << e;
+        if (var != nullptr)
+          dr << " in variable " << var->name;
 
+        dr << info << "while converting ";
         if (n.pair)
           dr << " element pair '" << n << "'@'" << *r << "'";
         else
           dr << " element '" << n << "'";
-
-        if (var != nullptr)
-          dr << " in variable " << var->name;
       }
     }
   }
diff --git a/tests/directive/assert.testscript b/tests/directive/assert.testscript
index 2543536..d869863 100644
--- a/tests/directive/assert.testscript
+++ b/tests/directive/assert.testscript
@@ -23,7 +23,7 @@ EOE
 : invalid
 :
 $* <'assert junk' 2>>EOE != 0
-<stdin>:1:8: error: invalid bool value: 'junk'
+<stdin>:1:8: error: invalid bool value 'junk'
 EOE
 
 : null
diff --git a/tests/directive/config.testscript b/tests/directive/config.testscript
index e84ca0a..4049fa0 100644
--- a/tests/directive/config.testscript
+++ b/tests/directive/config.testscript
@@ -56,6 +56,7 @@ test.arguments =
 
   $* noop      config.test.fancy=junk 2>>EOE != 0
     error: invalid bool value 'junk' in variable config.test.fancy
+      info: while converting 'junk'
     EOE
 }
 
@@ -109,6 +110,7 @@ test.arguments =
 
   $* noop      config.test.fancy=junk 2>>EOE != 0
     error: invalid bool value 'junk' in variable config.test.fancy
+      info: while converting 'junk'
     EOE
 }
 
@@ -160,6 +162,7 @@ test.arguments =
 
   $* noop      config.test.fancy=junk 2>>EOE != 0
     error: invalid bool value 'junk' in variable config.test.fancy
+      info: while converting 'junk'
     EOE
 }
 
diff --git a/tests/expansion/subscript.testscript b/tests/expansion/subscript.testscript
index 0c06394..38728d4 100644
--- a/tests/expansion/subscript.testscript
+++ b/tests/expansion/subscript.testscript
@@ -85,7 +85,7 @@ EOE
 : invalid-subscript
 :
 $* <'print ($x[1a])' 2>>EOE != 0
-<stdin>:1:11: error: invalid value subscript: invalid uint64 value: '1a'
+<stdin>:1:11: error: invalid value subscript: invalid uint64 value '1a'
   <stdin>:1:9: info: use the '\[' escape sequence if this is a wildcard pattern
 EOE
 
-- 
cgit v1.1