public inbox for gcc@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH v6 0/4] P1689R5 support
@ 2023-06-06 20:50 Ben Boeckel
  2023-06-06 20:50 ` [PATCH v6 1/4] libcpp: reject codepoints above 0x10FFFF Ben Boeckel
                   ` (4 more replies)
  0 siblings, 5 replies; 14+ messages in thread
From: Ben Boeckel @ 2023-06-06 20:50 UTC (permalink / raw)
  To: gcc-patches; +Cc: Ben Boeckel, jason, nathan, fortran, gcc, brad.king

Hi,

This patch series adds initial support for ISO C++'s [P1689R5][], a
format for describing C++ module requirements and provisions based on
the source code. This is required because compiling C++ with modules is
not embarrassingly parallel and need to be ordered to ensure that
`import some_module;` can be satisfied in time by making sure that any
TU with `export import some_module;` is compiled first.

[P1689R5]: https://isocpp.org/files/papers/P1689R5.html

I've also added patches to include imported module CMI files and the
module mapper file as dependencies of the compilation. I briefly looked
into adding dependencies on response files as well, but that appeared to
need some code contortions to have a `class mkdeps` available before
parsing the command line or to keep the information around until one was
made.

I'd like feedback on the approach taken here with respect to the
user-visible flags. I'll also note that header units are not supported
at this time because the current `-E` behavior with respect to `import
<some_header>;` is to search for an appropriate `.gcm` file which is not
something such a "scan" can support. A new mode will likely need to be
created (e.g., replacing `-E` with `-fc++-module-scanning` or something)
where headers are looked up "normally" and processed only as much as
scanning requires.

FWIW, Clang as taken an alternate approach with its `clang-scan-deps`
tool rather than using the compiler directly.

Thanks,

--Ben

---
v5 -> v6:

- rebase onto `master` (585c660f041 (reload1: Change return type of
  predicate function from int to bool, 2023-06-06))
- fix crash related to reporting imported CMI files as dependencies
- rework utf-8 validity to patch the new `cpp_valid_utf8_p` function
  instead of the core utf-8 decoding routine to reject invalid
  codepoints (preserves higher-level error detection of invalid utf-8)
- harmonize of `fdeps` spelling in flags, variables, comments, etc.
- rename `-fdeps-output=` to `-fdeps-target=`

v4 -> v5:

- add dependency tracking for imported modules to `-MF`
- add dependency tracking for static module mapper files given to
  `-fmodule-mapper=`

v3 -> v4:

- add missing spaces between function names and arguments

v2 -> v3:

- changelog entries moved to commit messages
- documentation updated/added in the UTF-8 routine editing

v1 -> v2:

- removal of the `deps_write(extra)` parameter to option-checking where
  ndeeded
- default parameter of `cpp_finish(fdeps_stream = NULL)`
- unification of libcpp UTF-8 validity functions from v1
- test cases for flag parsing states (depflags-*) and p1689 output
  (p1689-*)

Ben Boeckel (4):
  libcpp: reject codepoints above 0x10FFFF
  p1689r5: initial support
  c++modules: report imported CMI files as dependencies
  c++modules: report module mapper files as a dependency

 gcc/c-family/c-opts.cc                        |  40 +++-
 gcc/c-family/c.opt                            |  12 +
 gcc/cp/mapper-client.cc                       |   4 +
 gcc/cp/mapper-client.h                        |   1 +
 gcc/cp/module.cc                              |  24 +-
 gcc/doc/invoke.texi                           |  15 ++
 gcc/testsuite/g++.dg/modules/depflags-f-MD.C  |   2 +
 gcc/testsuite/g++.dg/modules/depflags-f.C     |   1 +
 gcc/testsuite/g++.dg/modules/depflags-fi.C    |   3 +
 gcc/testsuite/g++.dg/modules/depflags-fj-MD.C |   3 +
 gcc/testsuite/g++.dg/modules/depflags-fj.C    |   4 +
 .../g++.dg/modules/depflags-fjo-MD.C          |   4 +
 gcc/testsuite/g++.dg/modules/depflags-fjo.C   |   5 +
 gcc/testsuite/g++.dg/modules/depflags-fo-MD.C |   3 +
 gcc/testsuite/g++.dg/modules/depflags-fo.C    |   4 +
 gcc/testsuite/g++.dg/modules/depflags-j-MD.C  |   2 +
 gcc/testsuite/g++.dg/modules/depflags-j.C     |   3 +
 gcc/testsuite/g++.dg/modules/depflags-jo-MD.C |   3 +
 gcc/testsuite/g++.dg/modules/depflags-jo.C    |   4 +
 gcc/testsuite/g++.dg/modules/depflags-o-MD.C  |   2 +
 gcc/testsuite/g++.dg/modules/depflags-o.C     |   3 +
 gcc/testsuite/g++.dg/modules/modules.exp      |   1 +
 gcc/testsuite/g++.dg/modules/p1689-1.C        |  18 ++
 gcc/testsuite/g++.dg/modules/p1689-1.exp.json |  27 +++
 gcc/testsuite/g++.dg/modules/p1689-2.C        |  16 ++
 gcc/testsuite/g++.dg/modules/p1689-2.exp.json |  16 ++
 gcc/testsuite/g++.dg/modules/p1689-3.C        |  14 ++
 gcc/testsuite/g++.dg/modules/p1689-3.exp.json |  16 ++
 gcc/testsuite/g++.dg/modules/p1689-4.C        |  14 ++
 gcc/testsuite/g++.dg/modules/p1689-4.exp.json |  14 ++
 gcc/testsuite/g++.dg/modules/p1689-5.C        |  14 ++
 gcc/testsuite/g++.dg/modules/p1689-5.exp.json |  14 ++
 gcc/testsuite/g++.dg/modules/test-p1689.py    | 222 ++++++++++++++++++
 gcc/testsuite/lib/modules.exp                 |  71 ++++++
 libcpp/charset.cc                             |   7 +
 libcpp/include/cpplib.h                       |  12 +-
 libcpp/include/mkdeps.h                       |  17 +-
 libcpp/init.cc                                |  13 +-
 libcpp/mkdeps.cc                              | 149 +++++++++++-
 39 files changed, 769 insertions(+), 28 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/modules/depflags-f-MD.C
 create mode 100644 gcc/testsuite/g++.dg/modules/depflags-f.C
 create mode 100644 gcc/testsuite/g++.dg/modules/depflags-fi.C
 create mode 100644 gcc/testsuite/g++.dg/modules/depflags-fj-MD.C
 create mode 100644 gcc/testsuite/g++.dg/modules/depflags-fj.C
 create mode 100644 gcc/testsuite/g++.dg/modules/depflags-fjo-MD.C
 create mode 100644 gcc/testsuite/g++.dg/modules/depflags-fjo.C
 create mode 100644 gcc/testsuite/g++.dg/modules/depflags-fo-MD.C
 create mode 100644 gcc/testsuite/g++.dg/modules/depflags-fo.C
 create mode 100644 gcc/testsuite/g++.dg/modules/depflags-j-MD.C
 create mode 100644 gcc/testsuite/g++.dg/modules/depflags-j.C
 create mode 100644 gcc/testsuite/g++.dg/modules/depflags-jo-MD.C
 create mode 100644 gcc/testsuite/g++.dg/modules/depflags-jo.C
 create mode 100644 gcc/testsuite/g++.dg/modules/depflags-o-MD.C
 create mode 100644 gcc/testsuite/g++.dg/modules/depflags-o.C
 create mode 100644 gcc/testsuite/g++.dg/modules/p1689-1.C
 create mode 100644 gcc/testsuite/g++.dg/modules/p1689-1.exp.json
 create mode 100644 gcc/testsuite/g++.dg/modules/p1689-2.C
 create mode 100644 gcc/testsuite/g++.dg/modules/p1689-2.exp.json
 create mode 100644 gcc/testsuite/g++.dg/modules/p1689-3.C
 create mode 100644 gcc/testsuite/g++.dg/modules/p1689-3.exp.json
 create mode 100644 gcc/testsuite/g++.dg/modules/p1689-4.C
 create mode 100644 gcc/testsuite/g++.dg/modules/p1689-4.exp.json
 create mode 100644 gcc/testsuite/g++.dg/modules/p1689-5.C
 create mode 100644 gcc/testsuite/g++.dg/modules/p1689-5.exp.json
 create mode 100644 gcc/testsuite/g++.dg/modules/test-p1689.py
 create mode 100644 gcc/testsuite/lib/modules.exp


base-commit: 585c660f041c57694a61b3e65085af5a54431be4
-- 
2.40.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v6 1/4] libcpp: reject codepoints above 0x10FFFF
  2023-06-06 20:50 [PATCH v6 0/4] P1689R5 support Ben Boeckel
@ 2023-06-06 20:50 ` Ben Boeckel
  2023-06-19 21:34   ` Jason Merrill
  2023-06-06 20:50 ` [PATCH v6 2/4] p1689r5: initial support Ben Boeckel
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 14+ messages in thread
From: Ben Boeckel @ 2023-06-06 20:50 UTC (permalink / raw)
  To: gcc-patches; +Cc: Ben Boeckel, jason, nathan, fortran, gcc, brad.king

Unicode does not support such values because they are unrepresentable in
UTF-16.

libcpp/

	* charset.cc: Reject encodings of codepoints above 0x10FFFF.
	UTF-16 does not support such codepoints and therefore all
	Unicode rejects such values.

Signed-off-by: Ben Boeckel <ben.boeckel@kitware.com>
---
 libcpp/charset.cc | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/libcpp/charset.cc b/libcpp/charset.cc
index d7f323b2cd5..3b34d804cf1 100644
--- a/libcpp/charset.cc
+++ b/libcpp/charset.cc
@@ -1886,6 +1886,13 @@ cpp_valid_utf8_p (const char *buffer, size_t num_bytes)
       int err = one_utf8_to_cppchar (&iter, &bytesleft, &cp);
       if (err)
 	return false;
+
+      /* Additionally, Unicode declares that all codepoints above 0010FFFF are
+	 invalid because they cannot be represented in UTF-16.
+
+	 Reject such values.*/
+      if (cp >= 0x10FFFF)
+	return false;
     }
   /* No problems encountered.  */
   return true;
-- 
2.40.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v6 2/4] p1689r5: initial support
  2023-06-06 20:50 [PATCH v6 0/4] P1689R5 support Ben Boeckel
  2023-06-06 20:50 ` [PATCH v6 1/4] libcpp: reject codepoints above 0x10FFFF Ben Boeckel
@ 2023-06-06 20:50 ` Ben Boeckel
  2023-06-06 20:50 ` [PATCH v6 3/4] c++modules: report imported CMI files as dependencies Ben Boeckel
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 14+ messages in thread
From: Ben Boeckel @ 2023-06-06 20:50 UTC (permalink / raw)
  To: gcc-patches; +Cc: Ben Boeckel, jason, nathan, fortran, gcc, brad.king

This patch implements support for [P1689R5][] to communicate to a build
system the C++20 module dependencies to build systems so that they may
build `.gcm` files in the proper order.

Support is communicated through the following three new flags:

- `-fdeps-format=` specifies the format for the output. Currently named
  `p1689r5`.

- `-fdeps-file=` specifies the path to the file to write the format to.

- `-fdeps-target=` specifies the `.o` that will be written for the TU
  that is scanned. This is required so that the build system can
  correlate the dependency output with the actual compilation that will
  occur.

CMake supports this format as of 17 Jun 2022 (to be part of 3.25.0)
using an experimental feature selection (to allow for future usage
evolution without committing to how it works today). While it remains
experimental, docs may be found in CMake's documentation for
experimental features.

Future work may include using this format for Fortran module
dependencies as well, however this is still pending work.

[P1689R5]: https://isocpp.org/files/papers/P1689R5.html
[cmake-experimental]: https://gitlab.kitware.com/cmake/cmake/-/blob/master/Help/dev/experimental.rst

TODO:

- header-unit information fields

Header units (including the standard library headers) are 100%
unsupported right now because the `-E` mechanism wants to import their
BMIs. A new mode (i.e., something more workable than existing `-E`
behavior) that mocks up header units as if they were imported purely
from their path and content would be required.

