Handle duplicate suppression of multi-element libraries (-l foo)

See GitHub issue #114 for context.

7 files changed, 305 insertions, 150 deletions

diff --git a/libbuild2/cc/common.cxx b/libbuild2/cc/common.cxx
index f307b77..dabc007 100644
--- a/libbuild2/cc/common.cxx
+++ b/libbuild2/cc/common.cxx
@@ -32,10 +32,6 @@ namespace build2
     // *.export.libs is up-to-date (which will happen automatically if it is
     // listed as a prerequisite of this library).
     //
-    // Storing a reference to library path in proc_lib is legal (it comes
-    // either from the target's path or from one of the *.libs variables
-    // neither of which should change on this run).
-    //
     // Note that the order of processing is:
     //
     // 1. options (x.* then cc.* to be consistent with poptions/loptions)
@@ -47,7 +43,23 @@ namespace build2
     // array that contains the current library dependency chain all the way to
     // the library passed to process_libraries(). The first element of this
     // array is NULL. If this argument is NULL, then this is a library without
-    // a target (e.g., -lpthread) and its name is in the second argument.
+    // a target (e.g., -lpthread) and its name is in the second argument
+    // (which could be resolved to an absolute path or passed as an -l<name>
+    // option). Otherwise, (the first argument is not NULL), the second
+    // argument contains the target path (which can be empty in case of the
+    // unknown DLL path).
+    //
+    // Initially, the second argument (library name) was a string (e.g.,
+    // -lpthread) but there are cases where the library is identified with
+    // multiple options, such as -framework CoreServices (there are also cases
+    // like -Wl,--whole-archive -lfoo -lbar -Wl,--no-whole-archive). So now it
+    // is a vector_view that contains a fragment of options (from one of the
+    // *.libs variables) that corresponds to the library (or several
+    // libraries, as in the --whole-archive example above).
+    //
+    // Storing a reference to elements of library name in proc_lib is legal
+    // (they come either from the target's path or from one of the *.libs
+    // variables neither of which should change on this run).
     //
     // If proc_impl always returns false (that is, we are only interested in
     // interfaces), then top_li can be absent. This makes process_libraries()
@@ -69,15 +81,16 @@ namespace build2
       bool la,
       lflags lf,
       const function<bool (const target&,
-                           bool la)>& proc_impl, // Implementation?
-      const function<void (const target* const*, // Can be NULL.
-                           const string& path,   // Library path.
-                           lflags,               // Link flags.
-                           bool sys)>& proc_lib, // True if system library.
+                           bool la)>& proc_impl,            // Implementation?
+      const function<void (const target* const*,            // Can be NULL.
+                           const small_vector<reference_wrapper<
+                             const string>, 2>&,            // Library "name".
+                           lflags,                          // Link flags.
+                           bool sys)>& proc_lib,            // System library?
       const function<void (const target&,
-                           const string& type,   // cc.type
-                           bool com,             // cc. or x.
-                           bool exp)>& proc_opt, // *.export.
+                           const string& type,              // cc.type
+                           bool com,                        // cc. or x.
+                           bool exp)>& proc_opt,            // *.export.
       bool self /*= false*/,                     // Call proc_lib on l?
       small_vector<const target*, 16>* chain) const
     {
@@ -196,6 +209,8 @@ namespace build2
 
       // Next process the library itself if requested.
       //
+      small_vector<reference_wrapper<const string>, 2> proc_lib_name; // Reuse.
+
       if (self && proc_lib)
       {
         chain->push_back (&l);
@@ -211,7 +226,8 @@ namespace build2
                 ? cast_false<bool> (l.vars[c_system])
                 : !p.empty () && sys (top_sysd, p.string ()));
 
-        proc_lib (&chain->back (), p.string (), lf, s);
+        proc_lib_name = {p.string ()};
+        proc_lib (&chain->back (), proc_lib_name, lf, s);
       }
 
       const scope& bs (t == nullptr || cc ? top_bs : l.base_scope ());
@@ -299,17 +315,79 @@ namespace build2
           return s;
         };
 
