Re: [PATCH v5 5/5] c++modules: report module mapper files as a dependency

[Jason Merrill <jason@redhat.com>]

On 1/25/23 16:06, Ben Boeckel wrote:
> It affects the build, and if used as a static file, can reliably be
> tracked using the `-MF` mechanism.

Hmm, this seems a bit like making all .o depend on the Makefile; it 
shouldn't be necessary to rebuild all TUs that use modules when we add 
another module to the mapper file.  What is your expected use case for 
this dependency?

> gcc/cp/:
> 
> 	* mapper-client.cc, mapper-client.h (open_module_client): Accept
> 	dependency tracking and track module mapper files as
> 	dependencies.
> 	* module.cc (make_mapper, get_mapper): Pass the dependency
> 	tracking class down.
> 
> Signed-off-by: Ben Boeckel <ben.boeckel@kitware.com>
> ---
>   gcc/cp/mapper-client.cc |  4 ++++
>   gcc/cp/mapper-client.h  |  1 +
>   gcc/cp/module.cc        | 18 +++++++++---------
>   3 files changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/gcc/cp/mapper-client.cc b/gcc/cp/mapper-client.cc
> index 39e80df2d25..0ce5679d659 100644
> --- a/gcc/cp/mapper-client.cc
> +++ b/gcc/cp/mapper-client.cc
> @@ -34,6 +34,7 @@ along with GCC; see the file COPYING3.  If not see
>   #include "diagnostic-core.h"
>   #include "mapper-client.h"
>   #include "intl.h"
> +#include "mkdeps.h"
>   
>   #include "../../c++tools/resolver.h"
>   
> @@ -132,6 +133,7 @@ spawn_mapper_program (char const **errmsg, std::string &name,
>   
>   module_client *
>   module_client::open_module_client (location_t loc, const char *o,
> +				   class mkdeps *deps,
>   				   void (*set_repo) (const char *),
>   				   char const *full_program_name)
>   {
> @@ -285,6 +287,8 @@ module_client::open_module_client (location_t loc, const char *o,
>   	  errmsg = "opening";
>   	else
>   	  {
> +	    /* Add the mapper file to the dependency tracking. */
> +	    deps_add_dep (deps, name.c_str ());
>   	    if (int l = r->read_tuple_file (fd, ident, false))
>   	      {
>   		if (l > 0)
> diff --git a/gcc/cp/mapper-client.h b/gcc/cp/mapper-client.h
> index b32723ce296..a3b0b8adc51 100644
> --- a/gcc/cp/mapper-client.h
> +++ b/gcc/cp/mapper-client.h
> @@ -55,6 +55,7 @@ public:
>   
>   public:
>     static module_client *open_module_client (location_t loc, const char *option,
> +					    class mkdeps *,
>   					    void (*set_repo) (const char *),
>   					    char const *);
>     static void close_module_client (location_t loc, module_client *);
> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> index dbd1b721616..37066bf072b 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -3969,12 +3969,12 @@ static GTY(()) vec<tree, va_gc> *partial_specializations;
>   /* Our module mapper (created lazily).  */
>   module_client *mapper;
>   
> -static module_client *make_mapper (location_t loc);
> -inline module_client *get_mapper (location_t loc)
> +static module_client *make_mapper (location_t loc, class mkdeps *deps);
> +inline module_client *get_mapper (location_t loc, class mkdeps *deps)
>   {
>     auto *res = mapper;
>     if (!res)
> -    res = make_mapper (loc);
> +    res = make_mapper (loc, deps);
>     return res;
>   }
>   
> @@ -14031,7 +14031,7 @@ get_module (const char *ptr)
>   /* Create a new mapper connecting to OPTION.  */
>   
>   module_client *
> -make_mapper (location_t loc)
> +make_mapper (location_t loc, class mkdeps *deps)
>   {
>     timevar_start (TV_MODULE_MAPPER);
>     const char *option = module_mapper_name;
> @@ -14039,7 +14039,7 @@ make_mapper (location_t loc)
>       option = getenv ("CXX_MODULE_MAPPER");
>   
>     mapper = module_client::open_module_client
> -    (loc, option, &set_cmi_repo,
> +    (loc, option, deps, &set_cmi_repo,
>        (save_decoded_options[0].opt_index == OPT_SPECIAL_program_name)
>        && save_decoded_options[0].arg != progname
>        ? save_decoded_options[0].arg : nullptr);
> @@ -19503,7 +19503,7 @@ maybe_translate_include (cpp_reader *reader, line_maps *lmaps, location_t loc,
>     dump.push (NULL);
>   
>     dump () && dump ("Checking include translation '%s'", path);
> -  auto *mapper = get_mapper (cpp_main_loc (reader));
> +  auto *mapper = get_mapper (cpp_main_loc (reader), cpp_get_deps (reader));
>   
>     size_t len = strlen (path);
>     path = canonicalize_header_name (NULL, loc, true, path, len);
> @@ -19619,7 +19619,7 @@ module_begin_main_file (cpp_reader *reader, line_maps *lmaps,
>   static void
>   name_pending_imports (cpp_reader *reader)
>   {
> -  auto *mapper = get_mapper (cpp_main_loc (reader));
> +  auto *mapper = get_mapper (cpp_main_loc (reader), cpp_get_deps (reader));
>   
>     if (!vec_safe_length (pending_imports))
>       /* Not doing anything.  */
> @@ -20089,7 +20089,7 @@ init_modules (cpp_reader *reader)
>   
>     if (!flag_module_lazy)
>       /* Get the mapper now, if we're not being lazy.  */
> -    get_mapper (cpp_main_loc (reader));
> +    get_mapper (cpp_main_loc (reader), cpp_get_deps (reader));
>   
>     if (!flag_preprocess_only)
>       {
> @@ -20299,7 +20299,7 @@ late_finish_module (cpp_reader *reader,  module_processing_cookie *cookie,
>   
>     if (!errorcount)
>       {
> -      auto *mapper = get_mapper (cpp_main_loc (reader));
> +      auto *mapper = get_mapper (cpp_main_loc (reader), cpp_get_deps (reader));
>         mapper->ModuleCompiled (state->get_flatname ());
>       }
>     else if (cookie->cmi_name)