- non-utf8 paths

The current standard says that paths that are not unambiguously
represented using UTF-8 are not supported (because these cases are rare
and the extra complication is not worth it at this time). Future
versions of the format might have ways of encoding non-UTF-8 paths. For
now, this patch just doesn't support non-UTF-8 paths (ignoring the
"unambiguously represetable in UTF-8" case).

- figure out why junk gets placed at the end of the file

Sometimes it seems like the file gets a lot of `NUL` bytes appended to
it. It happens rarely and seems to be the result of some
`ftruncate`-style call which results in extra padding in the contents.
Noting it here as an observation at least.

libcpp/

	* include/cpplib.h: Add cpp_deps_format enum.
	(cpp_options): Add format field
	(cpp_finish): Add dependency stream parameter.
	* include/mkdeps.h (deps_add_module_target): Add new preprocessor
	parameter used for C++ module tracking.
	* init.cc (cpp_finish): Add new preprocessor parameter used for C++
	module tracking.
	* mkdeps.cc (mkdeps): Implement P1689R5 output.

gcc/

	* doc/invoke.texi: Document -fdeps-format=, -fdeps-file=, and
	-fdeps-target= flags.

gcc/c-family/

	* c-opts.cc (c_common_handle_option): Add fdeps_file variable and
	-fdeps-format=, -fdeps-file=, and -fdeps-target= parsing.
	* c.opt: Add -fdeps-format=, -fdeps-file=, and -fdeps-target=
	flags.

gcc/cp/

	* module.cc (preprocessed_module): Pass whether the module is
	exported to dependency tracking.

gcc/testsuite/

	* g++.dg/modules/depflags-f-MD.C: New test.
	* g++.dg/modules/depflags-f.C: New test.
	* g++.dg/modules/depflags-fi.C: New test.
	* g++.dg/modules/depflags-fj-MD.C: New test.
	* g++.dg/modules/depflags-fj.C: New test.
	* g++.dg/modules/depflags-fjo-MD.C: New test.
	* g++.dg/modules/depflags-fjo.C: New test.
	* g++.dg/modules/depflags-fo-MD.C: New test.
	* g++.dg/modules/depflags-fo.C: New test.
	* g++.dg/modules/depflags-j-MD.C: New test.
	* g++.dg/modules/depflags-j.C: New test.
	* g++.dg/modules/depflags-jo-MD.C: New test.
	* g++.dg/modules/depflags-jo.C: New test.
	* g++.dg/modules/depflags-o-MD.C: New test.
	* g++.dg/modules/depflags-o.C: New test.
	* g++.dg/modules/p1689-1.C: New test.
	* g++.dg/modules/p1689-1.exp.json: New test expectation.
	* g++.dg/modules/p1689-2.C: New test.
	* g++.dg/modules/p1689-2.exp.json: New test expectation.
	* g++.dg/modules/p1689-3.C: New test.
	* g++.dg/modules/p1689-3.exp.json: New test expectation.
	* g++.dg/modules/p1689-4.C: New test.
	* g++.dg/modules/p1689-4.exp.json: New test expectation.
	* g++.dg/modules/p1689-5.C: New test.
	* g++.dg/modules/p1689-5.exp.json: New test expectation.
	* g++.dg/modules/modules.exp: Load new P1689 library routines.
	* g++.dg/modules/test-p1689.py: New tool for validating P1689 output.
	* lib/modules.exp: Support for validating P1689 outputs.

Signed-off-by: Ben Boeckel <ben.boeckel@kitware.com>
---
 gcc/c-family/c-opts.cc                        |  40 +++-
 gcc/c-family/c.opt                            |  12 +
 gcc/cp/module.cc                              |   3 +-
 gcc/doc/invoke.texi                           |  15 ++
 gcc/testsuite/g++.dg/modules/depflags-f-MD.C  |   2 +
 gcc/testsuite/g++.dg/modules/depflags-f.C     |   1 +
 gcc/testsuite/g++.dg/modules/depflags-fi.C    |   3 +
 gcc/testsuite/g++.dg/modules/depflags-fj-MD.C |   3 +
 gcc/testsuite/g++.dg/modules/depflags-fj.C    |   4 +
 .../g++.dg/modules/depflags-fjo-MD.C          |   4 +
 gcc/testsuite/g++.dg/modules/depflags-fjo.C   |   5 +
 gcc/testsuite/g++.dg/modules/depflags-fo-MD.C |   3 +
 gcc/testsuite/g++.dg/modules/depflags-fo.C    |   4 +
 gcc/testsuite/g++.dg/modules/depflags-j-MD.C  |   2 +
 gcc/testsuite/g++.dg/modules/depflags-j.C     |   3 +
 gcc/testsuite/g++.dg/modules/depflags-jo-MD.C |   3 +
 gcc/testsuite/g++.dg/modules/depflags-jo.C    |   4 +
 gcc/testsuite/g++.dg/modules/depflags-o-MD.C  |   2 +
 gcc/testsuite/g++.dg/modules/depflags-o.C     |   3 +
 gcc/testsuite/g++.dg/modules/modules.exp      |   1 +
 gcc/testsuite/g++.dg/modules/p1689-1.C        |  18 ++
 gcc/testsuite/g++.dg/modules/p1689-1.exp.json |  27 +++
 gcc/testsuite/g++.dg/modules/p1689-2.C        |  16 ++
 gcc/testsuite/g++.dg/modules/p1689-2.exp.json |  16 ++
 gcc/testsuite/g++.dg/modules/p1689-3.C        |  14 ++
 gcc/testsuite/g++.dg/modules/p1689-3.exp.json |  16 ++
 gcc/testsuite/g++.dg/modules/p1689-4.C        |  14 ++
 gcc/testsuite/g++.dg/modules/p1689-4.exp.json |  14 ++
 gcc/testsuite/g++.dg/modules/p1689-5.C        |  14 ++
 gcc/testsuite/g++.dg/modules/p1689-5.exp.json |  14 ++
 gcc/testsuite/g++.dg/modules/test-p1689.py    | 222 ++++++++++++++++++
 gcc/testsuite/lib/modules.exp                 |  71 ++++++
 libcpp/include/cpplib.h                       |  12 +-
 libcpp/include/mkdeps.h                       |  17 +-
 libcpp/init.cc                                |  13 +-
 libcpp/mkdeps.cc                              | 149 +++++++++++-
 36 files changed, 745 insertions(+), 19 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/modules/depflags-f-MD.C
 create mode 100644 gcc/testsuite/g++.dg/modules/depflags-f.C
 create mode 100644 gcc/testsuite/g++.dg/modules/depflags-fi.C
 create mode 100644 gcc/testsuite/g++.dg/modules/depflags-fj-MD.C
 create mode 100644 gcc/testsuite/g++.dg/modules/depflags-fj.C
 create mode 100644 gcc/testsuite/g++.dg/modules/depflags-fjo-MD.C
 create mode 100644 gcc/testsuite/g++.dg/modules/depflags-fjo.C
 create mode 100644 gcc/testsuite/g++.dg/modules/depflags-fo-MD.C
 create mode 100644 gcc/testsuite/g++.dg/modules/depflags-fo.C
 create mode 100644 gcc/testsuite/g++.dg/modules/depflags-j-MD.C
 create mode 100644 gcc/testsuite/g++.dg/modules/depflags-j.C
 create mode 100644 gcc/testsuite/g++.dg/modules/depflags-jo-MD.C
 create mode 100644 gcc/testsuite/g++.dg/modules/depflags-jo.C
 create mode 100644 gcc/testsuite/g++.dg/modules/depflags-o-MD.C
 create mode 100644 gcc/testsuite/g++.dg/modules/depflags-o.C
 create mode 100644 gcc/testsuite/g++.dg/modules/p1689-1.C
 create mode 100644 gcc/testsuite/g++.dg/modules/p1689-1.exp.json
 create mode 100644 gcc/testsuite/g++.dg/modules/p1689-2.C
 create mode 100644 gcc/testsuite/g++.dg/modules/p1689-2.exp.json
 create mode 100644 gcc/testsuite/g++.dg/modules/p1689-3.C
 create mode 100644 gcc/testsuite/g++.dg/modules/p1689-3.exp.json
 create mode 100644 gcc/testsuite/g++.dg/modules/p1689-4.C
 create mode 100644 gcc/testsuite/g++.dg/modules/p1689-4.exp.json
 create mode 100644 gcc/testsuite/g++.dg/modules/p1689-5.C
 create mode 100644 gcc/testsuite/g++.dg/modules/p1689-5.exp.json
 create mode 100644 gcc/testsuite/g++.dg/modules/test-p1689.py
 create mode 100644 gcc/testsuite/lib/modules.exp

diff --git a/gcc/c-family/c-opts.cc b/gcc/c-family/c-opts.cc
index c68a2a27469..3958aa7c7f2 100644
--- a/gcc/c-family/c-opts.cc
+++ b/gcc/c-family/c-opts.cc
@@ -77,6 +77,9 @@ static bool verbose;
 /* Dependency output file.  */
 static const char *deps_file;
 
+/* Enhanced dependency output file.  */
+static const char *fdeps_file;
+
 /* The prefix given by -iprefix, if any.  */
 static const char *iprefix;
 
@@ -360,6 +363,23 @@ c_common_handle_option (size_t scode, const char *arg, HOST_WIDE_INT value,
       deps_file = arg;
       break;
 
+    case OPT_fdeps_format_:
+      if (!strcmp (arg, "p1689r5"))
+	cpp_opts->deps.format = DEPS_FMT_P1689R5;
+      else
+	error ("%<-fdeps-format=%> unknown format %<%s%>", arg);
+      break;
+
+    case OPT_fdeps_file_:
+      deps_seen = true;
+      fdeps_file = arg;
+      break;
+
+    case OPT_fdeps_target_:
+      deps_seen = true;
+      defer_opt (code, arg);
+      break;
+
     case OPT_MF:
       deps_seen = true;
       deps_file = arg;
@@ -1272,6 +1292,7 @@ void
 c_common_finish (void)
 {
   FILE *deps_stream = NULL;
+  FILE *fdeps_stream = NULL;
 
   /* Note that we write the dependencies even if there are errors. This is
      useful for handling outdated generated headers that now trigger errors
@@ -1300,9 +1321,24 @@ c_common_finish (void)
      locations with input_location, which would be incorrect now.  */
   override_libcpp_locations = false;
 
+  if (cpp_opts->deps.format != DEPS_FMT_NONE)
+    {
+      if (!fdeps_file)
+	fdeps_stream = out_stream;
+      else if (fdeps_file[0] == '-' && fdeps_file[1] == '\0')
+	fdeps_stream = stdout;
+      else
+	{
+	  fdeps_stream = fopen (fdeps_file, "w");
+	  if (!fdeps_stream)
+	    fatal_error (input_location, "opening dependency file %s: %m",
+			 fdeps_file);
+	}
+    }
+
   /* For performance, avoid tearing down cpplib's internal structures
      with cpp_destroy ().  */
-  cpp_finish (parse_in, deps_stream);
+  cpp_finish (parse_in, deps_stream, fdeps_stream);
 
   if (deps_stream && deps_stream != out_stream && deps_stream != stdout
       && (ferror (deps_stream) || fclose (deps_stream)))
@@ -1374,6 +1410,8 @@ handle_deferred_opts (void)
 
 	if (opt->code == OPT_MT || opt->code == OPT_MQ)
 	  deps_add_target (deps, opt->arg, opt->code == OPT_MQ);
+	else if (opt->code == OPT_fdeps_target_)
+	  deps_add_output (deps, opt->arg, true);
       }
 }
 
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 3333cddeece..0e5dcbe3dab 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -256,6 +256,18 @@ MT
 C ObjC C++ ObjC++ Joined Separate MissingArgError(missing makefile target after %qs)
 -MT <target>	Add a target that does not require quoting.
 