-        auto proc_int = [&l,
-                         &proc_impl, &proc_lib, &proc_opt, chain,
+        // Determine the length of the library name fragment as well as
+        // whether it is a system library. Possible length values are:
+        //
+        // 1 - just the argument itself (-lpthread)
+        // 2 - argument and next element (-l pthread, -framework CoreServices)
+        // 0 - unrecognized/until the end (-Wl,--whole-archive ...)
+        //
+        auto sense_fragment = [&sys_simple, this] (const string& l) ->
+          pair<size_t, bool>
+        {
+          size_t n;
+          bool s (true);
+
+          if (tsys == "win32-msvc")
+          {
+            if (l[0] == '/')
+            {
+              // Some other option (e.g., /WHOLEARCHIVE:<name>).
+              //
+              n = 0;
+            }
+            else
+            {
+              // Presumably a path.
+              //
+              n = 1;
+              s = sys_simple (l);
+            }
+          }
+          else
+          {
+            if (l[0] == '-')
+            {
+              // -l<name>, -l <name>
+              //
+              if (l[1] == 'l')
+              {
+                n = l.size () == 2 ? 2 : 1;
+              }
+              // -framework <name> (Mac OS)
+              //
+              else if (tsys == "darwin" && l == "-framework")
+              {
+                n = 2;
+              }
+              // Some other option (e.g., -Wl,--whole-archive).
+              //
+              else
+                n = 0;
+            }
+            else
+            {
+              // Presumably a path.
+              //
+              n = 1;
+              s = sys_simple (l);
+            }
+          }
+
+          return make_pair (n, s);
+        };
+
+        auto proc_int = [&l, chain,
+                         &proc_impl, &proc_lib, &proc_lib_name, &proc_opt,
                          &sysd, &usrd,
-                         &find_sysd, &find_linfo, &sys_simple,
+                         &find_sysd, &find_linfo, &sense_fragment,
                          &bs, a, &li, impl, this] (const lookup& lu)
         {
           const vector<name>* ns (cast_null<vector<name>> (lu));
           if (ns == nullptr || ns->empty ())
             return;
 
-          for (auto i (ns->begin ()), e (ns->end ()); i != e; ++i)
+          for (auto i (ns->begin ()), e (ns->end ()); i != e; )
           {
             const name& n (*i);
 
@@ -321,7 +399,20 @@ namespace build2
               // files).
               //
               if (proc_lib)
-                proc_lib (nullptr, n.value, 0, sys_simple (n.value));
+              {
+                pair<size_t, bool> r (sense_fragment (n.value));
+
+                proc_lib_name.clear ();
+                for (auto e1 (r.first != 0 ? i + r.first : e);
+                     i != e && i != e1 && i->simple ();
+                     ++i)
+                {
+                  proc_lib_name.push_back (i->value);
+                }
+
+                proc_lib (nullptr, proc_lib_name, 0, r.second);
+                continue;
+              }
             }
             else
             {
@@ -380,23 +471,36 @@ namespace build2
                                  t, t.is_a<liba> () || t.is_a<libux> (), 0,
                                  proc_impl, proc_lib, proc_opt, true, chain);
             }
+
+            ++i;
           }
         };
 
         // Process libraries from *.libs (of type strings).
         //
-        auto proc_imp = [&proc_lib, &sys_simple] (const lookup& lu)
+        auto proc_imp = [&proc_lib, &proc_lib_name,
+                         &sense_fragment] (const lookup& lu)
         {
           const strings* ns (cast_null<strings> (lu));
           if (ns == nullptr || ns->empty ())
             return;
 
-          for (const string& n: *ns)
+          for (auto i (ns->begin ()), e (ns->end ()); i != e; )
           {
             // This is something like -lpthread or shell32.lib so should be a
             // valid path.
             //
-            proc_lib (nullptr, n, 0, sys_simple (n));
+            pair<size_t, bool> r (sense_fragment (*i));
+
+            proc_lib_name.clear ();
+            for (auto e1 (r.first != 0 ? i + r.first : e);
+                 i != e && i != e1;
+                 ++i)
+            {
+              proc_lib_name.push_back (*i);
+            }
+
+            proc_lib (nullptr, proc_lib_name, 0, r.second);
           }
         };
 
diff --git a/libbuild2/cc/common.hxx b/libbuild2/cc/common.hxx
index 758c675..77819db 100644
--- a/libbuild2/cc/common.hxx
+++ b/libbuild2/cc/common.hxx
@@ -282,7 +282,9 @@ namespace build2
         bool,
         lflags,
         const function<bool (const target&, bool)>&,
