[PATCH] c++/modules: Slightly clean up error for referencing TU-local entity

[Jason Merrill]

On 8/18/24 8:11 AM, Nathaniel Shead wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
> 
> Or should we even just remove the warning entirely?  I'm not sure it
> really adds all that much, since it's usual AFAICT for errors to prevent
> the intended outputs from being generated.

Agreed, removing it seems fine.

> -- >8 --
> 
> It was pointed out to me that the current error referencing an internal
> linkage entity reads almost like an ICE message, with the message
> finishing with the unhelpful:
> 
> m.cpp:1:8: error: failed to write compiled module: Bad file data
>      1 | export module M;
>        |        ^~~~~~
> 
> It would probably be clearer to just emit the same warning that we do in
> other cases where we don't write modules due to errors.
> 
> gcc/cp/ChangeLog:
> 
> 	* module.cc (module_state::write_begin): Return a boolean to
> 	indicate errors rather than just doing to->set_error().
> 	(finish_module_processing): Check for failed write_begin and
> 	disable module writing in that case.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/modules/block-decl-2.C: Adjust error message.
> 	* g++.dg/modules/internal-1.C: Likewise.
> 
> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> ---
>   gcc/cp/module.cc                            | 30 ++++++++++++---------
>   gcc/testsuite/g++.dg/modules/block-decl-2.C |  2 +-
>   gcc/testsuite/g++.dg/modules/internal-1.C   |  2 +-
>   3 files changed, 19 insertions(+), 15 deletions(-)
> 
> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> index f4d137b13a1..9f23feece09 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -3681,7 +3681,7 @@ class GTY((chain_next ("%h.parent"), for_user)) module_state {
>   
>    public:
>     /* Read and write module.  */
> -  void write_begin (elf_out *to, cpp_reader *,
> +  bool write_begin (elf_out *to, cpp_reader *,
>   		    module_state_config &, unsigned &crc);
>     void write_end (elf_out *to, cpp_reader *,
>   		  module_state_config &, unsigned &crc);
> @@ -18317,7 +18317,7 @@ ool_cmp (const void *a_, const void *b_)
>        MOD_SNAME_PFX.cfg      : config data
>   */
>   
> -void
> +bool
>   module_state::write_begin (elf_out *to, cpp_reader *reader,
>   			   module_state_config &config, unsigned &crc)
>   {
> @@ -18395,10 +18395,7 @@ module_state::write_begin (elf_out *to, cpp_reader *reader,
>     table.find_dependencies (this);
>   
>     if (!table.finalize_dependencies ())
> -    {
> -      to->set_error ();
> -      return;
> -    }
> +    return false;
>   
>   #if CHECKING_P
>     /* We're done verifying at-most once reading, reset to verify
> @@ -18595,6 +18592,8 @@ module_state::write_begin (elf_out *to, cpp_reader *reader,
>     // so-controlled.
>     if (false)
>       write_env (to);
> +
> +  return true;
>   }
>   
>   // Finish module writing after we've emitted all dynamic initializers.
> @@ -20847,22 +20846,27 @@ finish_module_processing (cpp_reader *reader)
>   
>         cookie = new module_processing_cookie (cmi_name, tmp_name, fd, e);
>   
> +      bool report_error = false;
>         if (errorcount
>   	  /* Don't write the module if it contains an erroneous template.  */
>   	  || (erroneous_templates
>   	      && !erroneous_templates->is_empty ()))
> -	warning_at (state->loc, 0, "not writing module %qs due to errors",
> -		    state->get_flatname ());
> +	report_error = true;
>         else if (cookie->out.begin ())
>   	{
> -	  cookie->began = true;
> -	  auto loc = input_location;
>   	  /* So crashes finger-point the module decl.  */
> -	  input_location = state->loc;
> -	  state->write_begin (&cookie->out, reader, cookie->config, cookie->crc);
> -	  input_location = loc;
> +	  iloc_sentinel ils = state->loc;
> +	  if (state->write_begin (&cookie->out, reader, cookie->config,
> +				  cookie->crc))
> +	    cookie->began = true;
> +	  else
> +	    report_error = true;
>   	}
>   
> +      if (report_error)
> +	warning_at (state->loc, 0, "not writing module %qs due to errors",
> +		    state->get_flatname ());
> +
>         dump.pop (n);
>         timevar_stop (TV_MODULE_EXPORT);
>   
> diff --git a/gcc/testsuite/g++.dg/modules/block-decl-2.C b/gcc/testsuite/g++.dg/modules/block-decl-2.C
> index 974e26f9b7a..90f18b30945 100644
> --- a/gcc/testsuite/g++.dg/modules/block-decl-2.C
> +++ b/gcc/testsuite/g++.dg/modules/block-decl-2.C
> @@ -18,4 +18,4 @@ export extern "C++" auto foo() {
>     return X{};
>   }
>   
> -// { dg-prune-output "failed to write compiled module" }
> +// { dg-prune-output "not writing module" }
> diff --git a/gcc/testsuite/g++.dg/modules/internal-1.C b/gcc/testsuite/g++.dg/modules/internal-1.C
> index 45d3bf06f28..399dd68b92e 100644
> --- a/gcc/testsuite/g++.dg/modules/internal-1.C
> +++ b/gcc/testsuite/g++.dg/modules/internal-1.C
> @@ -1,6 +1,6 @@
>   // { dg-additional-options "-fmodules-ts" }
>   
> -export module frob; // { dg-error "failed to write" }
> +export module frob; // { dg-warning "not writing module" }
>   // { dg-module-cmi !frob }
>   
>   namespace {