+fdeps-format=
+C ObjC C++ ObjC++ NoDriverArg Joined MissingArgError(missing format after %qs)
+Format for output dependency information.  Supported (\"p1689r5\").
+
+fdeps-file=
+C ObjC C++ ObjC++ NoDriverArg Joined MissingArgError(missing output path after %qs)
+File for output dependency information.
+
+fdeps-target=
+C ObjC C++ ObjC++ NoDriverArg Joined MissingArgError(missing path after %qs)
+-fdeps-target=obj.o Output file for the compile step.
+
 P
 C ObjC C++ ObjC++
 Do not generate #line directives.
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index ecde98d69b4..c80f139eb82 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -19832,7 +19832,8 @@ preprocessed_module (cpp_reader *reader)
 		  && (module->is_interface () || module->is_partition ()))
 		deps_add_module_target (deps, module->get_flatname (),
 					maybe_add_cmi_prefix (module->filename),
-					module->is_header());
+					module->is_header (),
+					module->is_exported ());
 	      else
 		deps_add_module_dep (deps, module->get_flatname ());
 	    }
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 9130104af22..41952739b62 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -2807,6 +2807,21 @@ is @option{-fpermitted-flt-eval-methods=c11}.  The default when in a GNU
 dialect (@option{-std=gnu11} or similar) is
 @option{-fpermitted-flt-eval-methods=ts-18661-3}.
 
+@opindex fdeps-file
+@item -fdeps-file=@var{file}
+Where to write structured dependency information.
+
+@opindex fdeps-format
+@item -fdeps-format=@var{format}
+The format to use for structured dependency information. @samp{p1689r5} is the
+only supported format right now.  Note that when this argument is specified, the
+output of @samp{-MF} is stripped of some information (namely C++ modules) so
+that it does not use extended makefile syntax not understood by most tools.
+
+@opindex fdeps-target
+@item -fdeps-target=@var{file}
+Analogous to @option{-MT} but for structured dependency information.
+
 @opindex fplan9-extensions
 @item -fplan9-extensions
 Accept some non-standard constructs used in Plan 9 code.
diff --git a/gcc/testsuite/g++.dg/modules/depflags-f-MD.C b/gcc/testsuite/g++.dg/modules/depflags-f-MD.C
new file mode 100644
index 00000000000..2bd980c0047
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/depflags-f-MD.C
@@ -0,0 +1,2 @@
+// { dg-additional-options -MD }
+// { dg-additional-options -fdeps-format=p1689r5 }
diff --git a/gcc/testsuite/g++.dg/modules/depflags-f.C b/gcc/testsuite/g++.dg/modules/depflags-f.C
new file mode 100644
index 00000000000..126e691601c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/depflags-f.C
@@ -0,0 +1 @@
+// { dg-additional-options -fdeps-format=p1689r5 }
diff --git a/gcc/testsuite/g++.dg/modules/depflags-fi.C b/gcc/testsuite/g++.dg/modules/depflags-fi.C
new file mode 100644
index 00000000000..81a1e62719a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/depflags-fi.C
@@ -0,0 +1,3 @@
+// { dg-additional-options -fdeps-format=invalid }
+
+// { dg-prune-output "error: '-fdeps-format=' unknown format 'invalid'"  }
diff --git a/gcc/testsuite/g++.dg/modules/depflags-fj-MD.C b/gcc/testsuite/g++.dg/modules/depflags-fj-MD.C
new file mode 100644
index 00000000000..f85b294123a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/depflags-fj-MD.C
@@ -0,0 +1,3 @@
+// { dg-additional-options -MD }
+// { dg-additional-options -fdeps-file=depflags-3.json }
+// { dg-additional-options -fdeps-format=p1689r5 }
diff --git a/gcc/testsuite/g++.dg/modules/depflags-fj.C b/gcc/testsuite/g++.dg/modules/depflags-fj.C
new file mode 100644
index 00000000000..153d6f24c34
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/depflags-fj.C
@@ -0,0 +1,4 @@
+// { dg-additional-options -fdeps-file=depflags-3.json }
+// { dg-additional-options -fdeps-format=p1689r5 }
+
+// { dg-prune-output "error: to generate dependencies you must specify either '-M' or '-MM'" }
diff --git a/gcc/testsuite/g++.dg/modules/depflags-fjo-MD.C b/gcc/testsuite/g++.dg/modules/depflags-fjo-MD.C
new file mode 100644
index 00000000000..39870d40204
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/depflags-fjo-MD.C
@@ -0,0 +1,4 @@
+// { dg-additional-options -MD }
+// { dg-additional-options -fdeps-file=depflags-3.json }
+// { dg-additional-options -fdeps-target=depflags-1.C }
+// { dg-additional-options -fdeps-format=p1689r5 }
diff --git a/gcc/testsuite/g++.dg/modules/depflags-fjo.C b/gcc/testsuite/g++.dg/modules/depflags-fjo.C
new file mode 100644
index 00000000000..c58ee836d89
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/depflags-fjo.C
@@ -0,0 +1,5 @@
+// { dg-additional-options -fdeps-file=depflags-3.json }
+// { dg-additional-options -fdeps-target=depflags-1.C }
+// { dg-additional-options -fdeps-format=p1689r5 }
+
+// { dg-prune-output "error: to generate dependencies you must specify either '-M' or '-MM'" }
diff --git a/gcc/testsuite/g++.dg/modules/depflags-fo-MD.C b/gcc/testsuite/g++.dg/modules/depflags-fo-MD.C
new file mode 100644
index 00000000000..6da03f9133f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/depflags-fo-MD.C
@@ -0,0 +1,3 @@
+// { dg-additional-options -MD }
+// { dg-additional-options -fdeps-format=p1689r5 }
+// { dg-additional-options -fdeps-target=depflags-1.C }
diff --git a/gcc/testsuite/g++.dg/modules/depflags-fo.C b/gcc/testsuite/g++.dg/modules/depflags-fo.C
new file mode 100644
index 00000000000..5deb27a4598
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/depflags-fo.C
@@ -0,0 +1,4 @@
+// { dg-additional-options -fdeps-format=p1689r5 }
+// { dg-additional-options -fdeps-target=depflags-1.C }
+
+// { dg-prune-output "error: to generate dependencies you must specify either '-M' or '-MM'" }
diff --git a/gcc/testsuite/g++.dg/modules/depflags-j-MD.C b/gcc/testsuite/g++.dg/modules/depflags-j-MD.C
new file mode 100644
index 00000000000..cb4acf7090f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/depflags-j-MD.C
@@ -0,0 +1,2 @@
+// { dg-additional-options -MD }
+// { dg-additional-options -fdeps-file=depflags-3.json }
diff --git a/gcc/testsuite/g++.dg/modules/depflags-j.C b/gcc/testsuite/g++.dg/modules/depflags-j.C
new file mode 100644
index 00000000000..680eb502f2a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/depflags-j.C
@@ -0,0 +1,3 @@
+// { dg-additional-options -fdeps-file=depflags-3.json }
+
+// { dg-prune-output "error: to generate dependencies you must specify either '-M' or '-MM'" }
diff --git a/gcc/testsuite/g++.dg/modules/depflags-jo-MD.C b/gcc/testsuite/g++.dg/modules/depflags-jo-MD.C
new file mode 100644
index 00000000000..3f88d078e2d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/depflags-jo-MD.C
@@ -0,0 +1,3 @@
+// { dg-additional-options -MD }
+// { dg-additional-options -fdeps-file=depflags-3.json }
+// { dg-additional-options -fdeps-target=depflags-1.C }
diff --git a/gcc/testsuite/g++.dg/modules/depflags-jo.C b/gcc/testsuite/g++.dg/modules/depflags-jo.C
new file mode 100644
index 00000000000..47eff5fcd23
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/depflags-jo.C
@@ -0,0 +1,4 @@
+// { dg-additional-options -fdeps-file=depflags-3.json }
+// { dg-additional-options -fdeps-target=depflags-1.C }
+
+// { dg-prune-output "error: to generate dependencies you must specify either '-M' or '-MM'" }
diff --git a/gcc/testsuite/g++.dg/modules/depflags-o-MD.C b/gcc/testsuite/g++.dg/modules/depflags-o-MD.C
new file mode 100644
index 00000000000..1b67558b687
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/depflags-o-MD.C
@@ -0,0 +1,2 @@
+// { dg-additional-options -MD }
+// { dg-additional-options -fdeps-target=depflags-1.C }
diff --git a/gcc/testsuite/g++.dg/modules/depflags-o.C b/gcc/testsuite/g++.dg/modules/depflags-o.C
new file mode 100644
index 00000000000..38d645081b6
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/depflags-o.C
@@ -0,0 +1,3 @@
+// { dg-additional-options -fdeps-target=depflags-1.C }
+
+// { dg-prune-output "error: to generate dependencies you must specify either '-M' or '-MM'" }
diff --git a/gcc/testsuite/g++.dg/modules/modules.exp b/gcc/testsuite/g++.dg/modules/modules.exp
index dc302d3d0af..b6929544c59 100644
--- a/gcc/testsuite/g++.dg/modules/modules.exp
+++ b/gcc/testsuite/g++.dg/modules/modules.exp
@@ -28,6 +28,7 @@
 # { dg-module-do [link|run] [xfail] [options] } # link [and run]
 
 load_lib g++-dg.exp
+load_lib modules.exp
 
 # If a testcase doesn't have special options, use these.
 global DEFAULT_CXXFLAGS