-        const function<void (const target* const*, const string&, lflags, bool)>&,
+        const function<void (const target* const*,
+                             const small_vector<reference_wrapper<const string>, 2>&,
+                             lflags, bool)>&,
         const function<void (const target&, const string&, bool, bool)>&,
         bool = false,
         small_vector<const target*, 16>* = nullptr) const;
diff --git a/libbuild2/cc/compile-rule.cxx b/libbuild2/cc/compile-rule.cxx
index ce586bb..40fb5bb 100644
--- a/libbuild2/cc/compile-rule.cxx
+++ b/libbuild2/cc/compile-rule.cxx
@@ -6193,10 +6193,11 @@ namespace build2
           const target*& lt;
         } d {a, ht, lt};
 
-        auto lib = [&d] (const target* const* lc,
-                         const string&,
-                         lflags,
-                         bool)
+        auto lib = [&d] (
+          const target* const* lc,
+          const small_vector<reference_wrapper<const string>, 2>&,
+          lflags,
+          bool)
         {
           // It's unfortunate we have no way to bail out.
           //
diff --git a/libbuild2/cc/link-rule.cxx b/libbuild2/cc/link-rule.cxx
index 0ab61b1..b4175d6 100644
--- a/libbuild2/cc/link-rule.cxx
+++ b/libbuild2/cc/link-rule.cxx
@@ -1558,10 +1558,11 @@ namespace build2
         return la;
       };
 
