Re: [PATCH v5 3/5] p1689r5: initial support

[Jason Merrill <jason@redhat.com>]

On 6/20/23 15:46, Ben Boeckel wrote:
> On Tue, Feb 14, 2023 at 16:50:27 -0500, Jason Merrill wrote:
>> On 1/25/23 13:06, Ben Boeckel wrote:

>>> 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.
>> >> I notice that the cpp dependency generation tries (in open_file_failed)
>> to continue after encountering a missing file, is that not sufficient 
>> for header units?  Or adjustable to be sufficient?
> 
> No. Header units can introduce macros which can be used to modify the
> set of modules that are imported. Included headers are "discovered"
> dependencies and don't modify the build graph (just add more files that
> trigger a rebuild) and can be collected during compilation. Module
> dependencies are needed to get the build correct in the first place in
> order to:
> 
> - order module compilations in the build graph so that imported modules
>   are ready before anything using them; and
> - computing the set of flags needed for telling the compiler where
>   imported modules' CMI files should be located.

So if the header unit CMI isn't available during dependency generation, 
would it be better to just #include the header?

>>> +  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;
>>
>> You probably want to check that deps_stream and fdeps_stream don't end
>> up as the same stream.
> 
> Hmm. But `stdout` is probably fine to use for both though. Basically:
> 
>      if (fdeps_stream == out_stream && fdeps_stream != stdout)
>        make_diagnostic_noise ();

(fdeps_stream == deps_stream, but sure, that's reasonable.

>> So, I take it this is the common use case you have in mind, generating
>> Make dependencies for the p1689 file?  When are you thinking the Make
>> dependencies for the .o are generated?  At build time?
> 
> Yes. If an included file changes, the scanning should be performed
> again. The compilation will have its own `-MF` as well (which should
> point to the same files plus the CMI files it ends up reading).
> 
>> I'm a bit surprised you're using .json rather than an extension that
>> indicates what the information is.
> 
> I can change that; the filename doesn't *really* matter (e.g., CMake
> uses `.ddi` for "dynamic dependency information").

That works.

>>> `-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.
>>
>> I'm not sure I see the distinction; Makefiles also express ordering
>> dependencies.  In both cases, you want to find out from the files what
>> order you will want to process them in when building the project.
> 
> Makefiles can express ordering dependencies, but not the `-M` snippets;
> these are for files that, if changed, should trigger a rebuild. This is > fundamentally different than module dependencies which instead indicate
> which *compiles* (or CMI generation if using a 2-phase setup) need to
> complete before compilation (or CMI generation) of the scanned TU can be
> performed. Generally generated headers will be ordered manually in the
> build system description. However, maintaining that same level for
> in-source dependency information on a per-source level is a *far* higher
> burden.

The main difference I see is that the CMI might not exist yet.  As you 
say, we don't want to require people to write all the dependencies by 
hand, but that just means we need to be able to generate the 
dependencies automatically.  In the Make-only model I'm thinking of, one 
would collect dependencies on an initial failing build, and then start 
over from the beginning again with the dependencies we discovered.  It's 
the same two-phase scan and build, but one that uses the same compile 
commands for both phases.

Anyway, this isn't an objection to this patch, just another model I also 
want to support.

>>> <snip JSON output diff>
>>
>> Is there a reason not to use the gcc/json.h interface for JSON output?
> 
> This is `libcpp`; is that not a dependency cycle?

Ah, indeed.  We could move it to libiberty, but it would need 
significant adjustments to remove its dependencies on other stuff in 
gcc/.  So maybe just add a TODO comment about that, along with adding 
comments before the functions.

Jason