Re: [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 <nathanielosh...@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 {