-      auto lib = [&d, this] (const target* const* lc,
-                             const string& p,
-                             lflags f,
-                             bool)
+      auto lib = [&d, this] (
+        const target* const* lc,
+        const small_vector<reference_wrapper<const string>, 2>& ns,
+        lflags f,
+        bool)
       {
         // Note: see also make_header_sidebuild().
 
@@ -1584,15 +1585,15 @@ namespace build2
         // From the process_libraries() semantics we know that this callback
         // is always called and always after the options callbacks.
         //
-        appended_library& al (l != nullptr
-                              ? d.ls.append (*l, d.args.size ())
-                              : d.ls.append (p, d.args.size ()));
+        appended_library* al (l != nullptr
+                              ? &d.ls.append (*l, d.args.size ())
+                              : d.ls.append (ns, d.args.size ()));
 
-        if (al.end != appended_library::npos) // Closed.
+        if (al != nullptr && al->end != appended_library::npos) // Closed.
         {
           // Hoist the elements corresponding to this library to the end.
           //
-          if (al.begin != al.end)
+          if (al->begin != al->end)
           {
             // Rotate to the left the subrange starting from the first element
             // of this library and until the end so that the element after the
@@ -1600,23 +1601,23 @@ namespace build2
             // subrange. We also need to adjust begin/end of libraries
             // affected by the rotation.
             //
-            rotate (d.args.begin () + al.begin,
-                    d.args.begin () + al.end,
+            rotate (d.args.begin () + al->begin,
+                    d.args.begin () + al->end,
                     d.args.end ());
 
-            size_t n (al.end - al.begin);
+            size_t n (al->end - al->begin);
 
             for (appended_library& al1: d.ls)
             {
-              if (al1.begin >= al.end)
+              if (al1.begin >= al->end)
               {
                 al1.begin -= n;
                 al1.end -= n;
               }
             }
 
-            al.end = d.args.size ();
-            al.begin = al.end - n;
+            al->end = d.args.size ();
+            al->begin = al->end - n;
           }
 
           return;
@@ -1628,7 +1629,10 @@ namespace build2
           // static library.
           //
           if (d.li.type != otype::a)
-            d.args.push_back (p);
+          {
+            for (const string& n: ns)
+              d.args.push_back (n);
+          }
         }
         else
         {
@@ -1742,7 +1746,8 @@ namespace build2
         }
 
       done:
-        al.end = d.args.size (); // Close.
+        if (al != nullptr)
+          al->end = d.args.size (); // Close.
       };
 
       auto opt = [&d, this] (const target& lt,
@@ -1813,17 +1818,21 @@ namespace build2
         return la;
       };
 
-      auto lib = [&d, this] (const target* const* lc,
-                             const string& p,
-                             lflags f,
-                             bool)
+      auto lib = [&d, this] (
+        const target* const* lc,
+        const small_vector<reference_wrapper<const string>, 2>& ns,
+        lflags f,
+        bool)
       {
         const file* l (lc != nullptr ? &(*lc)->as<file> () : nullptr);
 
         if (l == nullptr)
         {
           if (d.li.type != otype::a)
-            d.cs.append (p);
+          {
+            for (const string& n: ns)
+              d.cs.append (n);
+          }
         }
         else
         {
@@ -1939,10 +1948,11 @@ namespace build2
         bool               link;
       } d {ls, args, link};
 
-      auto lib = [&d, this] (const target* const* lc,
-                             const string& f,
-                             lflags,
-                             bool sys)
+      auto lib = [&d, this] (
+        const target* const* lc,
+        const small_vector<reference_wrapper<const string>, 2>& ns,
+        lflags,
+        bool sys)
       {
         const file* l (lc != nullptr ? &(*lc)->as<file> () : nullptr);
 
@@ -1952,6 +1962,17 @@ namespace build2
         if (sys)
           return;
 
+        auto append = [&d] (const string& f)
+        {
+          string o (d.link ? "-Wl,-rpath-link," : "-Wl,-rpath,");
+
+          size_t p (path::traits_type::rfind_separator (f));
+          assert (p != string::npos);
+
+          o.append (f, 0, (p != 0 ? p : 1)); // Don't include trailing slash.
+          d.args.push_back (move (o));
+        };
+
         if (l != nullptr)
         {
           if (!l->is_a<libs> ())
@@ -1965,10 +1986,11 @@ namespace build2
           // We handle rpath similar to the compilation case by adding the
           // options on the first occurrence and ignoring all the subsequent.
           //
-          if (find (d.ls.begin (), d.ls.end (), l) != d.ls.end ())
-            return;
-
-          d.ls.push_back (l);
+          if (find (d.ls.begin (), d.ls.end (), l) == d.ls.end ())
+          {
+            append (ns[0]);
+            d.ls.push_back (l);
+          }
         }
         else
         {
@@ -1977,36 +1999,28 @@ namespace build2
           // better than checking for a platform-specific extension (maybe
           // we should cache it somewhere).
           //
-          size_t p (path::traits_type::find_extension (f));
+          for (const string& f: ns)
+          {
+            size_t p (path::traits_type::find_extension (f));
 
-          if (p == string::npos)
-            return;
+            if (p == string::npos)
+              return;
 
-          ++p; // Skip dot.
+            ++p; // Skip dot.
 
-          bool c (true);
-          const char* e;
+            bool c (true);
+            const char* e;
 
-          if      (tclass == "windows") {e = "dll"; c = false;}
-          else if (tsys == "darwin")     e = "dylib";
-          else                           e = "so";
+            if      (tclass == "windows") {e = "dll"; c = false;}
+            else if (tsys == "darwin")     e = "dylib";
+            else                           e = "so";
 
-          if ((c
-               ? f.compare (p, string::npos, e)
-               : icasecmp (f.c_str () + p, e)) != 0)
-            return;
+            if ((c
+                 ? f.compare (p, string::npos, e)
+                 : icasecmp (f.c_str () + p, e)) == 0)
+              append (f);
+          }
         }
-
-        // Ok, if we are here then it means we have a non-system, shared
-        // library and its absolute path is in f.
-        //
-        string o (d.link ? "-Wl,-rpath-link," : "-Wl,-rpath,");
-
-        size_t p (path::traits_type::rfind_separator (f));
-        assert (p != string::npos);
-
-        o.append (f, 0, (p != 0 ? p : 1)); // Don't include trailing slash.
-        d.args.push_back (move (o));
       };
 
 
diff --git a/libbuild2/cc/link-rule.hxx b/libbuild2/cc/link-rule.hxx
index baccf8d..33a4f1c 100644
--- a/libbuild2/cc/link-rule.hxx
+++ b/libbuild2/cc/link-rule.hxx
@@ -58,8 +58,14 @@ namespace build2
       {
         static const size_t npos = size_t (~0);
 
-        uintptr_t l;   // Pointer to library target (last bit 0) or to
-                       // library name (-lpthread or path; last bit 1).
+        // Each appended_library represents either a library target or a
+        // library name fragment up to 2 elements long:
+        //
+        //                          target         |         name
+        //                --------------------------------------------------
+        const void* l1; //      library target     | library name[1] or NULL
+        const void* l2; //           NULL          | library name[0]
+
         size_t begin;  // First arg belonging to this library.
         size_t end;    // Past last arg belonging to this library.
       };
@@ -73,38 +79,52 @@ namespace build2
         appended_library&
         append (const file& l, size_t b)
         {
-          auto p (reinterpret_cast<uintptr_t> (&l));
           auto i (find_if (begin (), end (),
-                           [p] (const appended_library& al)
+                           [&l] (const appended_library& al)
                            {
-                             return al.l == p;
+                             return al.l2 == nullptr && al.l1 == &l;
                            }));
 
           if (i != end ())
             return *i;
 
-          push_back (appended_library {p, b, appended_library::npos});
+          push_back (appended_library {&l, nullptr, b, appended_library::npos});
           return back ();
         }
 
-        appended_library&
-        append (const string& l, size_t b)
+        // Return NULL if no duplicate tracking can be performed for this
+        // library.
+        //
+        appended_library*
+        append (const small_vector<reference_wrapper<const string>, 2>& ns,
+                size_t b)
         {
-          auto i (find_if (begin (), end (),
-                           [&l] (const appended_library& al)
-                           {
-                             return (al.l & 1) != 0 &&
-                               l == *reinterpret_cast<const string*> (
-                                 al.l & ~uintptr_t (1));
-                           }));
+          size_t n (ns.size ());
+
+          if (n > 2)
+            return nullptr;
+
+          auto i (
+            find_if (
+              begin (), end (),
+              [&ns, n] (const appended_library& al)
+              {
+                return al.l2 != nullptr &&
+                  *static_cast<const string*> (al.l2) == ns[0].get () &&
+                  (n == 2
+                   ? (al.l1 != nullptr &&
+                      *static_cast<const string*> (al.l1) == ns[1].get ())
+                   : al.l1 == nullptr);
+              }));
 
           if (i != end ())
-            return *i;
+            return &*i;
 
-          push_back (
-            appended_library {
-              reinterpret_cast<uintptr_t> (&l) | 1, b, appended_library::npos});
-          return back ();
+          push_back (appended_library {
+              n == 2 ? &ns[1].get () : nullptr, &ns[0].get (),
+              b, appended_library::npos});
+
+          return &back ();
         }
       };
 
diff --git a/libbuild2/cc/pkgconfig.cxx b/libbuild2/cc/pkgconfig.cxx
index d44b0ec..671a51a 100644
--- a/libbuild2/cc/pkgconfig.cxx
+++ b/libbuild2/cc/pkgconfig.cxx
@@ -1635,11 +1635,11 @@ namespace build2
           bool priv (false);
           auto imp = [&priv] (const target&, bool la) {return priv && la;};
 
-          auto lib = [&save_library_target,
-                      &save_library_name] (const target* const* lc,
-                                           const string& p,
-                                           lflags,
-                                           bool)
+          auto lib = [&save_library_target, &save_library_name] (
+            const target* const* lc,
+            const small_vector<reference_wrapper<const string>, 2>& ns,
+            lflags,
+            bool)
           {
             const file* l (lc != nullptr ? &(*lc)->as<file> () : nullptr);
 
@@ -1649,7 +1649,12 @@ namespace build2
                 save_library_target (*l);
             }
             else
-              save_library_name (p); // Something "system'y", save as is.
+            {
+              // Something "system'y", save as is.
+              //
+              for (const string& n: ns)
+                save_library_name (n);
+            }
           };
 
           auto opt = [] (const target&, const string&, bool, bool)
