[v7,0/4] P1689R5 support

Message ID 20230702163211.3396210-1-ben.boeckel@kitware.com
Headers
Series P1689R5 support |

Message

Ben Boeckel July 2, 2023, 4:32 p.m. UTC
  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

---
v6 -> v7:

- rebase onto `master` (80ae426a195 (d: Fix core.volatile.volatileLoad
  discarded if result is unused, 2023-07-02))
- add test cases for patches 3 and 4 (new dependency reporting in `-MF`)
- add a Python script to test aspects of generated dependency files
- a new `join` spec function to support `-fdeps-*` defaults based on the
  `-o` flag (needed to strip the leading space that appears otherwise)
- note that JSON writing support should be factored out for use by
  `libcpp` and `gcc` (libiberty?)
- use `.ddi` for the extension of `-fdeps-*` output files by default
- support defaults for `-fdeps-file=` and `-fdeps-target=` when only
  `-fdeps-format=` is provided (with tests)
- error if `-MF` and `-fdeps-file=` are both the same (non-`stdout`)
  file as their formats are incompatible
- expand the documentation on how the `-fdeps-*` flags should be used

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):
  spec: add a spec function to join arguments
  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                        |  44 +++-
 gcc/c-family/c.opt                            |  12 +
 gcc/cp/mapper-client.cc                       |   5 +
 gcc/cp/mapper-client.h                        |   1 +
 gcc/cp/module.cc                              |  24 +-
 gcc/doc/invoke.texi                           |  27 +++
 gcc/gcc.cc                                    |  19 +-
 gcc/json.h                                    |   3 +
 gcc/testsuite/g++.dg/modules/depflags-f-MD.C  |   2 +
 gcc/testsuite/g++.dg/modules/depflags-f.C     |   3 +
 gcc/testsuite/g++.dg/modules/depflags-fi.C    |   4 +
 gcc/testsuite/g++.dg/modules/depflags-fj-MD.C |   3 +
 .../g++.dg/modules/depflags-fj-MF-share.C     |   6 +
 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/depreport-1_a.C  |  10 +
 gcc/testsuite/g++.dg/modules/depreport-1_b.C  |  12 +
 .../g++.dg/modules/depreport-2.modmap         |   2 +
 gcc/testsuite/g++.dg/modules/depreport-2_a.C  |  15 ++
 gcc/testsuite/g++.dg/modules/depreport-2_b.C  |  14 ++
 gcc/testsuite/g++.dg/modules/modules.exp      |   1 +
 gcc/testsuite/g++.dg/modules/p1689-1.C        |  17 ++
 gcc/testsuite/g++.dg/modules/p1689-1.exp.ddi  |  27 +++
 gcc/testsuite/g++.dg/modules/p1689-2.C        |  15 ++
 gcc/testsuite/g++.dg/modules/p1689-2.exp.ddi  |  16 ++
 gcc/testsuite/g++.dg/modules/p1689-3.C        |  13 +
 gcc/testsuite/g++.dg/modules/p1689-3.exp.ddi  |  16 ++
 gcc/testsuite/g++.dg/modules/p1689-4.C        |  13 +
 gcc/testsuite/g++.dg/modules/p1689-4.exp.ddi  |  14 ++
 gcc/testsuite/g++.dg/modules/p1689-5.C        |  13 +
 gcc/testsuite/g++.dg/modules/p1689-5.exp.ddi  |  14 ++
 .../g++.dg/modules/p1689-file-default.C       |  16 ++
 .../g++.dg/modules/p1689-file-default.exp.ddi |  27 +++
 .../g++.dg/modules/p1689-target-default.C     |  16 ++
 .../modules/p1689-target-default.exp.ddi      |  27 +++
 gcc/testsuite/g++.dg/modules/test-depfile.py  | 207 ++++++++++++++++
 gcc/testsuite/g++.dg/modules/test-p1689.py    | 222 ++++++++++++++++++
 gcc/testsuite/lib/modules.exp                 | 100 ++++++++
 libcpp/include/cpplib.h                       |  12 +-
 libcpp/include/mkdeps.h                       |   9 +-
 libcpp/init.cc                                |  13 +-
 libcpp/mkdeps.cc                              | 153 +++++++++++-
 51 files changed, 1181 insertions(+), 23 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-MF-share.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/depreport-1_a.C
 create mode 100644 gcc/testsuite/g++.dg/modules/depreport-1_b.C
 create mode 100644 gcc/testsuite/g++.dg/modules/depreport-2.modmap
 create mode 100644 gcc/testsuite/g++.dg/modules/depreport-2_a.C
 create mode 100644 gcc/testsuite/g++.dg/modules/depreport-2_b.C
 create mode 100644 gcc/testsuite/g++.dg/modules/p1689-1.C
 create mode 100644 gcc/testsuite/g++.dg/modules/p1689-1.exp.ddi
 create mode 100644 gcc/testsuite/g++.dg/modules/p1689-2.C
 create mode 100644 gcc/testsuite/g++.dg/modules/p1689-2.exp.ddi
 create mode 100644 gcc/testsuite/g++.dg/modules/p1689-3.C
 create mode 100644 gcc/testsuite/g++.dg/modules/p1689-3.exp.ddi
 create mode 100644 gcc/testsuite/g++.dg/modules/p1689-4.C
 create mode 100644 gcc/testsuite/g++.dg/modules/p1689-4.exp.ddi
 create mode 100644 gcc/testsuite/g++.dg/modules/p1689-5.C
 create mode 100644 gcc/testsuite/g++.dg/modules/p1689-5.exp.ddi
 create mode 100644 gcc/testsuite/g++.dg/modules/p1689-file-default.C
 create mode 100644 gcc/testsuite/g++.dg/modules/p1689-file-default.exp.ddi
 create mode 100644 gcc/testsuite/g++.dg/modules/p1689-target-default.C
 create mode 100644 gcc/testsuite/g++.dg/modules/p1689-target-default.exp.ddi
 create mode 100644 gcc/testsuite/g++.dg/modules/test-depfile.py
 create mode 100644 gcc/testsuite/g++.dg/modules/test-p1689.py
 create mode 100644 gcc/testsuite/lib/modules.exp