diff --git a/gcc/testsuite/g++.dg/modules/p1689-1.C b/gcc/testsuite/g++.dg/modules/p1689-1.C
new file mode 100644
index 00000000000..3654c924b44
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/p1689-1.C
@@ -0,0 +1,18 @@
+// { dg-additional-options -E }
+// { dg-additional-options -MT }
+// { dg-additional-options p1689-1.json }
+// { dg-additional-options -MD }
+// { dg-additional-options -fmodules-ts }
+// { dg-additional-options -fdeps-format=p1689r5 }
+// { dg-additional-options -fdeps-target=p1689-1.o }
+// { dg-additional-options -fdeps-file=p1689-1.json }
+
+// Export a module that uses modules, re-exports modules, and partitions.
+
+export module foo;
+export import foo:part1;
+import foo:part2;
+
+export import bar;
+
+// { dg-final { run-check-p1689-valid p1689-1.json p1689-1.exp.json } }
diff --git a/gcc/testsuite/g++.dg/modules/p1689-1.exp.json b/gcc/testsuite/g++.dg/modules/p1689-1.exp.json
new file mode 100644
index 00000000000..c5648ac7ae5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/p1689-1.exp.json
@@ -0,0 +1,27 @@
+{
+    "rules": [
+        {
+            "primary-output": "p1689-1.o",
+            "provides": [
+                {
+                    "logical-name": "foo",
+                    "is-interface": true
+                }
+            ],
+            "requires": [
+                "__P1689_unordered__",
+                {
+                    "logical-name": "bar"
+                },
+                {
+                    "logical-name": "foo:part1"
+                },
+                {
+                    "logical-name": "foo:part2"
+                }
+            ]
+        }
+    ],
+    "version": 0,
+    "revision": 0
+}
diff --git a/gcc/testsuite/g++.dg/modules/p1689-2.C b/gcc/testsuite/g++.dg/modules/p1689-2.C
new file mode 100644
index 00000000000..e32cd88f5a6
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/p1689-2.C
@@ -0,0 +1,16 @@
+// { dg-additional-options -E }
+// { dg-additional-options -MT }
+// { dg-additional-options p1689-2.json }
+// { dg-additional-options -MD }
+// { dg-additional-options -fmodules-ts }
+// { dg-additional-options -fdeps-format=p1689r5 }
+// { dg-additional-options -fdeps-target=p1689-2.o }
+// { dg-additional-options -fdeps-file=p1689-2.json }
+
+// Export a module partition that uses modules.
+
+export module foo:part1;
+
+#include <iostream>
+
+// { dg-final { run-check-p1689-valid p1689-2.json p1689-2.exp.json } }
diff --git a/gcc/testsuite/g++.dg/modules/p1689-2.exp.json b/gcc/testsuite/g++.dg/modules/p1689-2.exp.json
new file mode 100644
index 00000000000..6901172f277
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/p1689-2.exp.json
@@ -0,0 +1,16 @@
+{
+    "rules": [
+        {
+            "primary-output": "p1689-2.o",
+            "provides": [
+                {
+                    "logical-name": "foo:part1",
+                    "is-interface": true
+                }
+            ],
+            "requires": []
+        }
+    ],
+    "version": 0,
+    "revision": 0
+}
diff --git a/gcc/testsuite/g++.dg/modules/p1689-3.C b/gcc/testsuite/g++.dg/modules/p1689-3.C
new file mode 100644
index 00000000000..d0962cd432d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/p1689-3.C
@@ -0,0 +1,14 @@
+// { dg-additional-options -E }
+// { dg-additional-options -MT }
+// { dg-additional-options p1689-3.json }
+// { dg-additional-options -MD }
+// { dg-additional-options -fmodules-ts }
+// { dg-additional-options -fdeps-format=p1689r5 }
+// { dg-additional-options -fdeps-target=p1689-3.o }
+// { dg-additional-options -fdeps-file=p1689-3.json }
+
+// Provide a module partition.
+
+module foo:part2;
+
+// { dg-final { run-check-p1689-valid p1689-3.json p1689-3.exp.json } }
diff --git a/gcc/testsuite/g++.dg/modules/p1689-3.exp.json b/gcc/testsuite/g++.dg/modules/p1689-3.exp.json
new file mode 100644
index 00000000000..5a40beacd22
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/p1689-3.exp.json
@@ -0,0 +1,16 @@
+{
+    "rules": [
+        {
+            "primary-output": "p1689-3.o",
+            "provides": [
+                {
+                    "logical-name": "foo:part2",
+                    "is-interface": false
+                }
+            ],
+            "requires": []
+        }
+    ],
+    "version": 0,
+    "revision": 0
+}
diff --git a/gcc/testsuite/g++.dg/modules/p1689-4.C b/gcc/testsuite/g++.dg/modules/p1689-4.C
new file mode 100644
index 00000000000..a1935579074
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/p1689-4.C
@@ -0,0 +1,14 @@
+// { dg-additional-options -E }
+// { dg-additional-options -MT }
+// { dg-additional-options p1689-4.json }
+// { dg-additional-options -MD }
+// { dg-additional-options -fmodules-ts }
+// { dg-additional-options -fdeps-format=p1689r5 }
+// { dg-additional-options -fdeps-target=p1689-4.o }
+// { dg-additional-options -fdeps-file=p1689-4.json }
+
+// Module implementation unit.
+
+module foo;
+
+// { dg-final { run-check-p1689-valid p1689-4.json p1689-4.exp.json } }
diff --git a/gcc/testsuite/g++.dg/modules/p1689-4.exp.json b/gcc/testsuite/g++.dg/modules/p1689-4.exp.json
new file mode 100644
index 00000000000..b119f5654b1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/p1689-4.exp.json
@@ -0,0 +1,14 @@
+{
+    "rules": [
+        {
+            "primary-output": "p1689-4.o",
+            "requires": []
+                {
+                    "logical-name": "foo"
+                }
+            ]
+        }
+    ],
+    "version": 0,
+    "revision": 0
+}
diff --git a/gcc/testsuite/g++.dg/modules/p1689-5.C b/gcc/testsuite/g++.dg/modules/p1689-5.C
new file mode 100644
index 00000000000..f42933856a8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/p1689-5.C
@@ -0,0 +1,14 @@
+// { dg-additional-options -E }
+// { dg-additional-options -MT }
+// { dg-additional-options p1689-5.json }
+// { dg-additional-options -MD }
+// { dg-additional-options -fmodules-ts }
+// { dg-additional-options -fdeps-format=p1689r5 }
+// { dg-additional-options -fdeps-target=p1689-5.o }
+// { dg-additional-options -fdeps-file=p1689-5.json }
+
+// Use modules, don't provide anything.
+
+import bar;
+
+// { dg-final { run-check-p1689-valid p1689-5.json p1689-5.exp.json } }
diff --git a/gcc/testsuite/g++.dg/modules/p1689-5.exp.json b/gcc/testsuite/g++.dg/modules/p1689-5.exp.json
new file mode 100644
index 00000000000..18704ac8820
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/p1689-5.exp.json
@@ -0,0 +1,14 @@
+{
+    "rules": [
+        {
+            "primary-output": "p1689-5.o",
+            "requires": [
+                {
+                    "logical-name": "bar"
+                }
+            ]
+        }
+    ],
+    "version": 0,
+    "revision": 0
+}
diff --git a/gcc/testsuite/g++.dg/modules/test-p1689.py b/gcc/testsuite/g++.dg/modules/test-p1689.py
new file mode 100644
index 00000000000..2f07cc361aa
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/test-p1689.py
@@ -0,0 +1,222 @@
+import json
+
+
+# Parameters.
+ALL_ERRORS = False
+REPLACEMENTS = {}
+
+
+def _print_path(path):
+    '''Format a JSON path for output.'''
+    return '/'.join(path)
+
+
+def _report_error(msg):
+    '''Report an error.'''
+    full_msg = 'ERROR: ' + msg
+    if ALL_ERRORS:
+        print(full_msg)
+    else:
+        raise RuntimeError(full_msg)
+
+
+def _error_type_mismatch(path, actual, expect):
+    '''Report that there is a type mismatch.'''
+    _report_error('type mismatch at %s: actual: "%s" expect: "%s"' % (_print_path(path), actual, expect))
+
+
+def _error_unknown_type(path, typ):
+    '''Report that there is an unknown type in the JSON object.'''
+    _report_error('unknown type at %s: "%s"' % (_print_path(path), typ))
+
+
+def _error_length_mismatch(path, actual, expect):
+    '''Report a length mismatch in an object or array.'''
+    _report_error('length mismatch at %s: actual: "%s" expect: "%s"' % (_print_path(path), actual, expect))
+
+
+def _error_unexpect_value(path, actual, expect):
+    '''Report a value mismatch.'''
+    _report_error('value mismatch at %s: actual: "%s" expect: "%s"' % (_print_path(path), actual, expect))
+
+
+def _error_extra_key(path, key):
+    '''Report on a key that is unexpected.'''
+    _report_error('extra key at %s: "%s"' % (_print_path(path), key))
+
+
+def _error_missing_key(path, key):
+    '''Report on a key that is missing.'''
+    _report_error('extra key at %s: %s' % (_print_path(path), key))
+
+
+def _compare_object(path, actual, expect):
+    '''Compare a JSON object.'''
+    is_ok = True
+
+    if not len(actual) == len(expect):
+        _error_length_mismatch(path, len(actual), len(expect))
+        is_ok = False
+
+    for key in actual:
+        if key not in expect:
+            _error_extra_key(path, key)
+            is_ok = False
+        else:
+            sub_error = compare_json(path + [key], actual[key], expect[key])
+            if sub_error:
+                is_ok = False
+
+    for key in expect:
+        if key not in actual:
+            _error_missing_key(path, key)
+            is_ok = False
+
+    return is_ok
+
+
+def _compare_array(path, actual, expect):
+    '''Compare a JSON array.'''
+    is_ok = True
+
+    if not len(actual) == len(expect):
+        _error_length_mismatch(path, len(actual), len(expect))
+        is_ok = False
+
+    for (idx, (a, e)) in enumerate(zip(actual, expect)):
+        sub_error = compare_json(path + [str(idx)], a, e)
+        if sub_error:
+            is_ok = False
+
+    return is_ok
+
+
+def _make_replacements(value):
+    for (old, new) in REPLACEMENTS.values():
+        value = value.replace(old, new)
+    return value
+
+
+def _compare_string(path, actual, expect):
+    '''Compare a JSON string supporting replacements in the expected output.'''
+    expect = _make_replacements(expect)
+
+    if not actual == expect:
+        _error_unexpect_value(path, actual, expect)
+        return False
+    else:
+        print('%s is ok: %s' % (_print_path(path), actual))
+    return True
+
+
+def _compare_number(path, actual, expect):
+    '''Compare a JSON integer.'''
+    if not actual == expect:
+        _error_unexpect_value(path, actual, expect)
+        return False
+    else:
+        print('%s is ok: %s' % (_print_path(path), actual))
+    return True
+
+
+def _inspect_ordering(arr):
+    req_ordering = True
+
+    if not arr:
+        return arr, req_ordering
+
+    if arr[0] == '__P1689_unordered__':
+        arr.pop(0)
+        req_ordering = False
+
+    return arr, req_ordering
+
+
+def compare_json(path, actual, expect):
+    actual_type = type(actual)
+    expect_type = type(expect)
+
+    is_ok = True
+
+    if not actual_type == expect_type:
+        _error_type_mismatch(path, actual_type, expect_type)
+        is_ok = False
+    elif actual_type == dict:
+        is_ok = _compare_object(path, actual, expect)
+    elif actual_type == list:
+        expect, req_ordering = _inspect_ordering(expect)
+        if not req_ordering:
+            actual = set(actual)
+            expect = set(expect)
+        is_ok = _compare_array(path, actual, expect)
+    elif actual_type == str:
+        is_ok = _compare_string(path, actual, expect)
+    elif actual_type == float:
+        is_ok = _compare_number(path, actual, expect)
+    elif actual_type == int:
+        is_ok = _compare_number(path, actual, expect)
+    elif actual_type == bool:
+        is_ok = _compare_number(path, actual, expect)
+    elif actual_type == type(None):
+        pass
+    else:
+        _error_unknown_type(path, actual_type)
+        is_ok = False
+
+    return is_ok
+
+
+def validate_p1689(actual, expect):
+    '''Validate a P1689 file against an expected output file.
+
+    Returns `False` if it fails, `True` if they are the same.
+    '''
+    with open(actual, 'r') as fin:
+        actual_content = fin.read()
+    with open(expect, 'r') as fin:
+        expect_content = fin.read()
+
+    actual_json = json.loads(actual_content)
+    expect_json = json.loads(expect_content)
+
+    return compare_json([], actual_json, expect_json)
+
+
+if __name__ == '__main__':
+    import sys
+
+    actual = None
+    expect = None
+
+    # Parse arguments.
+    args = sys.argv[1:]
+    while args:
+        # Take an argument.
+        arg = args.pop(0)
+
+        # Parse out replacement expressions.
+        if arg == '-r' or arg == '--replace':
+            replacement = args.pop(0)
+            (key, value) = replacement.split('=', maxsplit=1)
+            REPLACEMENTS[key] = value
+        # Flag to change how errors are reported.
+        elif arg == '-A' or arg == '--all':
+            ALL_ERRORS = True
+        # Required arguments.
+        elif arg == '-a' or arg == '--actual':
+            actual = args.pop(0)
+        elif arg == '-e' or arg == '--expect':
+            expect = args.pop(0)
+
+    # Validate that we have the required arguments.
+    if actual is None:
+        raise RuntimeError('missing "actual" file')
+    if expect is None:
+        raise RuntimeError('missing "expect" file')
+
+    # Do the actual work.
+    is_ok = validate_p1689(actual, expect)
+
+    # Fail if errors are found.
+    if not is_ok:
+        sys.exit(1)
diff --git a/gcc/testsuite/lib/modules.exp b/gcc/testsuite/lib/modules.exp
new file mode 100644
index 00000000000..c7cfda6aae4
--- /dev/null
+++ b/gcc/testsuite/lib/modules.exp
@@ -0,0 +1,71 @@
+#   Copyright (C) 1997-2022 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with GCC; see the file COPYING3.  If not see
+# <http://www.gnu.org/licenses/>.
+
+# Verify various kinds of gcov output: line counts, branch percentages,
+# and call return percentages.  None of this is language-specific.
+
+load_lib "target-supports.exp"
+
+#
+# clean-p1689-file -- delete a working file the compiler creates for p1689
+#
+# TESTCASE is the name of the test.
+# SUFFIX is file suffix
+
+proc clean-p1689-file { testcase suffix } {
+    set basename [file tail $testcase]
+    set base [file rootname $basename]
+    remote_file host delete $base.$suffix
+}
+
+#
+# clean-p1689 -- delete the working files the compiler creates for p1689
+#
+# TESTCASE is the name of the test.
+#
+proc clean-p1689 { testcase } {
+    clean-p1689-file $testcase "d"
+    clean-p1689-file $testcase "json"
+}
+
+# Call by dg-final to check a P1689 dependency file
+
+proc run-check-p1689-valid { depfile template } {
+    global srcdir subdir
+    # Extract the test file name from the arguments.
+    set testcase [file rootname [file tail $depfile]]
+
+    verbose "Running P1689 validation for $testcase in $srcdir/$subdir" 2
+    set testcase [remote_download host $testcase]
+
+    set pytest_script "test-p1689.py"
+    if { ![check_effective_target_recent_python3] } {
+      unsupported "$pytest_script python3 is missing"
+      return
+    }
+
+    verbose "running script" 1
+    spawn -noecho python3 $srcdir/$subdir/$pytest_script --all --actual $depfile --expect $srcdir/$subdir/$template
+
+    expect {
+      -re "ERROR: (\[^\r\n\]*)" {
+       fail $expect_out(0,string)
+       exp_continue
+      }
+    }
+
+    clean-p1689 $testcase
+}
diff --git a/libcpp/include/cpplib.h b/libcpp/include/cpplib.h
index a6f0abd894c..6fc907d12a9 100644
--- a/libcpp/include/cpplib.h
+++ b/libcpp/include/cpplib.h
@@ -302,6 +302,9 @@ typedef CPPCHAR_SIGNED_T cppchar_signed_t;
 /* Style of header dependencies to generate.  */
 enum cpp_deps_style { DEPS_NONE = 0, DEPS_USER, DEPS_SYSTEM };
 
