On Mon, Oct 10, 2022 at 17:04:09 -0400, Jason Merrill wrote:
> On 10/4/22 11:12, Ben Boeckel wrote:
> > 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.
>
> Thanks!
>
> > 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.
>
> Do you expect users to want to emit Makefile (-MM) and P1689
> dependencies from the same compilation?
Yes, the build system wants to know what files affect scanning as well
(e.g., `#include <iostream>` is still possible and if it changes, this
operation should be performed again. The `-M` flags do this quite nicely
already :) .
> > - `-fdep-output=` 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.
>
> The dependency machinery already needs to be able to figure out the name
> of the output file, can't we reuse that instead of specifying it on the
> command line?
This is how it determines the output of the compilation. Because it is
piggy-backing on the `-E` flag, the `-o` flag specifies the output of
the preprocessed source (purely a side-effect right now).
> > diff --git a/libcpp/include/cpplib.h b/libcpp/include/cpplib.h
> > index 2db1e9cbdfb..90787230a9e 100644
> > --- a/libcpp/include/cpplib.h
> > +++ b/libcpp/include/cpplib.h
> > @@ -298,6 +298,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 };
>
> Why not add this to cpp_deps_style?
It is orthogonal. The `-M` flags and `-fdeps-*` flags are similar, but
`-fdeps-*` dependencies are fundamentally different than `-M`
dependencies. The comment does need updated though.
`-M` is about discovered dependencies: those that you find out while
doing work. `-fdep-*` is about ordering dependencies: extracting
information from file content in order to even order future work around.
It stems from the loss of embarassing parallelism when building C++20
code that uses `import`. It's isomorphic to the Fortran module
compilation ordering problem.
> > @@ -387,7 +410,7 @@ make_write_vec (const mkdeps::vec<const char *> &vec,
> > FILE *fp,
> > .PHONY targets for all the dependencies too. */
> >
> > static void
> > -make_write (const cpp_reader *pfile, FILE *fp, unsigned int colmax)
> > +make_write (const cpp_reader *pfile, FILE *fp, unsigned int colmax, int
> > extra)
>
> Instead of adding the "extra" parameter...
Hmm. Not sure why I had named this so poorly. Maybe this comes from my
initial stab at this functionality in 2019 (the format has been hammered
out in ISO C++'s SG15 since then).
> > {
> > const mkdeps *d = pfile->deps;
> >
> > @@ -398,7 +421,7 @@ make_write (const cpp_reader *pfile, FILE *fp, unsigned
> > int colmax)
> > 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 (extra && CPP_OPTION (pfile, deps.modules) && d->cmi_name)
> > column = make_write_name (d->cmi_name, fp, column, colmax);
> > fputs (":", fp);
> > column++;
> > @@ -412,7 +435,7 @@ make_write (const cpp_reader *pfile, FILE *fp, unsigned
> > int colmax)
> > if (!CPP_OPTION (pfile, deps.modules))
> > return;
>
> ...how about checking CPP_OPTION for p1689r5 mode here?
I'll take a look at this.
> >
> > - if (d->modules.size ())
> > + if (extra && d->modules.size ())
> > {
> > column = make_write_vec (d->targets, fp, 0, colmax, d->quote_lwm);
> > if (d->cmi_name)
> > @@ -423,7 +446,7 @@ make_write (const cpp_reader *pfile, FILE *fp, unsigned
> > int colmax)
> > fputs ("\n", fp);
> > }
> >
> > - if (d->module_name)
> > + if (extra && d->module_name)
> > {
> > if (d->cmi_name)
> > {
> > @@ -455,7 +478,7 @@ make_write (const cpp_reader *pfile, FILE *fp, unsigned
> > int colmax)
> > }
> > }
> >
> > - if (d->modules.size ())
> > + if (extra && d->modules.size ())
> > {
> > column = fprintf (fp, "CXX_IMPORTS +=");
> > make_write_vec (d->modules, fp, column, colmax, 0, ".c++m");
> > @@ -468,9 +491,203 @@ 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,
> > + int extra)
> > +{
> > + make_write (pfile, fp, colmax, extra);
> > +}
> > +
> > +static bool
> > +is_utf8 (const char *name)
>
> Can we share utf8 parsing code with decode_utf8_char in pretty-print.cc?
I can look at factoring that out. I'll have to decode its logic to see
how much overlap there is.
Thanks,
--Ben
