From e188fbebff1d4ba01ec906867e2d7ef0824b01f8 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Tue, 1 Dec 2020 07:28:20 +0200 Subject: Modules-related refactoring --- libbuild2/cc/compile-rule.cxx | 92 +++++++++++++++++++++---------------------- libbuild2/cc/compile-rule.hxx | 7 ++-- libbuild2/cc/parser.cxx | 7 +--- libbuild2/cc/parser.hxx | 10 ++++- 4 files changed, 60 insertions(+), 56 deletions(-) diff --git a/libbuild2/cc/compile-rule.cxx b/libbuild2/cc/compile-rule.cxx index 42427d9..eba58fd 100644 --- a/libbuild2/cc/compile-rule.cxx +++ b/libbuild2/cc/compile-rule.cxx @@ -92,11 +92,10 @@ namespace build2 return s; } - static pair - to_module_info (const string& s) + static unit_type + to_module_info (const string& s, module_info& mi) { unit_type ut (unit_type::non_modular); - module_info mi; for (size_t b (0), e (0), n (s.size ()), m; e < n; ) { @@ -154,7 +153,7 @@ namespace build2 e += (d == '"' ? 2 : 1); } - return pair (move (ut), move (mi)); + return ut; } // preprocessed @@ -191,7 +190,7 @@ namespace build2 prerequisite_member src; auto_rmfile psrc; // Preprocessed source, if any. path dd; // Dependency database path. - size_t headers = 0; // Number of imported header units. + size_t header_units = 0; // Number of imported header units. module_positions modules = {0, 0, 0}; // Positions of imported modules. }; @@ -1064,6 +1063,8 @@ namespace build2 << "for target " << src << ": " << e; } + unit tu; + // If we have no #include directives (or header unit imports), then // skip header dependency extraction. // @@ -1073,7 +1074,9 @@ namespace build2 // Note: trace is used in a test. // l5 ([&]{trace << "extracting headers from " << src;}); - psrc = extract_headers (a, bs, t, li, src, md, dd, u, mt); + auto& is (tu.module_info.imports); + psrc = extract_headers (a, bs, t, li, src, md, dd, u, mt, is); + is.clear (); // No longer needed. } // Next we "obtain" the translation unit information. What exactly @@ -1097,7 +1100,6 @@ namespace build2 else u = true; // Database is invalid, force re-parse. - unit tu; for (bool first (true);; first = false) { if (u) @@ -1108,18 +1110,19 @@ namespace build2 if (dd.writing ()) dd.flush (); - auto p (parse_unit (a, t, li, src, psrc.first, md, dd.path)); + string ncs ( + parse_unit (a, t, li, src, psrc.first, md, dd.path, tu)); - if (!cs || *cs != p.second) + if (!cs || *cs != ncs) { assert (first); // Unchanged TU has a different checksum? - dd.write (p.second); + dd.write (ncs); } // // Don't clear the update flag if it was forced or the checksum // should not be relied upon. // - else if (first && !p.second.empty ()) + else if (first && !ncs.empty ()) { // Clear the update flag and set the touch flag. Unless there // is no (usable) object file, of course. See also the md.mt @@ -1131,8 +1134,6 @@ namespace build2 md.touch = true; } } - - tu = move (p.first); } if (modules) @@ -1150,9 +1151,7 @@ namespace build2 { if (string* l = dd.read ()) { - auto p (to_module_info (*l)); - tu.type = p.first; - tu.module_info = move (p.second); + tu.type = to_module_info (*l, tu.module_info); } else { @@ -1801,13 +1800,14 @@ namespace build2 // struct compile_rule::module_mapper_state //@@ gcc_module_mapper_state { - size_t headers = 0; // Number of header units imported. - size_t skip; // Number of depdb entries to skip. + size_t skip; // Number of depdb entries to skip. + size_t header_units = 0; // Number of header units imported. + module_imports& imports; // Unused (potentially duplicate suppression). small_vector batch; // Reuse buffers. - explicit - module_mapper_state (size_t skip_count): skip (skip_count) {} + module_mapper_state (size_t s, module_imports& i) + : skip (s), imports (i) {} }; void compile_rule:: @@ -1959,6 +1959,15 @@ namespace build2 continue; } + // Note also that we may see multiple imports of the same header + // if it's imported via multiple paths. There is nothing really we + // can do about it since we have to have each path in the file + // mapper (see below for details). + // + // At least in case of GCC, we don't see multiple imports for the + // same path nor multiple inclusions, regardless of whether the + // header uses #pragma once or include guards. + // The skip_count logic: in a nutshell (and similar to the non- // mapper case), we may have "processed" some portion of the // headers based on the depdb cache and we need to avoid @@ -2123,21 +2132,9 @@ namespace build2 // original (which we may need to normalize when we read // this mapping in extract_headers()). // - // @@ Also note that we will see multiple imports of the - // same file potentially blowing up the mapping. Should - // we keep track and suppress duplicates (preferably - // at the outset so that we can skip all the target - // entering, etc)? - // - // Ideally, this should survive accross multiple compiler - // invocations so that we can avoid import that are - // before the skip count. Could we not reuse the - // module_info::imports vector (even if we discrad this - // information, we could at least reuse the buffers)? - // tmp = "@ "; tmp.append (r, b, n); tmp += ' '; tmp += bp; dd.expect (tmp); - st.headers++; + st.header_units++; } r = "PATHNAME "; r += bp; @@ -3091,7 +3088,8 @@ namespace build2 match_data& md, depdb& dd, bool& update, - timestamp mt) const + timestamp mt, + module_imports& imports) const { tracer trace (x, "compile_rule::extract_headers"); @@ -3924,7 +3922,7 @@ namespace build2 if (bt.path () == bp) { - md.headers++; + md.header_units++; skip_count++; return *u; } @@ -4106,7 +4104,7 @@ namespace build2 // if (mod_mapper || sense_diag) { - module_mapper_state mm_state (skip_count); + module_mapper_state mm_state (skip_count, imports); const char* w (nullptr); try @@ -4258,7 +4256,7 @@ namespace build2 } if (mod_mapper) - md.headers += mm_state.headers; + md.header_units += mm_state.header_units; } // The idea is to reduce this to the stdout case. @@ -4701,17 +4699,19 @@ namespace build2 return make_pair (move (psrc), puse); } - // Return the translation unit information (first) and its checksum - // (second). If the checksum is empty, then it should not be used. + // Return the translation unit information (last argument) and its + // checksum (result). If the checksum is empty, then it should not be + // used. // - pair compile_rule:: + string compile_rule:: parse_unit (action a, file& t, linfo li, const file& src, auto_rmfile& psrc, const match_data& md, - const path& dd) const + const path& dd, + unit& tu) const { tracer trace (x, "compile_rule::parse_unit"); @@ -4935,7 +4935,7 @@ namespace build2 fdstream_mode::binary | fdstream_mode::skip); parser p; - unit tu (p.parse (is, path_name (*sp))); + p.parse (is, path_name (*sp), tu); is.close (); @@ -4993,9 +4993,7 @@ namespace build2 // accurate (parts of the translation unit could have been // #ifdef'ed out; see __build2_preprocess). // - return pair ( - move (tu), - reprocess ? string () : move (p.checksum)); + return reprocess ? string () : move (p.checksum); } // Fall through. @@ -6126,7 +6124,7 @@ namespace build2 { case compiler_type::gcc: { - if (md.headers != 0) + if (md.header_units != 0) { string s (relative (dd).string ()); s.insert (0, "-fmodule-mapper="); @@ -6177,7 +6175,7 @@ namespace build2 // // Note that it is also used to specify the output BMI file. // - if (md.headers == 0 && // In append_header_options()? + if (md.header_units == 0 && // In append_header_options()? (ms.start != 0 || ut == unit_type::module_intf || ut == unit_type::module_intf_part || diff --git a/libbuild2/cc/compile-rule.hxx b/libbuild2/cc/compile-rule.hxx index 87d79bb..2d80d18 100644 --- a/libbuild2/cc/compile-rule.hxx +++ b/libbuild2/cc/compile-rule.hxx @@ -143,12 +143,13 @@ namespace build2 pair extract_headers (action, const scope&, file&, linfo, const file&, match_data&, - depdb&, bool&, timestamp) const; + depdb&, bool&, timestamp, module_imports&) const; - pair + string parse_unit (action, file&, linfo, const file&, auto_rmfile&, - const match_data&, const path&) const; + const match_data&, const path&, + unit&) const; void extract_modules (action, const scope&, file&, linfo, diff --git a/libbuild2/cc/parser.cxx b/libbuild2/cc/parser.cxx index 9559b87..61d62b7 100644 --- a/libbuild2/cc/parser.cxx +++ b/libbuild2/cc/parser.cxx @@ -14,13 +14,11 @@ namespace build2 { using type = token_type; - unit parser:: - parse (ifdstream& is, const path_name& in) + void parser:: + parse (ifdstream& is, const path_name& in, unit& u) { lexer l (is, in); l_ = &l; - - unit u; u_ = &u; // If the source has errors then we want the compiler to issues the @@ -160,7 +158,6 @@ namespace build2 << "global module fragment"; checksum = l.checksum (); - return u; } void parser:: diff --git a/libbuild2/cc/parser.hxx b/libbuild2/cc/parser.hxx index 9abfc81..a1f1e57 100644 --- a/libbuild2/cc/parser.hxx +++ b/libbuild2/cc/parser.hxx @@ -24,7 +24,15 @@ namespace build2 { public: unit - parse (ifdstream&, const path_name&); + parse (ifdstream& is, const path_name& n) + { + unit r; + parse (is, n, r); + return r; + } + + void + parse (ifdstream&, const path_name&, unit&); private: void -- cgit v1.1