+/* Format of header dependencies to generate.  */
+enum cpp_deps_format { DEPS_FMT_NONE = 0, DEPS_FMT_P1689R5 };
+
 /* The possible normalization levels, from most restrictive to least.  */
 enum cpp_normalize_level {
   /* In NFKC.  */
@@ -589,6 +592,9 @@ struct cpp_options
     /* Style of header dependencies to generate.  */
     enum cpp_deps_style style;
 
+    /* Format of header dependencies to generate.  */
+    enum cpp_deps_format format;
+
     /* Assume missing files are generated files.  */
     bool missing_files;
 
@@ -1112,9 +1118,9 @@ extern void cpp_post_options (cpp_reader *);
 extern void cpp_init_iconv (cpp_reader *);
 
 /* Call this to finish preprocessing.  If you requested dependency
-   generation, pass an open stream to write the information to,
-   otherwise NULL.  It is your responsibility to close the stream.  */
-extern void cpp_finish (cpp_reader *, FILE *deps_stream);
+   generation, pass open stream(s) to write the information to,
+   otherwise NULL.  It is your responsibility to close the stream(s).  */
+extern void cpp_finish (cpp_reader *, FILE *deps_stream, FILE *fdeps_stream = NULL);
 
 /* Call this to release the handle at the end of preprocessing.  Any
    use of the handle after this function returns is invalid.  */
diff --git a/libcpp/include/mkdeps.h b/libcpp/include/mkdeps.h
index 920e2791334..33c7437a481 100644
--- a/libcpp/include/mkdeps.h
+++ b/libcpp/include/mkdeps.h
@@ -53,20 +53,29 @@ extern void deps_add_default_target (class mkdeps *, const char *);
 
 /* Adds a module target.  The module name and cmi name are copied.  */
 extern void deps_add_module_target (struct mkdeps *, const char *module,
-				    const char *cmi, bool is_header);
+				    const char *cmi, bool is_header,
+				    bool is_exported);
 
 /* Adds a module dependency.  The module name is copied.  */
 extern void deps_add_module_dep (struct mkdeps *, const char *module);
 
+/* Add an output.  */
+extern void deps_add_output (struct mkdeps *, const char *, bool);
+
 /* Add a dependency (appears on the right side of the colon) to the
    deps list.  Dependencies will be printed in the order that they
    were entered with this function.  By convention, the first
    dependency entered should be the primary source file.  */
 extern void deps_add_dep (class mkdeps *, const char *);
 
-/* Write out a deps buffer to a specified file.  The last argument
-   is the number of columns to word-wrap at (0 means don't wrap).  */
-extern void deps_write (const cpp_reader *, FILE *, unsigned int);
+/* Write out a deps buffer to a specified file.  The third argument
+   is the number of columns to word-wrap at (0 means don't wrap).
+   The last argument indicates whether to output extra information
+   (namely modules).  */
+extern void deps_write (const struct cpp_reader *, FILE *, unsigned int);
+
+/* Write out a deps buffer to a specified file in P1689R5 format.  */
+extern void deps_write_p1689r5 (const struct mkdeps *, FILE *);
 
 /* Write out a deps buffer to a file, in a form that can be read back
    with deps_restore.  Returns nonzero on error, in which case the
diff --git a/libcpp/init.cc b/libcpp/init.cc
index c508f06112a..d34fd6fdeef 100644
--- a/libcpp/init.cc
+++ b/libcpp/init.cc
@@ -855,7 +855,7 @@ read_original_directory (cpp_reader *pfile)
    Maybe it should also reset state, such that you could call
    cpp_start_read with a new filename to restart processing.  */
 void
-cpp_finish (cpp_reader *pfile, FILE *deps_stream)
+cpp_finish (struct cpp_reader *pfile, FILE *deps_stream, FILE *fdeps_stream)
 {
   /* Warn about unused macros before popping the final buffer.  */
   if (CPP_OPTION (pfile, warn_unused_macros))
@@ -869,8 +869,15 @@ cpp_finish (cpp_reader *pfile, FILE *deps_stream)
   while (pfile->buffer)
     _cpp_pop_buffer (pfile);
 
-  if (deps_stream)
-    deps_write (pfile, deps_stream, 72);
+  cpp_deps_format deps_format = CPP_OPTION (pfile, deps.format);
+  if (deps_format == DEPS_FMT_P1689R5 && fdeps_stream)
+    deps_write_p1689r5 (pfile->deps, fdeps_stream);
+
+  if (CPP_OPTION (pfile, deps.style) != DEPS_NONE
+      && deps_stream)
+    {
+      deps_write (pfile, deps_stream, 72);
+    }
 
   /* Report on headers that could use multiple include guards.  */
   if (CPP_OPTION (pfile, print_include_names))
diff --git a/libcpp/mkdeps.cc b/libcpp/mkdeps.cc
index 8f9585c3c0a..8c554499e41 100644
--- a/libcpp/mkdeps.cc
+++ b/libcpp/mkdeps.cc
@@ -81,7 +81,8 @@ public:
   };
 
   mkdeps ()
-    : module_name (NULL), cmi_name (NULL), is_header_unit (false), quote_lwm (0)
+    : primary_output (NULL), module_name (NULL), cmi_name (NULL)
+    , is_header_unit (false), is_exported (false), quote_lwm (0)
   {
   }
   ~mkdeps ()
@@ -90,6 +91,9 @@ public:
 
     for (i = targets.size (); i--;)
       free (const_cast <char *> (targets[i]));
+    free (const_cast <char *> (primary_output));
+    for (i = outputs.size (); i--;)
+      free (const_cast <char *> (outputs[i]));
     for (i = deps.size (); i--;)
       free (const_cast <char *> (deps[i]));
     for (i = vpath.size (); i--;)
@@ -103,6 +107,8 @@ public:
 public:
   vec<const char *> targets;
   vec<const char *> deps;
+  const char * primary_output;
+  vec<const char *> outputs;
   vec<velt> vpath;
   vec<const char *> modules;
 
@@ -110,6 +116,7 @@ public:
   const char *module_name;
   const char *cmi_name;
   bool is_header_unit;
+  bool is_exported;
   unsigned short quote_lwm;
 };
 
@@ -288,6 +295,21 @@ deps_add_default_target (class mkdeps *d, const char *tgt)
     }
 }
 
+/* Adds an output O.  We make a copy, so it need not be a permanent
+   string.  */
+void
+deps_add_output (struct mkdeps *d, const char *o, bool is_primary)
+{
+  o = apply_vpath (d, o);
+  if (is_primary)
+  {
+    if (d->primary_output)
+      d->outputs.push (d->primary_output);
+    d->primary_output = xstrdup (o);
+  } else
+    d->outputs.push (xstrdup (o));
+}
+
 void
 deps_add_dep (class mkdeps *d, const char *t)
 {
@@ -325,12 +347,13 @@ deps_add_vpath (class mkdeps *d, const char *vpath)
 
 void
 deps_add_module_target (struct mkdeps *d, const char *m,
-			const char *cmi, bool is_header_unit)
+			const char *cmi, bool is_header_unit, bool is_exported)
 {
   gcc_assert (!d->module_name);
   
   d->module_name = xstrdup (m);
   d->is_header_unit = is_header_unit;
+  d->is_exported = is_exported;
   d->cmi_name = xstrdup (cmi);
 }
 
@@ -395,10 +418,15 @@ make_write (const cpp_reader *pfile, FILE *fp, unsigned int colmax)
   if (colmax && colmax < 34)
     colmax = 34;
 
