From d53c53ee45fa0a0e32d59354c81b26ea530edaac Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Sat, 5 Aug 2017 16:48:26 +0200 Subject: Implement src-to-out re-mapping for generated headers --- build2/cc/compile.cxx | 276 +++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 218 insertions(+), 58 deletions(-) diff --git a/build2/cc/compile.cxx b/build2/cc/compile.cxx index 22b3b89..79d40c3 100644 --- a/build2/cc/compile.cxx +++ b/build2/cc/compile.cxx @@ -5,6 +5,8 @@ #include #include // exit() +#include // strlen() + #include #include #include @@ -1467,6 +1469,49 @@ namespace build2 // bool sense_diag (false); + // And here is another problem: if we have an already generated header + // in src and the one in out does not yet exist, then the compiler will + // pick the one in src and we won't even notice. Note that this is not + // only an issue with mixing in- and out-of-tree builds (which does feel + // wrong but is oh so convenient): this is also a problem with + // pre-generated headers, a technique we use to make installing the + // generator by end-users optional by shipping pre-generated headers. + // + // This is a nasty problem that doesn't seem to have a perfect solution + // (except, perhaps, C++ modules). So what we are going to do is try to + // rectify the situation by detecting and automatically re-mapping such + // mis-inclusions. It works as follows. + // + // First we will build a map of src/out pairs that were specified with + // -I. Here, for performance and simplicity, we will assume that they + // always come in pairs with out first and src second. We build this + // map lazily only if we are running the preprocessor and reuse it + // between restarts. + // + // With the map in hand we can then check each included header for + // potentially having a doppelganger in the out tree. If this is the + // case, then we calculate a corresponding header in the out tree and, + // (this is the most important part), check if there is a target for + // this header in the out tree. This should be fairly accurate and not + // require anything explicit from the user except perhaps for a case + // where the header is generated out of nothing (so there is no need to + // explicitly mention its target in the buildfile). But this probably + // won't be very common. + // + // One tricky area in this setup are target groups: if the generated + // sources are mentioned in the buildfile as a group, then there might + // be no header target (yet). The way we solve this is by requiring code + // generator rules to cooperate and create at least the header target as + // part of the group creation. While not all members of the group may be + // generated depending on the options (e.g., inline files might be + // suppressed), headers are usually non-optional. + // + // Note that we use path_map instead of dir_path_map to allow searching + // using path (file path). + // + using srcout_map = path_map; + srcout_map so_map; + // The gen argument to init_args() is in/out. The caller signals whether // to force the generated header support and on return it signals // whether this support is enabled. The first call to init_args is @@ -1478,12 +1523,13 @@ namespace build2 auto init_args = [&t, act, li, &src, &md, &psrc, &sense_diag, &rs, &bs, - pp, &env, &args, &args_gen, &args_i, &out, &drm, this] + pp, &env, &args, &args_gen, &args_i, &out, &drm, + &so_map, this] (bool& gen) -> const path* { const path* r (nullptr); - if (args.empty ()) + if (args.empty ()) // First call. { assert (!gen); @@ -1528,6 +1574,107 @@ namespace build2 append_options (args, t, c_poptions); append_options (args, t, x_poptions); + // Populate the src-out with the -I$out_base -I$src_base pairs. + // + { + // Try to be fast and efficient by reusing buffers as much as + // possible. + // + string ds; + + // Previous -I innermost scope if out_base plus the difference + // between the scope path and the -I path (normally empty). + // + const scope* s (nullptr); + dir_path p; + + for (auto i (args.begin ()), e (args.end ()); i != e; ++i) + { + // -I can either be in the "-Ifoo" or "-I foo" form. For VC it + // can also be /I. + // + const char* o (*i); + size_t n (strlen (o)); + + if (n < 2 || (o[0] != '-' && o[0] != '/') || o[1] != 'I') + { + s = nullptr; + continue; + } + + if (n == 2) + { + if (++i == e) + break; // Let the compiler complain. + + ds = *i; + } + else + ds.assign (o + 2, n - 2); + + // Note that we don't normalize the paths since it would be + // quite expensive and normally the pairs we are inerested in + // are already normalized (since usually specified as + // -I$src/out_*). + // + // @@ Maybe we should ignore any paths containing '.', '..' + // components for safety. + // + dir_path d (move (ds)); // Move the buffer in. + + if (d.absolute ()) + { + // If we have a candidate out_base, see if this is its + // src_base. + // + if (s != nullptr) + { + const dir_path& bp (s->src_path ()); + + if (d.sub (bp)) + { + if (p.empty () || d.leaf (bp) == p) + { + // We've got a pair. + // + so_map.emplace (move (d), s->out_path () / p); + continue; + } + } + + // Not a pair. Fall through to consider as out_base. + // + s = nullptr; + } + + // See if this path is inside a project with an out-of-tree + // build and is in the out directory tree. + // + const scope& bs (scopes.find (d)); + if (bs.root_scope () != nullptr) + { + const dir_path& bp (bs.out_path ()); + if (bp != bs.src_path ()) + { + bool e; + if ((e = (d == bp)) || d.sub (bp)) + { + s = &bs; + if (e) + p.clear (); + else + p = d.leaf (bp); + } + } + } + } + else + s = nullptr; + + ds = move (d).string (); // Move the buffer out. + } + } + // Extra system header dirs (last). // for (const dir_path& d: sys_inc_dirs) @@ -1721,7 +1868,7 @@ namespace build2 // Build the prefix map lazily only if we have non-existent files. // Also reuse it over restarts since it doesn't change. // - prefix_map pm; + optional pfx_map; // If any prerequisites that we have extracted changed, then we have to // redo the whole thing. The reason for this is auto-generated headers: @@ -1760,27 +1907,30 @@ namespace build2 // from the depdb cache or from the compiler run. Return whether the // extraction process should be restarted. // - auto add = [&trace, &pm, + auto add = [&trace, &pfx_map, &so_map, act, &t, li, &dd, &updating, mt, - &bs, this] (path f, bool cache) -> bool + &bs, this] + (path f, bool cache) -> bool { - // Find or maybe insert the target. + // Find or maybe insert the target. The directory is only moved + // from if insert is true. // - auto find = [&trace, &t, this] ( - const path& f, bool insert) -> const path_target* + auto find = [&trace, &t, this] + (dir_path&& d, path&& f, bool insert) -> const path_target* { - // Split the name into its directory part, the name part, and - // extension. Here we can assume the name part is a valid filesystem - // name. + // Split the file into its name part and extension. Here we can + // assume the name part is a valid filesystem name. // // Note that if the file has no extension, we record an empty // extension rather than NULL (which would signify that the default // extension should be added). // - dir_path d (f.directory ()); - string n (f.leaf ().base ().string ()); string e (f.extension ()); + string n (move (f).string ()); + + if (!e.empty ()) + n.resize (n.size () - e.size () - 1); // One for the dot. // Determine the target type. // @@ -1841,8 +1991,6 @@ namespace build2 return static_cast (r); }; - const path_target* pt (nullptr); - // If it's not absolute then it either does not (yet) exist or is // a relative ""-include (see init_args() for details). Reduce the // second case to absolute. @@ -1876,6 +2024,8 @@ namespace build2 } #endif + const path_target* pt (nullptr); + // If still relative then it does not exist. // if (f.relative ()) @@ -1887,58 +2037,38 @@ namespace build2 // l4 ([&]{trace << "non-existent header '" << f << "'";}); - // If we already did this and build_prefix_map() returned empty, - // then we would have failed below. - // - if (pm.empty ()) - pm = build_prefix_map (bs, t, act, li); + if (!pfx_map) + pfx_map = build_prefix_map (bs, t, act, li); // First try the whole file. Then just the directory. // // @@ Has to be a separate map since the prefix can be // the same as the file name. // - // auto i (pm.find (f)); + // auto i (pfx_map->find (f)); // Find the most qualified prefix of which we are a sub-path. // - auto i (pm.end ()); - - if (!pm.empty ()) + if (!pfx_map->empty ()) { - const dir_path& d (f.directory ()); - i = pm.upper_bound (d); + dir_path d (f.directory ()); + auto i (pfx_map->find_sub (d)); - // Get the greatest less than, if any. We might still not be a - // sub. Note also that we still have to check the last element if - // upper_bound() returned end(). - // - if (i == pm.begin () || !d.sub ((--i)->first)) - i = pm.end (); - } - - if (i != pm.end ()) - { - // If this is a prefixless mapping, then only use it if we can - // resolve it to an existing target (i.e., it is explicitly - // spelled out in a buildfile). - // - // Note that at some point we will probably have a list of - // directories. - // - if (i->first.empty ()) + if (i != pfx_map->end ()) { - path p (i->second / f); - l4 ([&]{trace << "trying as auto-generated " << p;}); - pt = find (p, false); + // If this is a prefixless mapping, then only use it if we can + // resolve it to an existing target (i.e., it is explicitly + // spelled out in a buildfile). + // + // Note that at some point we will probably have a list of + // directories. + // + pt = find (i->second / d, f.leaf (), !i->first.empty ()); if (pt != nullptr) - f = move (p); - } - else - { - f = i->second / f; - l4 ([&]{trace << "mapped as auto-generated " << f;}); - pt = find (f, true); + { + f = i->second / f; + l4 ([&]{trace << "mapped as auto-generated " << f;}); + } } } @@ -1954,14 +2084,44 @@ namespace build2 { // We used to just normalize the path but that could result in an // invalid path (e.g., on CentOS 7 with Clang 3.4) because of the - // symlinks. So now we realize (i.e., realpath(3)) it instead. If - // it comes from the depdb, in which case we've already done that. + // symlinks. So now we realize (i.e., realpath(3)) it instead. + // Unless it comes from the depdb, in which case we've already done + // that. This is also where we handle src-out remap (again, not + // needed if cached) // if (!cache) + { f.realize (); - l6 ([&]{trace << "injecting " << f;}); - pt = find (f, true); + if (!so_map.empty ()) + { + // Find the most qualified prefix of which we are a sub-path. + // + auto i (so_map.find_sub (f)); + if (i != so_map.end ()) + { + // Ok, there is an out tree for this headers. Remap to a path + // from the out tree and see if there is a target for it. + // + dir_path d (i->second); + d /= f.leaf (i->first).directory (); + pt = find (move (d), f.leaf (), false); // d is not moved from. + + if (pt != nullptr) + { + path p (d / f.leaf ()); + l4 ([&]{trace << "re-mapping " << f << " to " << p;}); + f = move (p); + } + } + } + } + + if (pt == nullptr) + { + l6 ([&]{trace << "injecting " << f;}); + pt = find (f.directory (), f.leaf (), true); + } } // Cache the path. -- cgit v1.1