diff --git a/libbuild2/cc/windows-rpath.cxx b/libbuild2/cc/windows-rpath.cxx
index 70eef73..7b572df 100644
--- a/libbuild2/cc/windows-rpath.cxx
+++ b/libbuild2/cc/windows-rpath.cxx
@@ -58,10 +58,11 @@ namespace build2
       //
       auto imp = [] (const target&, bool) {return true;};
 
-      auto lib = [&r] (const target* const* lc,
-                       const string& f,
-                       lflags,
-                       bool sys)
+      auto lib = [&r] (
+        const target* const* lc,
+        const small_vector<reference_wrapper<const string>, 2>& ns,
+        lflags,
+        bool sys)
       {
         const file* l (lc != nullptr ? &(*lc)->as<file> () : nullptr);
 
@@ -76,8 +77,13 @@ namespace build2
         {
           // This can be an "undiscovered" DLL (see search_library()).
           //
-          if (!l->is_a<libs> () || l->path ().empty ()) // Also covers binless.
-            return;
+          if (l->is_a<libs> () && !l->path ().empty ()) // Also covers binless.
+          {
+            timestamp t (l->load_mtime ());
+
+            if (t > r)
+              r = t;
+          }
         }
         else
         {
@@ -91,20 +97,19 @@ namespace build2
           //
           //    Though this can happen on MinGW with direct DLL link...
           //
-          size_t p (path::traits_type::find_extension (f));
-
-          if (p == string::npos || icasecmp (f.c_str () + p + 1, "dll") != 0)
-            return;
-        }
+          for (const string& f: ns)
+          {
+            size_t p (path::traits_type::find_extension (f));
 
-        // Ok, this is a DLL.
-        //
-        timestamp t (l != nullptr
-                     ? l->load_mtime ()
-                     : mtime (f.c_str ()));
+            if (p != string::npos && icasecmp (f.c_str () + p + 1, "dll") == 0)
+            {
+              timestamp t (mtime (f.c_str ()));
 
-        if (t > r)
-          r = t;
+              if (t > r)
+                r = t;
+            }
+          }
+        }
       };
 
       for (const prerequisite_target& pt: t.prerequisite_targets[a])