+  /* Write out C++ modules information if no other `-fdeps-format=`
+   * option is given. */
+  cpp_deps_format deps_format = CPP_OPTION (pfile, deps.format);
+  bool write_make_modules_deps = deps_format == DEPS_FMT_NONE;
+
   if (d->deps.size ())
     {
       column = make_write_vec (d->targets, fp, 0, colmax, d->quote_lwm);
-      if (CPP_OPTION (pfile, deps.modules) && d->cmi_name)
+      if (write_make_modules_deps && CPP_OPTION (pfile, deps.modules) && d->cmi_name)
 	column = make_write_name (d->cmi_name, fp, column, colmax);
       fputs (":", fp);
       column++;
@@ -412,7 +440,7 @@ make_write (const cpp_reader *pfile, FILE *fp, unsigned int colmax)
   if (!CPP_OPTION (pfile, deps.modules))
     return;
 
-  if (d->modules.size ())
+  if (write_make_modules_deps && d->modules.size ())
     {
       column = make_write_vec (d->targets, fp, 0, colmax, d->quote_lwm);
       if (d->cmi_name)
@@ -423,7 +451,7 @@ make_write (const cpp_reader *pfile, FILE *fp, unsigned int colmax)
       fputs ("\n", fp);
     }
 
-  if (d->module_name)
+  if (write_make_modules_deps && d->module_name)
     {
       if (d->cmi_name)
 	{
@@ -455,7 +483,7 @@ make_write (const cpp_reader *pfile, FILE *fp, unsigned int colmax)
 	}
     }
   
-  if (d->modules.size ())
+  if (write_make_modules_deps && d->modules.size ())
     {
       column = fprintf (fp, "CXX_IMPORTS +=");
       make_write_vec (d->modules, fp, column, colmax, 0, ".c++m");
@@ -468,11 +496,118 @@ make_write (const cpp_reader *pfile, FILE *fp, unsigned int colmax)
 /* Really we should be opening fp here.  */
 
 void
-deps_write (const cpp_reader *pfile, FILE *fp, unsigned int colmax)
+deps_write (const struct cpp_reader *pfile, FILE *fp, unsigned int colmax)
 {
   make_write (pfile, fp, colmax);
 }
 
+static void
+p1689r5_write_filepath (const char *name, FILE *fp)
+{
+  if (cpp_valid_utf8_p (name, strlen (name)))
+    {
+      fputc ('"', fp);
+      for (const char* c = name; *c; c++)
+	{
+	  // Escape control characters.
+	  if (ISCNTRL (*c))
+	    fprintf (fp, "\\u%04x", *c);
+	  // JSON escape characters.
+	  else if (*c == '"' || *c == '\\')
+	    {
+	      fputc ('\\', fp);
+	      fputc (*c, fp);
+	    }
+	  // Everything else.
+	  else
+	    fputc (*c, fp);
+	}
+      fputc ('"', fp);
+    }
+  else
+    {
+      // TODO: print an error
+    }
+}
+
+static void
+p1689r5_write_vec (const mkdeps::vec<const char *> &vec, FILE *fp)
+{
+  for (unsigned ix = 0; ix != vec.size (); ix++)
+    {
+      p1689r5_write_filepath (vec[ix], fp);
+      if (ix < vec.size () - 1)
+	fputc (',', fp);
+      fputc ('\n', fp);
+    }
+}
+
+void
+deps_write_p1689r5 (const struct mkdeps *d, FILE *fp)
+{
+  fputs ("{\n", fp);
+
+  fputs ("\"rules\": [\n", fp);
+  fputs ("{\n", fp);
+
+  if (d->primary_output)
+    {
+      fputs ("\"primary-output\": ", fp);
+      p1689r5_write_filepath (d->primary_output, fp);
+      fputs (",\n", fp);
+    }
+
+  if (d->outputs.size ())
+    {
+      fputs ("\"outputs\": [\n", fp);
+      p1689r5_write_vec (d->outputs, fp);
+      fputs ("],\n", fp);
+    }
+
+  if (d->module_name)
+    {
+      fputs ("\"provides\": [\n", fp);
+      fputs ("{\n", fp);
+
+      fputs ("\"logical-name\": ", fp);
+      p1689r5_write_filepath (d->module_name, fp);
+      fputs (",\n", fp);
+
+      fprintf (fp, "\"is-interface\": %s\n", d->is_exported ? "true" : "false");
+
+      // TODO: header-unit information
+
+      fputs ("}\n", fp);
+      fputs ("],\n", fp);
+    }
+
+  fputs ("\"requires\": [\n", fp);
+  for (size_t i = 0; i < d->modules.size (); i++)
+    {
+      if (i != 0)
+	fputs (",\n", fp);
+      fputs ("{\n", fp);
+
+      fputs ("\"logical-name\": ", fp);
+      p1689r5_write_filepath (d->modules[i], fp);
+      fputs ("\n", fp);
+
+      // TODO: header-unit information
+
+      fputs ("}\n", fp);
+    }
+  fputs ("]\n", fp);
+
+  fputs ("}\n", fp);
+
+  fputs ("],\n", fp);
+
+  fputs ("\"version\": 0,\n", fp);
+  fputs ("\"revision\": 0\n", fp);
+
+  fputs ("}\n", fp);
+}
+
 /* Write out a deps buffer to a file, in a form that can be read back
    with deps_restore.  Returns nonzero on error, in which case the
    error number will be in errno.  */
-- 
2.40.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v6 3/4] c++modules: report imported CMI files as dependencies
  2023-06-06 20:50 [PATCH v6 0/4] P1689R5 support Ben Boeckel
  2023-06-06 20:50 ` [PATCH v6 1/4] libcpp: reject codepoints above 0x10FFFF Ben Boeckel
  2023-06-06 20:50 ` [PATCH v6 2/4] p1689r5: initial support Ben Boeckel
@ 2023-06-06 20:50 ` Ben Boeckel
  2023-06-06 20:50 ` [PATCH v6 4/4] c++modules: report module mapper files as a dependency Ben Boeckel
  2023-06-08 17:59 ` [PATCH v6 0/4] P1689R5 support Maxim Kuvyrkov
  4 siblings, 0 replies; 14+ messages in thread
From: Ben Boeckel @ 2023-06-06 20:50 UTC (permalink / raw)
  To: gcc-patches; +Cc: Ben Boeckel, jason, nathan, fortran, gcc, brad.king

They affect the build, so report them via `-MF` mechanisms.

gcc/cp/

	* module.cc (do_import): Report imported CMI files as
	dependencies.

Signed-off-by: Ben Boeckel <ben.boeckel@kitware.com>
---
 gcc/cp/module.cc | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index c80f139eb82..e88ce0a1818 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -18966,6 +18966,9 @@ module_state::do_import (cpp_reader *reader, bool outermost)
       dump () && dump ("CMI is %s", file);
       if (note_module_cmi_yes || inform_cmi_p)
 	inform (loc, "reading CMI %qs", file);
+      /* Add the CMI file to the dependency tracking. */
+      if (cpp_get_deps (reader))
+	deps_add_dep (cpp_get_deps (reader), file);
       fd = open (file, O_RDONLY | O_CLOEXEC | O_BINARY);
       e = errno;
     }
-- 
2.40.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v6 4/4] c++modules: report module mapper files as a dependency
  2023-06-06 20:50 [PATCH v6 0/4] P1689R5 support Ben Boeckel
                   ` (2 preceding siblings ...)
  2023-06-06 20:50 ` [PATCH v6 3/4] c++modules: report imported CMI files as dependencies Ben Boeckel
@ 2023-06-06 20:50 ` Ben Boeckel
  2023-06-08 17:59 ` [PATCH v6 0/4] P1689R5 support Maxim Kuvyrkov
  4 siblings, 0 replies; 14+ messages in thread
From: Ben Boeckel @ 2023-06-06 20:50 UTC (permalink / raw)
  To: gcc-patches; +Cc: Ben Boeckel, jason, nathan, fortran, gcc, brad.king

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

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 e88ce0a1818..9dbb53d2aaf 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);
@@ -19504,7 +19504,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);
@@ -19620,7 +19620,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.  */
@@ -20090,7 +20090,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)
     {
@@ -20300,7 +20300,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)
-- 
2.40.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v6 0/4] P1689R5 support
  2023-06-06 20:50 [PATCH v6 0/4] P1689R5 support Ben Boeckel
                   ` (3 preceding siblings ...)
  2023-06-06 20:50 ` [PATCH v6 4/4] c++modules: report module mapper files as a dependency Ben Boeckel
@ 2023-06-08 17:59 ` Maxim Kuvyrkov
  2023-06-16 19:48   ` Ben Boeckel
  4 siblings, 1 reply; 14+ messages in thread
From: Maxim Kuvyrkov @ 2023-06-08 17:59 UTC (permalink / raw)
  To: Ben Boeckel; +Cc: gcc-patches, Jason Merrill, nathan, fortran, gcc, brad.king

> On Jun 7, 2023, at 00:50, Ben Boeckel via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> 
> Hi,
> 
> This patch series adds initial support for ISO C++'s [P1689R5][], a
> format for describing C++ module requirements and provisions based on
> the source code. This is required because compiling C++ with modules is
> not embarrassingly parallel and need to be ordered to ensure that
> `import some_module;` can be satisfied in time by making sure that any
> TU with `export import some_module;` is compiled first.

Hi Ben,

This patch series causes ICEs on arm-linux-gnueabihf.  Would you please investigate?  Please let me know if you need any in reproducing these.

=== g++ tests ===

Running g++:g++.dg/modules/modules.exp ...
FAIL: g++.dg/modules/ben-1_a.C -std=c++17 (internal compiler error: Segmentation fault)
FAIL: g++.dg/modules/ben-1_a.C -std=c++17 (test for excess errors)
FAIL: g++.dg/modules/ben-1_a.C -std=c++2a (internal compiler error: Segmentation fault)
FAIL: g++.dg/modules/ben-1_a.C -std=c++2a (test for excess errors)
FAIL: g++.dg/modules/ben-1_a.C -std=c++2b (internal compiler error: Segmentation fault)
FAIL: g++.dg/modules/ben-1_a.C -std=c++2b (test for excess errors)
FAIL: g++.dg/modules/ben-1_a.C module-cmi =partitions/module-import.mod (partitions/module-import.mod)
FAIL: g++.dg/modules/ben-1_b.C -std=c++17 (internal compiler error: Segmentation fault)
FAIL: g++.dg/modules/ben-1_b.C -std=c++17 (test for excess errors)
FAIL: g++.dg/modules/ben-1_b.C -std=c++2a (internal compiler error: Segmentation fault)
FAIL: g++.dg/modules/ben-1_b.C -std=c++2a (test for excess errors)
FAIL: g++.dg/modules/ben-1_b.C -std=c++2b (internal compiler error: Segmentation fault)
FAIL: g++.dg/modules/ben-1_b.C -std=c++2b (test for excess errors)
FAIL: g++.dg/modules/ben-1_b.C module-cmi =module.mod (module.mod)
FAIL: g++.dg/modules/gc-2_a.C -std=c++17 (internal compiler error: Segmentation fault)
FAIL: g++.dg/modules/gc-2_a.C -std=c++17 (test for excess errors)
FAIL: g++.dg/modules/gc-2_a.C -std=c++2a (internal compiler error: Segmentation fault)
FAIL: g++.dg/modules/gc-2_a.C -std=c++2a (test for excess errors)
FAIL: g++.dg/modules/gc-2_a.C -std=c++2b (internal compiler error: Segmentation fault)
FAIL: g++.dg/modules/gc-2_a.C -std=c++2b (test for excess errors)
FAIL: g++.dg/modules/gc-2_a.C module-cmi =map-1_a.nms (map-1_a.nms)
UNRESOLVED: g++.dg/modules/map-1 -std=c++17 execute
UNRESOLVED: g++.dg/modules/map-1 -std=c++17 link
UNRESOLVED: g++.dg/modules/map-1 -std=c++2a execute
UNRESOLVED: g++.dg/modules/map-1 -std=c++2a link
UNRESOLVED: g++.dg/modules/map-1 -std=c++2b execute
UNRESOLVED: g++.dg/modules/map-1 -std=c++2b link
FAIL: g++.dg/modules/map-1_a.C -std=c++17 (internal compiler error: Segmentation fault)
FAIL: g++.dg/modules/map-1_a.C -std=c++17 (test for excess errors)
FAIL: g++.dg/modules/map-1_a.C -std=c++2a (internal compiler error: Segmentation fault)
FAIL: g++.dg/modules/map-1_a.C -std=c++2a (test for excess errors)
FAIL: g++.dg/modules/map-1_a.C -std=c++2b (internal compiler error: Segmentation fault)
FAIL: g++.dg/modules/map-1_a.C -std=c++2b (test for excess errors)
FAIL: g++.dg/modules/map-1_a.C module-cmi =map-1_a.nms (map-1_a.nms)
FAIL: g++.dg/modules/map-1_b.C -std=c++17 (internal compiler error: Segmentation fault)
FAIL: g++.dg/modules/map-1_b.C -std=c++17 (test for excess errors)
FAIL: g++.dg/modules/map-1_b.C -std=c++2a (internal compiler error: Segmentation fault)
FAIL: g++.dg/modules/map-1_b.C -std=c++2a (test for excess errors)
FAIL: g++.dg/modules/map-1_b.C -std=c++2b (internal compiler error: Segmentation fault)
FAIL: g++.dg/modules/map-1_b.C -std=c++2b (test for excess errors)
FAIL: g++.dg/modules/map-2.C -std=c++17 at line 8 (test for errors, line 7)
FAIL: g++.dg/modules/map-2.C -std=c++17 at line 9 (test for errors, line )
FAIL: g++.dg/modules/map-2.C -std=c++17 (internal compiler error: Segmentation fault)
FAIL: g++.dg/modules/map-2.C -std=c++17 (test for excess errors)
FAIL: g++.dg/modules/map-2.C -std=c++2a at line 8 (test for errors, line 7)
FAIL: g++.dg/modules/map-2.C -std=c++2a at line 9 (test for errors, line )
FAIL: g++.dg/modules/map-2.C -std=c++2a (internal compiler error: Segmentation fault)
FAIL: g++.dg/modules/map-2.C -std=c++2a (test for excess errors)
FAIL: g++.dg/modules/map-2.C -std=c++2b at line 8 (test for errors, line 7)
FAIL: g++.dg/modules/map-2.C -std=c++2b at line 9 (test for errors, line )
FAIL: g++.dg/modules/map-2.C -std=c++2b (internal compiler error: Segmentation fault)
FAIL: g++.dg/modules/map-2.C -std=c++2b (test for excess errors)
===

Thanks,

--
Maxim Kuvyrkov
https://www.linaro.org





> 
> [P1689R5]: https://isocpp.org/files/papers/P1689R5.html
> 
> I've also added patches to include imported module CMI files and the
> module mapper file as dependencies of the compilation. I briefly looked
> into adding dependencies on response files as well, but that appeared to
> need some code contortions to have a `class mkdeps` available before
> parsing the command line or to keep the information around until one was
> made.
> 
> I'd like feedback on the approach taken here with respect to the
> user-visible flags. I'll also note that header units are not supported
> at this time because the current `-E` behavior with respect to `import
> <some_header>;` is to search for an appropriate `.gcm` file which is not
> something such a "scan" can support. A new mode will likely need to be
> created (e.g., replacing `-E` with `-fc++-module-scanning` or something)
> where headers are looked up "normally" and processed only as much as
> scanning requires.
> 
> FWIW, Clang as taken an alternate approach with its `clang-scan-deps`
> tool rather than using the compiler directly.
> 
> Thanks,
> 
> --Ben
> 
> ---
> v5 -> v6:
> 
> - rebase onto `master` (585c660f041 (reload1: Change return type of
>  predicate function from int to bool, 2023-06-06))
> - fix crash related to reporting imported CMI files as dependencies
> - rework utf-8 validity to patch the new `cpp_valid_utf8_p` function
>  instead of the core utf-8 decoding routine to reject invalid
>  codepoints (preserves higher-level error detection of invalid utf-8)
> - harmonize of `fdeps` spelling in flags, variables, comments, etc.
> - rename `-fdeps-output=` to `-fdeps-target=`
> 
> v4 -> v5:
> 
> - add dependency tracking for imported modules to `-MF`
> - add dependency tracking for static module mapper files given to
>  `-fmodule-mapper=`
> 
> v3 -> v4:
> 
> - add missing spaces between function names and arguments
> 
> v2 -> v3:
> 
> - changelog entries moved to commit messages
> - documentation updated/added in the UTF-8 routine editing
> 
> v1 -> v2:
> 
> - removal of the `deps_write(extra)` parameter to option-checking where
>  ndeeded
> - default parameter of `cpp_finish(fdeps_stream = NULL)`
> - unification of libcpp UTF-8 validity functions from v1
> - test cases for flag parsing states (depflags-*) and p1689 output
>  (p1689-*)
> 
> Ben Boeckel (4):
>  libcpp: reject codepoints above 0x10FFFF
>  p1689r5: initial support
>  c++modules: report imported CMI files as dependencies
>  c++modules: report module mapper files as a dependency
> 
> gcc/c-family/c-opts.cc                        |  40 +++-
> gcc/c-family/c.opt                            |  12 +
> gcc/cp/mapper-client.cc                       |   4 +
> gcc/cp/mapper-client.h                        |   1 +
> gcc/cp/module.cc                              |  24 +-
> gcc/doc/invoke.texi                           |  15 ++
> gcc/testsuite/g++.dg/modules/depflags-f-MD.C  |   2 +
> gcc/testsuite/g++.dg/modules/depflags-f.C     |   1 +
> gcc/testsuite/g++.dg/modules/depflags-fi.C    |   3 +
> gcc/testsuite/g++.dg/modules/depflags-fj-MD.C |   3 +
> gcc/testsuite/g++.dg/modules/depflags-fj.C    |   4 +
> .../g++.dg/modules/depflags-fjo-MD.C          |   4 +
> gcc/testsuite/g++.dg/modules/depflags-fjo.C   |   5 +
> gcc/testsuite/g++.dg/modules/depflags-fo-MD.C |   3 +
> gcc/testsuite/g++.dg/modules/depflags-fo.C    |   4 +
> gcc/testsuite/g++.dg/modules/depflags-j-MD.C  |   2 +
> gcc/testsuite/g++.dg/modules/depflags-j.C     |   3 +
> gcc/testsuite/g++.dg/modules/depflags-jo-MD.C |   3 +
> gcc/testsuite/g++.dg/modules/depflags-jo.C    |   4 +
> gcc/testsuite/g++.dg/modules/depflags-o-MD.C  |   2 +
> gcc/testsuite/g++.dg/modules/depflags-o.C     |   3 +
> gcc/testsuite/g++.dg/modules/modules.exp      |   1 +
> gcc/testsuite/g++.dg/modules/p1689-1.C        |  18 ++
> gcc/testsuite/g++.dg/modules/p1689-1.exp.json |  27 +++
> gcc/testsuite/g++.dg/modules/p1689-2.C        |  16 ++
> gcc/testsuite/g++.dg/modules/p1689-2.exp.json |  16 ++
> gcc/testsuite/g++.dg/modules/p1689-3.C        |  14 ++
> gcc/testsuite/g++.dg/modules/p1689-3.exp.json |  16 ++
> gcc/testsuite/g++.dg/modules/p1689-4.C        |  14 ++
> gcc/testsuite/g++.dg/modules/p1689-4.exp.json |  14 ++
> gcc/testsuite/g++.dg/modules/p1689-5.C        |  14 ++
> gcc/testsuite/g++.dg/modules/p1689-5.exp.json |  14 ++
> gcc/testsuite/g++.dg/modules/test-p1689.py    | 222 ++++++++++++++++++
> gcc/testsuite/lib/modules.exp                 |  71 ++++++
> libcpp/charset.cc                             |   7 +
> libcpp/include/cpplib.h                       |  12 +-
> libcpp/include/mkdeps.h                       |  17 +-
> libcpp/init.cc                                |  13 +-
> libcpp/mkdeps.cc                              | 149 +++++++++++-
> 39 files changed, 769 insertions(+), 28 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/modules/depflags-f-MD.C
> create mode 100644 gcc/testsuite/g++.dg/modules/depflags-f.C
> create mode 100644 gcc/testsuite/g++.dg/modules/depflags-fi.C
> create mode 100644 gcc/testsuite/g++.dg/modules/depflags-fj-MD.C
> create mode 100644 gcc/testsuite/g++.dg/modules/depflags-fj.C
> create mode 100644 gcc/testsuite/g++.dg/modules/depflags-fjo-MD.C
> create mode 100644 gcc/testsuite/g++.dg/modules/depflags-fjo.C
> create mode 100644 gcc/testsuite/g++.dg/modules/depflags-fo-MD.C
> create mode 100644 gcc/testsuite/g++.dg/modules/depflags-fo.C
> create mode 100644 gcc/testsuite/g++.dg/modules/depflags-j-MD.C
> create mode 100644 gcc/testsuite/g++.dg/modules/depflags-j.C
> create mode 100644 gcc/testsuite/g++.dg/modules/depflags-jo-MD.C
> create mode 100644 gcc/testsuite/g++.dg/modules/depflags-jo.C
> create mode 100644 gcc/testsuite/g++.dg/modules/depflags-o-MD.C
> create mode 100644 gcc/testsuite/g++.dg/modules/depflags-o.C
> create mode 100644 gcc/testsuite/g++.dg/modules/p1689-1.C
> create mode 100644 gcc/testsuite/g++.dg/modules/p1689-1.exp.json
> create mode 100644 gcc/testsuite/g++.dg/modules/p1689-2.C
> create mode 100644 gcc/testsuite/g++.dg/modules/p1689-2.exp.json
> create mode 100644 gcc/testsuite/g++.dg/modules/p1689-3.C
> create mode 100644 gcc/testsuite/g++.dg/modules/p1689-3.exp.json
> create mode 100644 gcc/testsuite/g++.dg/modules/p1689-4.C
> create mode 100644 gcc/testsuite/g++.dg/modules/p1689-4.exp.json
> create mode 100644 gcc/testsuite/g++.dg/modules/p1689-5.C
> create mode 100644 gcc/testsuite/g++.dg/modules/p1689-5.exp.json
> create mode 100644 gcc/testsuite/g++.dg/modules/test-p1689.py
> create mode 100644 gcc/testsuite/lib/modules.exp
> 
> 
> base-commit: 585c660f041c57694a61b3e65085af5a54431be4
> -- 
> 2.40.1
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v6 0/4] P1689R5 support
  2023-06-08 17:59 ` [PATCH v6 0/4] P1689R5 support Maxim Kuvyrkov
@ 2023-06-16 19:48   ` Ben Boeckel
  2023-06-16 23:15     ` Ben Boeckel
  2023-06-17  3:55     ` Jason Merrill
  0 siblings, 2 replies; 14+ messages in thread
From: Ben Boeckel @ 2023-06-16 19:48 UTC (permalink / raw)
  To: Maxim Kuvyrkov
  Cc: gcc-patches, Jason Merrill, nathan, fortran, gcc, brad.king

On Thu, Jun 08, 2023 at 21:59:13 +0400, Maxim Kuvyrkov wrote:
> This patch series causes ICEs on arm-linux-gnueabihf.  Would you
> please investigate?  Please let me know if you need any in reproducing
> these.

Finally back at it. I tried on aarch64, but wasn't able to reproduce the
errors (alas, it is probably a 32bit thing…let me try with `-m32`). Is
there hardware I can access to try this out on the same target triple?

Alternatively, a backtrace may be able to help pinpoint it enough if you
have the cycles.

Thanks,

--Ben

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v6 0/4] P1689R5 support
  2023-06-16 19:48   ` Ben Boeckel
@ 2023-06-16 23:15     ` Ben Boeckel
  2023-06-17  3:55     ` Jason Merrill
  1 sibling, 0 replies; 14+ messages in thread
From: Ben Boeckel @ 2023-06-16 23:15 UTC (permalink / raw)
  To: Maxim Kuvyrkov
  Cc: gcc-patches, Jason Merrill, nathan, fortran, gcc, brad.king

On Fri, Jun 16, 2023 at 15:48:59 -0400, Ben Boeckel wrote:
> On Thu, Jun 08, 2023 at 21:59:13 +0400, Maxim Kuvyrkov wrote:
> > This patch series causes ICEs on arm-linux-gnueabihf.  Would you
> > please investigate?  Please let me know if you need any in reproducing
> > these.
> 
> Finally back at it. I tried on aarch64, but wasn't able to reproduce the
> errors (alas, it is probably a 32bit thing…let me try with `-m32`). Is
> there hardware I can access to try this out on the same target triple?

Trying inside of an i386 container also came up with nothing…I'll try
qemu.

--Ben

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v6 0/4] P1689R5 support
  2023-06-16 19:48   ` Ben Boeckel
  2023-06-16 23:15     ` Ben Boeckel
