Generalize attributes to be comma-separated with arbitrary values

Before:

x = [string null]

After:

x = [string, null]

8 files changed, 119 insertions, 71 deletions

diff --git a/libbuild2/config/operation.cxx b/libbuild2/config/operation.cxx
index 944070b..9f241e8 100644
--- a/libbuild2/config/operation.cxx
+++ b/libbuild2/config/operation.cxx
@@ -43,7 +43,7 @@ namespace build2
         ofs << "# Created automatically by the config module." << endl
             << "#" << endl
             << "src_root = ";
-        to_stream (ofs, name (src_root), true, '@'); // Quote.
+        to_stream (ofs, name (src_root), true /* quote */, '@');
         ofs << endl;
 
         ofs.close ();
@@ -72,7 +72,7 @@ namespace build2
         ofs << "# Created automatically by the config module." << endl
             << "#" << endl
             << "out_root = ";
-        to_stream (ofs, name (out_root), true, '@'); // Quote.
+        to_stream (ofs, name (out_root), true /* quote */, '@');
         ofs << endl;
 
         ofs.close ();
@@ -464,7 +464,7 @@ namespace build2
               else
               {
                 os << " = ";
-                to_stream (os, ns, true, '@'); // Quote.
+                to_stream (os, ns, true /* quote */, '@');
               }
 
               os << endl;
diff --git a/libbuild2/function+call.test.testscript b/libbuild2/function+call.test.testscript
index 569ed80..2e50181 100644
--- a/libbuild2/function+call.test.testscript
+++ b/libbuild2/function+call.test.testscript
@@ -109,7 +109,7 @@ $* <'print $nullable(nonull)' >'false'
 
 : null-fail
 :
-$* <'$dummy1([string null])' 2>>EOE != 0
+$* <'$dummy1([string, null])' 2>>EOE != 0
 error: invalid argument: null value
   buildfile:1:2: info: while calling dummy1(string)
 EOE
diff --git a/libbuild2/lexer.cxx b/libbuild2/lexer.cxx
index b405929..62469ae 100644
--- a/libbuild2/lexer.cxx
+++ b/libbuild2/lexer.cxx
@@ -56,29 +56,32 @@ namespace build2
       }
     case lexer_mode::values:
       {
-        // a: beginning and after `,`?
         s1 = " $(){},#\t\n";
         s2 = "          ";
         break;
       }
     case lexer_mode::switch_expressions:
       {
-        // a: beginning and after `,`?
         s1 = " $(){},:#\t\n";
         s2 = "           ";
         break;
       }
     case lexer_mode::case_patterns:
       {
-        // a: beginning and after `,` & `|`?
         s1 = " $(){},|:#\t\n";
         s2 = "            ";
         break;
       }
     case lexer_mode::attributes:
       {
-        s1 = " $(]#\t\n";
-        s2 = "       ";
+        s1 = " $()=,]#\t\n";
+        s2 = "          ";
+        break;
+      }
+    case lexer_mode::attribute_value:
+      {
+        s1 = " $(),]#\t\n";
+        s2 = "         ";
         break;
       }
     case lexer_mode::eval:
@@ -138,6 +141,7 @@ namespace build2
     case lexer_mode::switch_expressions:
     case lexer_mode::case_patterns:
     case lexer_mode::attributes:
+    case lexer_mode::attribute_value:
     case lexer_mode::variable:
     case lexer_mode::buildspec:     break;
     case lexer_mode::eval:          return next_eval ();
@@ -214,7 +218,7 @@ namespace build2
 
     // The following characters are special in all modes except attributes.
     //