@@ -139,10 +144,11 @@ namespace build2
 
       auto imp = [] (const target&, bool) {return true;};
 
-      auto lib = [&r, &bs] (const target* const* lc,
-                            const string& f,
-                            lflags,
-                            bool sys)
+      auto lib = [&r, &bs] (
+        const target* const* lc,
+        const small_vector<reference_wrapper<const string>, 2>& ns,
+        lflags,
+        bool sys)
       {
         const file* l (lc != nullptr ? &(*lc)->as<file> () : nullptr);
 
@@ -161,7 +167,7 @@ namespace build2
                                : nullptr);
             r.insert (
               windows_dll {
-                f,
+                ns[0],
                 pdb != nullptr ? &pdb->as<file> ().path ().string () : nullptr,
                 string ()
               });
@@ -169,35 +175,38 @@ namespace build2
         }
         else
         {
-          size_t p (path::traits_type::find_extension (f));
-
-          if (p != string::npos && icasecmp (f.c_str () + p + 1, "dll") == 0)
+          for (const string& f: ns)
           {
-            // See if we can find a corresponding .pdb.
-            //
-            windows_dll wd {f, nullptr, string ()};
-            string& pdb (wd.pdb_storage);
-
-            // First try "our" naming: foo.dll.pdb.
-            //
-            pdb = f;
-            pdb += ".pdb";
+            size_t p (path::traits_type::find_extension (f));
 
-            if (!exists (path (pdb)))
+            if (p != string::npos && icasecmp (f.c_str () + p + 1, "dll") == 0)
             {
-              // Then try the usual naming: foo.pdb.
+              // See if we can find a corresponding .pdb.
               //
-              pdb.assign (f, 0, p);
+              windows_dll wd {f, nullptr, string ()};
+              string& pdb (wd.pdb_storage);
+
+              // First try "our" naming: foo.dll.pdb.
+              //
+              pdb = f;
               pdb += ".pdb";
 
               if (!exists (path (pdb)))
-                pdb.clear ();
-            }
+              {
+                // Then try the usual naming: foo.pdb.
+                //
+                pdb.assign (f, 0, p);
+                pdb += ".pdb";
 
-            if (!pdb.empty ())
-              wd.pdb = &pdb;
+                if (!exists (path (pdb)))
+                  pdb.clear ();
+              }
 
-            r.insert (move (wd));
+              if (!pdb.empty ())
+                wd.pdb = &pdb;
+
+              r.insert (move (wd));
+            }
           }
         }
       };