@ 2023-06-17  3:55     ` Jason Merrill
  2023-06-17 14:43       ` Ben Boeckel
  1 sibling, 1 reply; 14+ messages in thread
From: Jason Merrill @ 2023-06-17  3:55 UTC (permalink / raw)
  To: Ben Boeckel; +Cc: Maxim Kuvyrkov, gcc-patches, nathan, fortran, gcc, brad.king

On Fri, Jun 16, 2023 at 3:49 PM Ben Boeckel <ben.boeckel@kitware.com> wrote:
>
> On Thu, Jun 08, 2023 at 21:59:13 +0400, Maxim Kuvyrkov wrote:
> > This patch series causes ICEs on arm-linux-gnueabihf.  Would you
> > please investigate?  Please let me know if you need any in reproducing
> > these.
>
> Finally back at it. I tried on aarch64, but wasn't able to reproduce the
> errors (alas, it is probably a 32bit thing…let me try with `-m32`). Is
> there hardware I can access to try this out on the same target triple?
>
> Alternatively, a backtrace may be able to help pinpoint it enough if you
> have the cycles.

I see the same thing with patch 4 on x86_64-pc-linux-gnu, e.g.

FAIL: g++.dg/modules/ben-1_a.C -std=c++17 (test for excess errors)
Excess errors:
/home/jason/gt/gcc/testsuite/g++.dg/modules/ben-1_a.C:9:1: internal
compiler error: Segmentation fault
0x19e2f3c crash_signal
        /home/jason/gt/gcc/toplev.cc:314
0x340f3f8 mkdeps::vec<mkdeps::velt>::size() const
        /home/jason/gt/libcpp/mkdeps.cc:57
0x340dc1f apply_vpath
        /home/jason/gt/libcpp/mkdeps.cc:194
0x340e08e deps_add_dep(mkdeps*, char const*)
        /home/jason/gt/libcpp/mkdeps.cc:318
0xea7b51 module_client::open_module_client(unsigned int, char const*,
mkdeps*, void (*)(char const*), char const*)
        /home/jason/gt/gcc/cp/mapper-client.cc:291
0xef2ba8 make_mapper
        /home/jason/gt/gcc/cp/module.cc:14042