base-commit: 80ae426a195a0d035640a6301da833564deade52
  

Comments

Jason Merrill Aug. 23, 2023, 7:29 p.m. UTC | #1
On 7/2/23 12:32, Ben Boeckel wrote:
> When passing `-o` flags to other options, the typical `-o foo` spelling
> leaves a leading whitespace when replacing elsewhere. This ends up
> creating flags spelled as `-some-option-with-arg= foo.ext` which doesn't
> parse properly. When attempting to make a spec function to just remove
> the leading whitespace, the argument splitting ends up masking the
> whitespace. However, the intended extension *also* ends up being its own
> argument.

Odd.  I looked into this to figure out what was going on, and now I 
understand: when process_brace_body handles e.g. %{o*:-fjoined=%.x%*}, 
first it replaces $* with the rest of the flag, i.e. "", resulting in 
-fjoined=, and then adds the argument as a separate argument to the 
result of substitution.  This seems strange, but works fine for the 
existing uses that build one Separate switch from another.

The other oddity you mention comes from

>   /* End of string.  If we are processing a spec function, we need to                                                               
>      end any pending argument.  */
>   if (processing_spec_function)
>     end_going_arg ();

so that when give_switch calls do_spec_1 twice for the basename and 
suffix, they end up as separate arguments to the spec function.  I don't 
know the purpose of this code; it doesn't seem to have been necessary 
for the if-exists spec function that was added in the same patch 
(r59241).  Removing this doesn't seem to break anything for me.

The join function works around both of these issues.  But I notice that 
the use of reconcat is a memory leak, and since we have the obstack 
available I've tweaked the function to use it.  I also added some 
documentation.

Joseph, any thoughts on these issues or the workaround?

Jason
  
Joseph Myers Aug. 23, 2023, 9:29 p.m. UTC | #2
On Wed, 23 Aug 2023, Jason Merrill via Gcc-patches wrote:

> Joseph, any thoughts on these issues or the workaround?

I don't have any comments here.