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

  It affects the build, and if used as a static file, can reliably be
tracked using the `-MF` mechanism.

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(-)
  

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)
  

On Fri, Jun 23, 2023 at 10:44:11 -0400, Jason Merrill wrote:
> 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 

Technically this is true: the command line for the TU lives in said
Makefile; if I updated it, a new TU would be really nice. This is a
long-standing limitation of `make` though. FWIW, `ninja` fixes it by
tracking the command line used and CMake's Makefiles generator handles
it by storing TU flags in an included file and depending on that file
from the TU output.

> shouldn't be necessary to rebuild all TUs that use modules when we add 
> another module to the mapper file.

If I change it from:

```
mod.a   mod.a.cmi
```

to:

```
mod.a   mod.a.replace.cmi
```

I'd expect a recompile. As with anything, this depends on the
granularity of the mapper files. A global mapper file is very similar to
a global response file and given that we don't have line-change
granularity dependency tracking…

>                                     What is your expected use case for 
> this dependency?

CMake, at least, uses a per-TU mapper file, so any build system using a
similar strategy handling the above case would only affect TUs that
actually list `mod.a`.

--Ben