aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBoris Kolpackov <boris@codesynthesis.com>2019-06-26 12:28:05 +0200
committerBoris Kolpackov <boris@codesynthesis.com>2019-06-26 12:28:05 +0200
commitd1fbcace59846d55c66e741dbc3d59e20ae3e5cf (patch)
treeaf1325d7d519bda64535f602c8ea1a3147b97b31
parentdc3d7c5fa4062cac36d718766504c87696a3de41 (diff)
Fix C/C++ link rule matching ambiguity by seeing-through utility libraries
-rw-r--r--build2/cc/common.cxx2
-rw-r--r--build2/cc/compile-rule.cxx10
-rw-r--r--build2/cc/install-rule.cxx4
-rw-r--r--build2/cc/link-rule.cxx147
-rw-r--r--build2/cc/link-rule.hxx12
-rw-r--r--build2/cc/utility.cxx14
-rw-r--r--build2/cc/utility.hxx7
-rw-r--r--build2/prerequisite.hxx8
8 files changed, 159 insertions, 45 deletions
diff --git a/build2/cc/common.cxx b/build2/cc/common.cxx
index b4b0a6d..fa774d7 100644
--- a/build2/cc/common.cxx
+++ b/build2/cc/common.cxx
@@ -485,7 +485,7 @@ namespace build2
// If this is lib{}/libu*{}, pick appropriate member.
//
if (const libx* l = xt->is_a<libx> ())
- xt = &link_member (*l, a, li); // Pick lib*{e,a,s}{}.
+ xt = link_member (*l, a, li); // Pick lib*{e,a,s}{}.
return xt->as<file> ();
}
diff --git a/build2/cc/compile-rule.cxx b/build2/cc/compile-rule.cxx
index 1ea3800..a1796eb 100644
--- a/build2/cc/compile-rule.cxx
+++ b/build2/cc/compile-rule.cxx
@@ -384,7 +384,7 @@ namespace build2
if (const target* pt = p.load ())
{
if (const libx* l = pt->is_a<libx> ())
- pt = &link_member (*l, a, li);
+ pt = link_member (*l, a, li);
bool la;
if (!((la = pt->is_a<liba> ()) ||
@@ -435,7 +435,7 @@ namespace build2
if (const target* pt = p.load ())
{
if (const libx* l = pt->is_a<libx> ())
- pt = &link_member (*l, a, li);
+ pt = link_member (*l, a, li);
bool la;
if (!((la = pt->is_a<liba> ()) ||
@@ -489,7 +489,7 @@ namespace build2
if (const target* pt = p.load ())
{
if (const libx* l = pt->is_a<libx> ())
- pt = &link_member (*l, a, li);
+ pt = link_member (*l, a, li);
bool la;
if (!((la = pt->is_a<liba> ()) ||
@@ -741,7 +741,7 @@ namespace build2
pt = &p.search (t);
if (const libx* l = pt->is_a<libx> ())
- pt = &link_member (*l, a, li);
+ pt = link_member (*l, a, li);
}
else
continue;
@@ -4713,7 +4713,7 @@ namespace build2
const target* lt (nullptr);
if (const libx* l = pt->is_a<libx> ())
- lt = &link_member (*l, a, li);
+ lt = link_member (*l, a, li);
else if (pt->is_a<liba> () || pt->is_a<libs> () || pt->is_a<libux> ())
lt = pt;
diff --git a/build2/cc/install-rule.cxx b/build2/cc/install-rule.cxx
index 00e8a63..5f8722b 100644
--- a/build2/cc/install-rule.cxx
+++ b/build2/cc/install-rule.cxx
@@ -57,7 +57,7 @@ namespace build2
// link. For libu*{} we want the "see through" logic.
//
if (const libx* l = pt->is_a<libx> ())
- pt = &link_member (*l, a, link_info (t.base_scope (), ot));
+ pt = link_member (*l, a, link_info (t.base_scope (), ot));
// Note: not redundant since we are returning a member.
//
@@ -285,7 +285,7 @@ namespace build2
const target* pt (&search (t, p));
if (const libx* l = pt->is_a<libx> ())
- pt = &link_member (*l, a, link_info (t.base_scope (), ot));
+ pt = link_member (*l, a, link_info (t.base_scope (), ot));
if ((st && pt->is_a<libs> ()) || (at && pt->is_a<liba> ()))
return pt->in (t.weak_scope ()) ? pt : nullptr;
diff --git a/build2/cc/link-rule.cxx b/build2/cc/link-rule.cxx
index 588f3ae..8146ee4 100644
--- a/build2/cc/link-rule.cxx
+++ b/build2/cc/link-rule.cxx
@@ -43,15 +43,13 @@ namespace build2
"insufficient space");
}
- bool link_rule::
- match (action a, target& t, const string& hint) const
+ link_rule::match_result link_rule::
+ match (action a, target& t, ltype lt, bool linkup) const
{
- tracer trace (x, "link_rule::match");
+ // NOTE: the target may be a group (see utility library logic below).
- // NOTE: may be called multiple times and for both inner and outer
- // operations (see install rules).
+ match_result r;
- ltype lt (link_type (t));
otype ot (lt.type);
// If this is a library, link-up to our group (this is the target group
@@ -60,12 +58,14 @@ namespace build2
// If we are called for the outer operation (see install rules), then
// use resolve_group() to delegate to inner.
//
- if (lt.library ())
+ if (linkup)
{
if (a.outer ())
resolve_group (a, t);
else if (t.group == nullptr)
t.group = &search (t,
+ // @@ Why are we picking libu{} over libul{}?
+ //
lt.utility ? libu::static_type : lib::static_type,
t.dir, t.out, t.name);
}
@@ -76,8 +76,6 @@ namespace build2
//
// Note also that we treat bmi{} as obj{}. @@ MODHDR hbmi{}?
//
- bool seen_x (false), seen_c (false), seen_obj (false), seen_lib (false);
-
for (prerequisite_member p: group_prerequisite_members (a, t))
{
// If excluded or ad hoc, then don't factor it into our tests.
@@ -90,59 +88,150 @@ namespace build2
// Header-only X library (or library with C source and X header).
(lt.library () && x_header (p, false /* c_hdr */)))
{
- seen_x = seen_x || true;
+ r.seen_x = r.seen_x || true;
}
else if (p.is_a<c> () ||
// Header-only C library.
(lt.library () && p.is_a<h> ()))
{
- seen_c = seen_c || true;
+ r.seen_c = r.seen_c || true;
}
else if (p.is_a<obj> () || p.is_a<bmi> ())
{
- seen_obj = seen_obj || true;
+ r.seen_obj = r.seen_obj || true;
}
else if (p.is_a<obje> () || p.is_a<bmie> ())
{
+ // We can make these "no-match" if/when there is a valid use case.
+ //
if (ot != otype::e)
fail << p.type ().name << "{} as prerequisite of " << t;
- seen_obj = seen_obj || true;
+ r.seen_obj = r.seen_obj || true;
}
else if (p.is_a<obja> () || p.is_a<bmia> ())
{
if (ot != otype::a)
fail << p.type ().name << "{} as prerequisite of " << t;
- seen_obj = seen_obj || true;
+ r.seen_obj = r.seen_obj || true;
}
else if (p.is_a<objs> () || p.is_a<bmis> ())
{
if (ot != otype::s)
fail << p.type ().name << "{} as prerequisite of " << t;
- seen_obj = seen_obj || true;
+ r.seen_obj = r.seen_obj || true;
}
- else if (p.is_a<libx> () ||
- p.is_a<liba> () ||
- p.is_a<libs> () ||
+ else if (p.is_a<libu> () ||
+ p.is_a<libul> () ||
p.is_a<libux> ())
{
- seen_lib = seen_lib || true;
+ // For a unility library we look at its prerequisites, recursively.
+ // Since these checks are not exactly light-weight, only do them if
+ // we haven't already seen any X prerequisites.
+ //
+ if (!r.seen_x)
+ {
+ // This is a bit iffy: in our model a rule can only search a
+ // target's prerequisites if it matches. But we don't yet know
+ // whether we match. However, it seems correct to assume that any
+ // rule-specific search will always resolve to an existing target
+ // if there is one. So perhaps it's time to relax this restriction
+ // a little?
+ //
+ const target* pt (p.search_existing ());
+
+ // It's possible we have no group but have a member so try that.
+ //
+ if (pt == nullptr && p.is_a<libx> ())
+ {
+ // We know this prerequisite member is a prerequisite since
+ // otherwise the above search would have returned the member
+ // target.
+ //
+ const target_type& tt (ot == otype::a ? libua::static_type :
+ ot == otype::s ? libus::static_type :
+ libue::static_type);
+
+ pt = search_existing (p.prerequisite.key (tt));
+ }
+
+ if (pt != nullptr)
+ {
+ // If this is a group then try to pick (again, if exists) a
+ // suitable member. If it doesn't exist, then we will only be
+ // considering the group's prerequisites.
+ //
+ if (const libx* gt = pt->is_a<libx> ())
+ {
+ linfo li {ot, lorder::a /* unused */};
+ if (const target* pm = link_member (*gt, a, li, true))
+ pt = pm;
+ }
+
+ //@@ TMP: const_cast: try to make target in match() const.
+
+ bool group (pt->is_a<libx> ());
+ match_result lr (match (a,
+ const_cast<target&> (*pt),
+ (group
+ ? ltype {ot , true /* utility */}
+ : link_type (*pt)),
+ !group /* linkup */));
+
+ // Do we need to propagate any other seen_* values? Hm, that
+ // would in fact match with the "see-through" semantics of
+ // utility libraries we have in other places.
+ //
+ r.seen_x = lr.seen_x;
+ }
+ else
+ r.seen_lib = r.seen_lib || true; // Consider as just a library.
+ }
+ }
+ else if (p.is_a<lib> () ||
+ p.is_a<liba> () ||
+ p.is_a<libs> ())
+ {
+ r.seen_lib = r.seen_lib || true;
}
- // If this is some other c-common header/source (say C++ in a C rule),
- // then we shouldn't try to handle that (it may need to be compiled,
- // etc). But we assume everyone can handle a C header.
+ // Some other c-common header/source (say C++ in a C rule) other than
+ // a C header (we assume everyone can hanle that).
//
else if (p.is_a<cc> () && !(x_header (p, true /* c_hdr */)))
{
- l4 ([&]{trace << "non-" << x_lang << " prerequisite " << p
- << " for target " << t;});
- return false;
+ r.seen_cc = true;
+ break;
}
}
- if (!(seen_x || seen_c || seen_obj || seen_lib))
+ return r;
+ }
+
+ bool link_rule::
+ match (action a, target& t, const string& hint) const
+ {
+ // NOTE: may be called multiple times and for both inner and outer
+ // operations (see the install rules).
+
+ tracer trace (x, "link_rule::match");
+
+ ltype lt (link_type (t));
+ match_result r (match (a, t, lt, lt.library ()));
+
+ // If this is some other c-common header/source (say C++ in a C rule),
+ // then we shouldn't try to handle that (it may need to be compiled,
+ // etc).
+ //
+ if (r.seen_cc)
+ {
+ l4 ([&]{trace << "non-" << x_lang << " prerequisite "
+ << "for target " << t;});
+ return false;
+ }
+
+ if (!(r.seen_x || r.seen_c || r.seen_obj || r.seen_lib))
{
l4 ([&]{trace << "no " << x_lang << ", C, or obj/lib prerequisite "
<< "for target " << t;});
@@ -152,7 +241,7 @@ namespace build2
// We will only chain a C source if there is also an X source or we were
// explicitly told to.
//
- if (seen_c && !seen_x && hint < x)
+ if (r.seen_c && !r.seen_x && hint < x)
{
l4 ([&]{trace << "C prerequisite without " << x_lang << " or hint "
<< "for target " << t;});
@@ -333,7 +422,7 @@ namespace build2
if ((ul = pt->is_a<libul> ()) ||
(ul = pt->is_a<libu> ()))
{
- pf = &link_member (*ul, a, li).as<file> ();
+ pf = &link_member (*ul, a, li)->as<file> ();
}
else if ((pf = pt->is_a<libue> ()) ||
(pf = pt->is_a<libus> ()) ||
@@ -536,7 +625,7 @@ namespace build2
// member.
//
if (const libx* l = pt->is_a<libx> ())
- pt = &link_member (*l, a, li);
+ pt = link_member (*l, a, li);
}
else
{
diff --git a/build2/cc/link-rule.hxx b/build2/cc/link-rule.hxx
index 1a77aef..2df22cf 100644
--- a/build2/cc/link-rule.hxx
+++ b/build2/cc/link-rule.hxx
@@ -24,6 +24,18 @@ namespace build2
public:
link_rule (data&&);
+ struct match_result
+ {
+ bool seen_x = false;
+ bool seen_c = false;
+ bool seen_cc = false;
+ bool seen_obj = false;
+ bool seen_lib = false;
+ };
+
+ match_result
+ match (action, target&, ltype, bool) const;
+
virtual bool
match (action, target&, const string&) const override;
diff --git a/build2/cc/utility.cxx b/build2/cc/utility.cxx
index 39f5c35..2751032 100644
--- a/build2/cc/utility.cxx
+++ b/build2/cc/utility.cxx
@@ -43,8 +43,8 @@ namespace build2
: v.size () > 1 && v[1] == "shared" ? lorder::a_s : lorder::a;
}
- const target&
- link_member (const bin::libx& x, action a, linfo li)
+ const target*
+ link_member (const bin::libx& x, action a, linfo li, bool exist)
{
bool ul;
@@ -72,12 +72,14 @@ namespace build2
// Called by the compile rule during execute.
//
- return phase == run_phase::match
- ? search (x, tt, x.dir, x.out, x.name)
- : *search_existing (tt, x.dir, x.out, x.name);
+ return phase == run_phase::match && !exist
+ ? &search (x, tt, x.dir, x.out, x.name)
+ : search_existing (tt, x.dir, x.out, x.name);
}
else
{
+ assert (!exist);
+
const lib& l (x.as<lib> ());
// Make sure group members are resolved.
@@ -107,7 +109,7 @@ namespace build2
}
}
- return *(ls ? static_cast<const target*> (l.s) : l.a);
+ return ls ? static_cast<const target*> (l.s) : l.a;
}
}
}
diff --git a/build2/cc/utility.hxx b/build2/cc/utility.hxx
index 82d9f71..bc6c3fb 100644
--- a/build2/cc/utility.hxx
+++ b/build2/cc/utility.hxx
@@ -60,8 +60,11 @@ namespace build2
// Given the link order return the library member to link. That is, liba{}
// or libs{} for lib{} and libue{}, libua{} or libus{} for libu*{}.
//
- const target&
- link_member (const bin::libx&, action, linfo);
+ // If existing is true, then only return the member target if it exists
+ // (currently only used and supported for utility libraries).
+ //
+ const target*
+ link_member (const bin::libx&, action, linfo, bool existing = false);
}
}
diff --git a/build2/prerequisite.hxx b/build2/prerequisite.hxx
index 551751b..258033d 100644
--- a/build2/prerequisite.hxx
+++ b/build2/prerequisite.hxx
@@ -131,6 +131,14 @@ namespace build2
return prerequisite_key {proj, {&type, &dir, &out, &name, ext}, &scope};
}
+ // As above but remap the target type to the specified.
+ //
+ prerequisite_key
+ key (const target_type_type& tt) const
+ {
+ return prerequisite_key {proj, {&tt, &dir, &out, &name, ext}, &scope};
+ }
+
// Return true if this prerequisite instance (physically) belongs to the
// target's prerequisite list. Note that this test only works if you use
// references to the container elements and the container hasn't been