0xf0896c get_mapper(unsigned int, mkdeps*)
        /home/jason/gt/gcc/cp/module.cc:3977
0xf032ac name_pending_imports
        /home/jason/gt/gcc/cp/module.cc:19623
0xf03a7d preprocessed_module(cpp_reader*)
        /home/jason/gt/gcc/cp/module.cc:19817
0xe85104 module_token_cdtor(cpp_reader*, unsigned long)
        /home/jason/gt/gcc/cp/lex.cc:548
0xf467b2 cp_lexer_new_main
        /home/jason/gt/gcc/cp/parser.cc:756
0xfc1e3a c_parse_file()
        /home/jason/gt/gcc/cp/parser.cc:49725
0x11c5bf5 c_common_parse_file()
        /home/jason/gt/gcc/c-family/c-opts.cc:1268


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v6 0/4] P1689R5 support
  2023-06-17  3:55     ` Jason Merrill
@ 2023-06-17 14:43       ` Ben Boeckel
  2023-06-20  0:54         ` Jason Merrill
  0 siblings, 1 reply; 14+ messages in thread
From: Ben Boeckel @ 2023-06-17 14:43 UTC (permalink / raw)
  To: Jason Merrill
  Cc: Maxim Kuvyrkov, gcc-patches, nathan, fortran, gcc, brad.king

On Fri, Jun 16, 2023 at 23:55:53 -0400, Jason Merrill wrote:
> I see the same thing with patch 4 on x86_64-pc-linux-gnu, e.g.
> 
> FAIL: g++.dg/modules/ben-1_a.C -std=c++17 (test for excess errors)
> Excess errors:
> /home/jason/gt/gcc/testsuite/g++.dg/modules/ben-1_a.C:9:1: internal
> compiler error: Segmentation fault
> 0x19e2f3c crash_signal
>         /home/jason/gt/gcc/toplev.cc:314
> 0x340f3f8 mkdeps::vec<mkdeps::velt>::size() const
>         /home/jason/gt/libcpp/mkdeps.cc:57
> 0x340dc1f apply_vpath
>         /home/jason/gt/libcpp/mkdeps.cc:194
> 0x340e08e deps_add_dep(mkdeps*, char const*)
>         /home/jason/gt/libcpp/mkdeps.cc:318
> 0xea7b51 module_client::open_module_client(unsigned int, char const*,
> mkdeps*, void (*)(char const*), char const*)
>         /home/jason/gt/gcc/cp/mapper-client.cc:291
> 0xef2ba8 make_mapper
>         /home/jason/gt/gcc/cp/module.cc:14042
> 0xf0896c get_mapper(unsigned int, mkdeps*)
>         /home/jason/gt/gcc/cp/module.cc:3977
> 0xf032ac name_pending_imports
>         /home/jason/gt/gcc/cp/module.cc:19623
> 0xf03a7d preprocessed_module(cpp_reader*)
>         /home/jason/gt/gcc/cp/module.cc:19817
> 0xe85104 module_token_cdtor(cpp_reader*, unsigned long)
>         /home/jason/gt/gcc/cp/lex.cc:548
> 0xf467b2 cp_lexer_new_main
>         /home/jason/gt/gcc/cp/parser.cc:756
> 0xfc1e3a c_parse_file()
>         /home/jason/gt/gcc/cp/parser.cc:49725
> 0x11c5bf5 c_common_parse_file()
>         /home/jason/gt/gcc/c-family/c-opts.cc:1268

Thanks. I missed a `nullptr` check before calling `deps_add_dep`. I
think I got misled by `make check` returning a zero exit code even if
there are failures.

Thanks,

--Ben

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v6 1/4] libcpp: reject codepoints above 0x10FFFF
  2023-06-06 20:50 ` [PATCH v6 1/4] libcpp: reject codepoints above 0x10FFFF Ben Boeckel
@ 2023-06-19 21:34   ` Jason Merrill
  2023-06-20 19:16     ` Damien Guibouret
  0 siblings, 1 reply; 14+ messages in thread
From: Jason Merrill @ 2023-06-19 21:34 UTC (permalink / raw)
  To: Ben Boeckel, gcc-patches; +Cc: nathan, fortran, gcc, brad.king

On 6/6/23 16:50, Ben Boeckel wrote:
> Unicode does not support such values because they are unrepresentable in
> UTF-16.

Pushed.

> libcpp/
> 
> 	* charset.cc: Reject encodings of codepoints above 0x10FFFF.
> 	UTF-16 does not support such codepoints and therefore all
> 	Unicode rejects such values.
> 
> Signed-off-by: Ben Boeckel <ben.boeckel@kitware.com>
> ---
>   libcpp/charset.cc | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/libcpp/charset.cc b/libcpp/charset.cc
> index d7f323b2cd5..3b34d804cf1 100644
> --- a/libcpp/charset.cc
> +++ b/libcpp/charset.cc
> @@ -1886,6 +1886,13 @@ cpp_valid_utf8_p (const char *buffer, size_t num_bytes)
>         int err = one_utf8_to_cppchar (&iter, &bytesleft, &cp);
>         if (err)
>   	return false;
> +
> +      /* Additionally, Unicode declares that all codepoints above 0010FFFF are
> +	 invalid because they cannot be represented in UTF-16.
> +
> +	 Reject such values.*/
> +      if (cp >= 0x10FFFF)
> +	return false;
>       }
>     /* No problems encountered.  */
>     return true;


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v6 0/4] P1689R5 support
  2023-06-17 14:43       ` Ben Boeckel
@ 2023-06-20  0:54         ` Jason Merrill
  0 siblings, 0 replies; 14+ messages in thread
From: Jason Merrill @ 2023-06-20  0:54 UTC (permalink / raw)
  To: Ben Boeckel; +Cc: Maxim Kuvyrkov, gcc-patches, nathan, fortran, gcc, brad.king

On 6/17/23 10:43, Ben Boeckel wrote:
> On Fri, Jun 16, 2023 at 23:55:53 -0400, Jason Merrill wrote:
>> I see the same thing with patch 4 on x86_64-pc-linux-gnu, e.g.
>>
>> FAIL: g++.dg/modules/ben-1_a.C -std=c++17 (test for excess errors)
>> Excess errors:
>> /home/jason/gt/gcc/testsuite/g++.dg/modules/ben-1_a.C:9:1: internal
>> compiler error: Segmentation fault
>> 0x19e2f3c crash_signal
>>          /home/jason/gt/gcc/toplev.cc:314
>> 0x340f3f8 mkdeps::vec<mkdeps::velt>::size() const
>>          /home/jason/gt/libcpp/mkdeps.cc:57
>> 0x340dc1f apply_vpath
>>          /home/jason/gt/libcpp/mkdeps.cc:194
>> 0x340e08e deps_add_dep(mkdeps*, char const*)
>>          /home/jason/gt/libcpp/mkdeps.cc:318
>> 0xea7b51 module_client::open_module_client(unsigned int, char const*,
>> mkdeps*, void (*)(char const*), char const*)
>>          /home/jason/gt/gcc/cp/mapper-client.cc:291
>> 0xef2ba8 make_mapper
>>          /home/jason/gt/gcc/cp/module.cc:14042
>> 0xf0896c get_mapper(unsigned int, mkdeps*)
>>          /home/jason/gt/gcc/cp/module.cc:3977
>> 0xf032ac name_pending_imports
>>          /home/jason/gt/gcc/cp/module.cc:19623
>> 0xf03a7d preprocessed_module(cpp_reader*)
>>          /home/jason/gt/gcc/cp/module.cc:19817
>> 0xe85104 module_token_cdtor(cpp_reader*, unsigned long)
>>          /home/jason/gt/gcc/cp/lex.cc:548
>> 0xf467b2 cp_lexer_new_main
>>          /home/jason/gt/gcc/cp/parser.cc:756
>> 0xfc1e3a c_parse_file()
>>          /home/jason/gt/gcc/cp/parser.cc:49725
>> 0x11c5bf5 c_common_parse_file()
>>          /home/jason/gt/gcc/c-family/c-opts.cc:1268
> 
> Thanks. I missed a `nullptr` check before calling `deps_add_dep`. I
> think I got misled by `make check` returning a zero exit code even if
> there are failures.

Aha!

Patches 3 and 4 could also use testcases.

Jason


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v6 1/4] libcpp: reject codepoints above 0x10FFFF
  2023-06-19 21:34   ` Jason Merrill
@ 2023-06-20 19:16     ` Damien Guibouret
  2023-06-20 19:49       ` Ben Boeckel
  0 siblings, 1 reply; 14+ messages in thread
From: Damien Guibouret @ 2023-06-20 19:16 UTC (permalink / raw)
  To: Jason Merrill, gcc

Le 19/06/2023 à 23:34, Jason Merrill a écrit :
> On 6/6/23 16:50, Ben Boeckel wrote:
>> Unicode does not support such values because they are unrepresentable in
>> UTF-16.
> 
> Pushed.
> 
>> libcpp/
>>
>>     * charset.cc: Reject encodings of codepoints above 0x10FFFF.
>>     UTF-16 does not support such codepoints and therefore all
>>     Unicode rejects such values.
>>
>> Signed-off-by: Ben Boeckel <ben.boeckel@kitware.com>
>> ---
>>   libcpp/charset.cc | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/libcpp/charset.cc b/libcpp/charset.cc
>> index d7f323b2cd5..3b34d804cf1 100644
>> --- a/libcpp/charset.cc
>> +++ b/libcpp/charset.cc
>> @@ -1886,6 +1886,13 @@ cpp_valid_utf8_p (const char *buffer, size_t 
>> num_bytes)
>>         int err = one_utf8_to_cppchar (&iter, &bytesleft, &cp);
>>         if (err)
>>       return false;
>> +
>> +      /* Additionally, Unicode declares that all codepoints above 
>> 0010FFFF are
>> +     invalid because they cannot be represented in UTF-16.
>> +
>> +     Reject such values.*/
>> +      if (cp >= 0x10FFFF)
>> +    return false;
>>       }
>>     /* No problems encountered.  */
>>     return true;
> 

Hello,

I think the comparison should be ">" instead of ">=" as 0x10ffff seems a 
valid value (Unicode says value above 0x10ffff is invalid).
Other tests around same value in this file are using ">".

Regards,

Damien

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v6 1/4] libcpp: reject codepoints above 0x10FFFF
  2023-06-20 19:16     ` Damien Guibouret
@ 2023-06-20 19:49       ` Ben Boeckel
  0 siblings, 0 replies; 14+ messages in thread
From: Ben Boeckel @ 2023-06-20 19:49 UTC (permalink / raw)
  To: Damien Guibouret; +Cc: Jason Merrill, gcc

On Tue, Jun 20, 2023 at 21:16:40 +0200, Damien Guibouret wrote:
> I think the comparison should be ">" instead of ">=" as 0x10ffff seems a 
> valid value (Unicode says value above 0x10ffff is invalid).
> Other tests around same value in this file are using ">".

Ah, good catch. I'll make a separate patch submission for that.

--Ben

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2023-06-20 19:49 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-06 20:50 [PATCH v6 0/4] P1689R5 support Ben Boeckel
2023-06-06 20:50 ` [PATCH v6 1/4] libcpp: reject codepoints above 0x10FFFF Ben Boeckel
2023-06-19 21:34   ` Jason Merrill
2023-06-20 19:16     ` Damien Guibouret
2023-06-20 19:49       ` Ben Boeckel
2023-06-06 20:50 ` [PATCH v6 2/4] p1689r5: initial support Ben Boeckel
2023-06-06 20:50 ` [PATCH v6 3/4] c++modules: report imported CMI files as dependencies Ben Boeckel
2023-06-06 20:50 ` [PATCH v6 4/4] c++modules: report module mapper files as a dependency Ben Boeckel
2023-06-08 17:59 ` [PATCH v6 0/4] P1689R5 support Maxim Kuvyrkov
2023-06-16 19:48   ` Ben Boeckel
2023-06-16 23:15     ` Ben Boeckel
2023-06-17  3:55     ` Jason Merrill
2023-06-17 14:43       ` Ben Boeckel
2023-06-20  0:54         ` Jason Merrill

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).