-    if (m != lexer_mode::attributes)
+    if (m != lexer_mode::attributes && m != lexer_mode::attribute_value)
     {
       switch (c)
       {
@@ -229,6 +233,14 @@ namespace build2
     {
       switch (c)
       {
+      case '=': return make_token (type::assign);
+      }
+    }
+
+    if (m == lexer_mode::attributes || m == lexer_mode::attribute_value)
+    {
+      switch (c)
+      {
       case ']':
         {
           state_.pop (); // Expire the attributes mode after closing `]`.
@@ -289,12 +301,14 @@ namespace build2
       }
     }
 
-    // The following characters are special in the values and buildspec mode.
+    // The following characters are special in the values and alike modes.
     //
     if (m == lexer_mode::buildspec          ||
         m == lexer_mode::values             ||
         m == lexer_mode::switch_expressions ||
-        m == lexer_mode::case_patterns)
+        m == lexer_mode::case_patterns      ||
+        m == lexer_mode::attributes         ||
+        m == lexer_mode::attribute_value)
     {
       switch (c)
       {
diff --git a/libbuild2/lexer.hxx b/libbuild2/lexer.hxx
index 3e2fb92..cc9cae6 100644
--- a/libbuild2/lexer.hxx
+++ b/libbuild2/lexer.hxx
@@ -25,10 +25,11 @@ namespace build2
   // values, e.g., `foo = g++`. In contrast, in the variable mode, we restrict
   // certain character (e.g., `/`) from appearing in the name. The values mode
   // is like value but recogizes `,` as special (used in contexts where we
-  // need to list multiple values). The attributes mode is also like value
-  // except it doesn't treat `{` and `}` as special (so we cannot have name
-  // groups in attributes) and recognizes the closing `]`. The eval mode is
-  // used in the evaluation context.
+  // need to list multiple values). The attributes/attribute_value modes are
+  // like values where each value is potentially a variable assignment; they
+  // don't treat `{` and `}` as special (so we cannot have name groups in
+  // attributes) as well as recognizes `=` and `]`. The eval mode is used in
+  // the evaluation context.
   //
   // A number of modes are "derived" from the value/values mode by recognizing
   // a few extra characters:
@@ -74,6 +75,7 @@ namespace build2
       case_patterns,
       switch_expressions,
       attributes,
+      attribute_value,
       eval,
       single_quoted,
       double_quoted,
diff --git a/libbuild2/parser.cxx b/libbuild2/parser.cxx
index d36d501..26df3fa 100644
--- a/libbuild2/parser.cxx
+++ b/libbuild2/parser.cxx
@@ -2917,10 +2917,16 @@ namespace build2
     const location& l (a.loc);
     const value_type* type (nullptr);
 
+    auto print = [storage = names ()] (diag_record& dr, const value& v) mutable
+    {
+      storage.clear ();
+      to_stream (dr.os, reverse (v, storage), true /* quote */, '@');
+    };
+
     for (auto& p: a.ats)
     {
       string& k (p.first);
-      string& v (p.second);
+      value& v (p.second);
 
       if (const value_type* t = map_type (k))
       {
@@ -2935,12 +2941,19 @@ namespace build2
         diag_record dr (fail (l));
         dr << "unknown variable attribute " << k;
 
-        if (!v.empty ())
-          dr << '=' << v;
+        if (!v.null)
+        {
+          dr << '=';
+          print (dr, v);
+        }
       }
 
-      if (!v.empty ())
-        fail (l) << "unexpected value for attribute " << k << ": " << v;
+      if (!v.null)
+      {
+        diag_record dr (fail (l));
+        dr << "unexpected value for attribute " << k << ": ";
+        print (dr, v);
+      }
     }
 
     if (type != nullptr)
@@ -2970,10 +2983,16 @@ namespace build2
     bool null (false);
     const value_type* type (nullptr);
 
+    auto print = [storage = names ()] (diag_record& dr, const value& v) mutable
+    {
+      storage.clear ();
+      to_stream (dr.os, reverse (v, storage), true /* quote */, '@');
+    };
+
     for (auto& p: a.ats)
     {
       string& k (p.first);
-      string& v (p.second);
+      value& v (p.second);
 
       if (k == "null")
       {
@@ -2996,12 +3015,19 @@ namespace build2
         diag_record dr (fail (l));
         dr << "unknown value attribute " << k;
 
-        if (!v.empty ())
-          dr << '=' << v;
+        if (!v.null)
+        {
+          dr << '=';
+          print (dr, v);
+        }
       }
 
-      if (!v.empty ())
-        fail (l) << "unexpected value for attribute " << k << ": " << v;
+      if (!v.null)
+      {
+        diag_record dr (fail (l));
+        dr << "unexpected value for attribute " << k << ": ";
+        print (dr, v);
+      }
     }
 
     // When do we set the type and when do we keep the original? This gets
@@ -3508,53 +3534,59 @@ namespace build2
     if (!has)
       return make_pair (false, l);
 
-    // Using '@' for attribute key-value pairs would be just too ugly. Seeing
-    // that we control what goes into keys/values, let's use a much nicer '='.
-    //
-    mode (lexer_mode::attributes, '=');
+    mode (lexer_mode::attributes);
     next (t, tt);
 
-    has = (tt != type::rsbrace);
-    if (has)
+    if ((has = (tt != type::rsbrace)))
     {
-      names ns (
-        parse_names (t, tt, pattern_mode::ignore, "attribute", nullptr));
-
-      if (!pre_parse_)
+      do
       {
-        attributes& a (attributes_.top ());
+        if (tt == type::newline || tt == type::eos)
+          break;
+
+        // Parse the attribute name with expansion (we rely on this in some
+        // old and hairy tests).
+        //
+        const location l (get_location (t));
 
-        for (auto i (ns.begin ()); i != ns.end (); ++i)
+        names ns (
+          parse_names (t, tt, pattern_mode::ignore, "attribute", nullptr));
+
+        string n;
+        value v;
+
+        if (!pre_parse_)
         {
-          string k, v;
+          // The list should contain a single, simple name.
+          //
+          if (ns.size () != 1 || !ns[0].simple () || ns[0].empty ())
+            fail (l) << "expected attribute name instead of " << ns;
 
-          try
-          {
-            k = convert<string> (move (*i));
-          }
-          catch (const invalid_argument&)
-          {
-            fail (l) << "invalid attribute key '" << *i << "'";
-          }
+          n = move (ns[0].value);
+        }
 
-          if (i->pair)
-          {
-            if (i->pair != '=')
-              fail (l) << "unexpected pair style in attributes";
+        if (tt == type::assign)
+        {
+          // To handle the value we switch into the attribute_value mode
+          // (which doesn't treat `=` as special).
+          //
+          mode (lexer_mode::attribute_value, '@');
+          next (t, tt);
 
-            try
-            {
-              v = convert<string> (move (*++i));
-            }
-            catch (const invalid_argument&)
-            {
-              fail (l) << "invalid attribute value '" << *i << "'";
-            }
-          }
+          v = (tt != type::comma && tt != type::rsbrace
+               ? parse_value (t, tt, pattern_mode::ignore, "attribute value")
+               : value (names ()));
 
-          a.ats.emplace_back (move (k), move (v));
+          expire_mode ();
         }
+
+        if (!pre_parse_)
+          attributes_.top ().ats.emplace_back (move (n), move (v));
+
+        if (tt == type::comma)
+          next (t, tt);
       }
+      while (tt != type::rsbrace);
     }
 
     if (tt != type::rsbrace)
diff --git a/libbuild2/parser.hxx b/libbuild2/parser.hxx
index 347466d..8eef03c 100644
--- a/libbuild2/parser.hxx
+++ b/libbuild2/parser.hxx
@@ -214,9 +214,9 @@ namespace build2
     //
     struct attributes
     {
-      bool has;                         // Has attributes flag.
-      location loc;                     // Start of attributes location.
-      vector<pair<string, string>> ats; // Attributes.
+      bool has;                                 // Has attributes flag.
+      location loc;                             // Start location.
+      small_vector<pair<string, value>, 1> ats; // Attributes.
 
       explicit operator bool () const {return has;}
     };
diff --git a/libbuild2/test/script/lexer.cxx b/libbuild2/test/script/lexer.cxx
index a65eb25..11913d5 100644
--- a/libbuild2/test/script/lexer.cxx
+++ b/libbuild2/test/script/lexer.cxx
@@ -131,7 +131,7 @@ namespace build2
             //
             assert (ps == '\0' ||
                     m == lexer_mode::eval ||
-                    m == lexer_mode::attributes);
+                    m == lexer_mode::attribute_value);
 
             base_lexer::mode (m, ps, esc);
             return;
diff --git a/libbuild2/test/script/parser+pre-parse.test.testscript b/libbuild2/test/script/parser+pre-parse.test.testscript
index f98512a..b2839eb 100644
--- a/libbuild2/test/script/parser+pre-parse.test.testscript
+++ b/libbuild2/test/script/parser+pre-parse.test.testscript
@@ -5,19 +5,19 @@
 : attribute
 :
 {
-  : pair
+  : name
   :
   $* <<EOI 2>>EOE != 0
-  x = [foo=bar]
+  x = [foo]
   EOI
-  testscript:1:5: error: unknown value attribute foo=bar
+  testscript:1:5: error: unknown value attribute foo
   EOE
 
-  : pair-empty
+  : name-value
   :
   $* <<EOI 2>>EOE != 0
-  x = [foo=]
+  x = [foo=bar]
   EOI
-  testscript:1:5: error: unknown value attribute foo
+  testscript:1:5: error: unknown value attribute foo=bar
   EOE
 }