[modules] Preprocessing requires compiled header unit modules

[modules] Preprocessing requires compiled header unit modules

[Ben Boeckel]

Hi,

I'm looking to update my patch which implements P1689[1] for GCC. I
originally wrote this back in February 2019 at Kona for a proof of
concept[2], but never really got the time to delve back in until now.
I'd like to get header units implemented prior to submitting it as it
will be an important part in the long run (as I don't think having a
partial implementation in the wild would be beneficial).

Now that modules have been merged, my patch works just fine for named
modules. However, for header unit modules, it runs into a problem that
imported header units are required to be compiled and available in the
mapper while scanning for dependencies.

Example code:

```c++ # use-header.cpp
module;

import "header-unit.hpp";

int main(int argc, char* argv[]) {
    return good;
}
```

```c++ # header-unit.hpp
constexpr int good = 0;
```

When trying to preprocess this:

```console
% g++ -std=c++20 -fmodules-ts -E use-header.cpp -o use-header.cpp.i
In module imported at use-header.cpp:3:1:
./header-unit.hpp: error: failed to read compiled module: No such file or directory
./header-unit.hpp: note: compiled module file is ‘gcm.cache/,/header-unit.hpp.gcm’
./header-unit.hpp: note: imports must be built before being imported
./header-unit.hpp: fatal error: returning to the gate for a mechanical issue
compilation terminated
```

There used to be no need to do this back prior to the modules landing in
`master`, but I can see this being an oversight in the meantime. I am
not familiar enough with the code to know how to untangle its current
state (attempting to bisect the change throws me back in to the depths
of the history without modules so "does it work" is unanswerable there).

I tried this on commits:

    4efde6781bba8d64b9dcff07e7efe71d35aa6f6a c++: Modules Is Landing
    f4ed267fa5b82d6dafbc8afc82baf45bfcae549c master as of the 23rd

Thanks,

--Ben

[1] https://wg21.link/p1689r4
[2] https://github.com/mathstuf/cxx-modules-sandbox/ (see the `docker`
branch for the instructions on putting things together)

Re: [modules] Preprocessing requires compiled header unit modules

[Boris Kolpackov]

Ben Boeckel <ben.boeckel@kitware.com> writes:

> However, for header unit modules, it runs into a problem that imported
> header units are required to be compiled and available in the mapper
> while scanning for dependencies.
> 
> Example code:
> 
> ```c++ # use-header.cpp
> module;
> 
> import "header-unit.hpp";
> 
> int main(int argc, char* argv[]) {
>     return good;
> }
> ```
>
> There used to be no need to do this back prior to the modules landing in
> `master`, but I can see this being an oversight in the meantime.

I don't think it is. A header unit (unlike a named module) may export
macros which could affect further dependencies. Consider:

import "header-unit.hpp"; // May or may not export macro FOO.

#ifdef FOO
import "header-unit2.hpp"
#endif

Re: [modules] Preprocessing requires compiled header unit modules

[Ben Boeckel]

On Thu, Apr 21, 2022 at 06:05:52 +0200, Boris Kolpackov wrote:
> I don't think it is. A header unit (unlike a named module) may export
> macros which could affect further dependencies. Consider:
> 
> import "header-unit.hpp"; // May or may not export macro FOO.
> 
> #ifdef FOO
> import "header-unit2.hpp"
> #endif

I agree that the header needs to be *found*, but scanning cannot require
a pre-existing BMI for that header. A new mode likely needs to be laid
down to get the information necessary (instead of just piggy-backing on
`-E` behavior to get what I want).

--Ben

Re: [modules] Preprocessing requires compiled header unit modules

[Iain Sandoe]

Hi Ben,

> On 21 Apr 2022, at 13:05, Ben Boeckel via Gcc <gcc@gcc.gnu.org> wrote:
> 
> On Thu, Apr 21, 2022 at 06:05:52 +0200, Boris Kolpackov wrote:
>> I don't think it is. A header unit (unlike a named module) may export
>> macros which could affect further dependencies. Consider:
>> 
>> import "header-unit.hpp"; // May or may not export macro FOO.

1.  If you know how this was built, then you could do an -E -fdirectives-only build (both
   GCC and clang support this now) to obtain the macros.

2.  I suppose we could invent a tool (or FE mode) to dump the macros exported by a HU ***

>> #ifdef FOO
>> import "header-unit2.hpp"
>> #endif
> 
> I agree that the header needs to be *found*, but scanning cannot require
> a pre-existing BMI for that header. A new mode likely needs to be laid
> down to get the information necessary (instead of just piggy-backing on
> `-E` behavior to get what I want).

perhaps that means (2)?

Iain

*** it’s kinda frustrating that this is hard infomation to get as a developer, so
    perhaps we can anticipate users wanting such output.

Re: [modules] Preprocessing requires compiled header unit modules

[Ben Boeckel]

On Thu, Apr 21, 2022 at 18:59:56 +0100, Iain Sandoe wrote:
> Hi Ben,
> 
> > On 21 Apr 2022, at 13:05, Ben Boeckel via Gcc <gcc@gcc.gnu.org> wrote:
> > 
> > On Thu, Apr 21, 2022 at 06:05:52 +0200, Boris Kolpackov wrote:
> >> I don't think it is. A header unit (unlike a named module) may export
> >> macros which could affect further dependencies. Consider:
> >> 
> >> import "header-unit.hpp"; // May or may not export macro FOO.
> 
> 1.  If you know how this was built, then you could do an -E -fdirectives-only build (both
>    GCC and clang support this now) to obtain the macros.

My understanding is that how it gets used determines how it should be
made for Clang (because the consumer's `-D`, `-W`, etc. flags matter). I
do not yet know how I am to support this in CMake.

> 2.  I suppose we could invent a tool (or FE mode) to dump the macros exported by a HU ***

Fun considerations:

  - are `-D` flags exported? `-U`?
  - how about this if `value` is the same as or different from the
    at-start expansion:

```c++
#undef SOME_MACRO
#define SOME_MACRO value
```

  - how about `#undef FOO`?

> >> #ifdef FOO
> >> import "header-unit2.hpp"
> >> #endif
> > 
> > I agree that the header needs to be *found*, but scanning cannot require
> > a pre-existing BMI for that header. A new mode likely needs to be laid
> > down to get the information necessary (instead of just piggy-backing on
> > `-E` behavior to get what I want).
> 
> perhaps that means (2)?

Can't it just read the header as if it wasn't imported? AFAIU, that's
what GCC did in Jan 2019. I understand that CPP state is probably not
easy, but something to consider.

> *** it’s kinda frustrating that this is hard infomation to get as a developer, so
>     perhaps we can anticipate users wanting such output.

I think cacheing and distributed build tools are the most likely
consumers of such information.

--Ben

Re: [modules] Preprocessing requires compiled header unit modules

[Iain Sandoe]

> On 21 Apr 2022, at 19:08, Ben Boeckel <ben.boeckel@kitware.com> wrote:
> 
> On Thu, Apr 21, 2022 at 18:59:56 +0100, Iain Sandoe wrote:
>> Hi Ben,
>> 
>>> On 21 Apr 2022, at 13:05, Ben Boeckel via Gcc <gcc@gcc.gnu.org> wrote:
>>> 
>>> On Thu, Apr 21, 2022 at 06:05:52 +0200, Boris Kolpackov wrote:
>>>> I don't think it is. A header unit (unlike a named module) may export
>>>> macros which could affect further dependencies. Consider:
>>>> 
>>>> import "header-unit.hpp"; // May or may not export macro FOO.
>> 
>> 1.  If you know how this was built, then you could do an -E -fdirectives-only build (both
>>   GCC and clang support this now) to obtain the macros.
> 
> My understanding is that how it gets used determines how it should be
> made for Clang (because the consumer's `-D`, `-W`, etc. flags matter). I
> do not yet know how I am to support this in CMake.

I think that is what I mean by “if you know how it is built” - it means you can replicate those
conditions, but produce the fdirectives-only output.  Perhaps I’m missing a subtelty …

>> 2.  I suppose we could invent a tool (or FE mode) to dump the macros exported by a HU ***
> 
> Fun considerations:
> 
>  - are `-D` flags exported? `-U`?
>  - how about this if `value` is the same as or different from the
>    at-start expansion:
> 
> ```c++
> #undef SOME_MACRO
> #define SOME_MACRO value
> ```
> 
>  - how about `#undef FOO`?

AFAIU, that is defined by the standard - only the defined state of a macro is exported.

So if the file contains
#define FOO 1
….
….
#undef FOO

— there will be no mention of FOO in the exported macros (at least, that is what
my impl. does ;) ).

— and the output would contain 'SOME_MACRO value’ for your other case.

This is quite different from the behaviour of PCH where the macro history is preserved.

>>>> #ifdef FOO
>>>> import "header-unit2.hpp"
>>>> #endif
>>> 
>>> I agree that the header needs to be *found*, but scanning cannot require
>>> a pre-existing BMI for that header. A new mode likely needs to be laid
>>> down to get the information necessary (instead of just piggy-backing on
>>> `-E` behavior to get what I want).
>> 
>> perhaps that means (2)?
> 
> Can't it just read the header as if it wasn't imported? AFAIU, that's
> what GCC did in Jan 2019. I understand that CPP state is probably not
> easy, but something to consider.

The BMIs (at least the two I’ve got some familiarity with) are quite complex - there
would have to be some mode that specifically extracted the macros only.

>> *** it’s kinda frustrating that this is hard infomation to get as a developer, so
>>    perhaps we can anticipate users wanting such output.
> 
> I think cacheing and distributed build tools are the most likely
> consumers of such information.

On an “industrial scale”, sure - but the ability for a user to check that what they think
is present _is_ present, is valuable - at least in debug.

Iain

Re: [modules] Preprocessing requires compiled header unit modules

[Boris Kolpackov]

Ben Boeckel <ben.boeckel@kitware.com> writes:

> On Thu, Apr 21, 2022 at 06:05:52 +0200, Boris Kolpackov wrote:
>
> > I don't think it is. A header unit (unlike a named module) may export
> > macros which could affect further dependencies. Consider:
> > 
> > import "header-unit.hpp"; // May or may not export macro FOO.
> > 
> > #ifdef FOO
> > import "header-unit2.hpp";
> > #endif
> 
> I agree that the header needs to be *found*, but scanning cannot require
> a pre-existing BMI for that header.

Well, if scanning cannot require a pre-existing BMI but a pre-existing
BMI is required to get accurate dependency information, then something
has to give.

You hint at a potential solution in your subsequent email:

> Can't it just read the header as if it wasn't imported? AFAIU, that's
> what GCC did in Jan 2019. I understand that CPP state is probably not
> easy, but something to consider.

The problem with this approach is that a header import and a header
include have subtly different semantics around macros. In particular,
the header import does not "see" macros defined by the importer while
the header include does. Here is an example:

// file: header-unit.hpp
//
#ifdef BAR
#define FOO
#endif

// file: importer.cpp
//
#define BAR
import "header-unit.hpp";    // Should not "see" BAR.
//#include "header-unit.hpp" // Should "see" BAR.

#ifdef FOO
import "header-unit2.hpp";
#endif

In this example, if you treat import of header-unit.hpp as
include, you will get incorrect dependency information.

So to make this work correctly we will need to re-create the
macro isolation semantics of import for include.

Even if we manage to do this, there are some implications I
am not sure we will like: the isolated macros will contain
inclusion guards, which means we will keep re-scanning the
same files potentially many many time. Here is an example,
assume each header-unitN.hpp includes or imports <functional>:

// file: importer.cpp
//
import <functional>; // Defined _GLIBCXX_FUNCTIONAL include

import "header-unit1.hpp"; // Ignores _GLIBCXX_FUNCTIONAL
import "header-unit2.hpp"; // Ditto.
import "header-unit3.hpp"; // Ditto.
import "header-unit4.hpp"; // Ditto.

Re: [modules] Preprocessing requires compiled header unit modules

[Iain Sandoe]

> On 22 Apr 2022, at 15:08, Boris Kolpackov <boris@codesynthesis.com> wrote:
> 
> Ben Boeckel <ben.boeckel@kitware.com> writes:
> 
>> On Thu, Apr 21, 2022 at 06:05:52 +0200, Boris Kolpackov wrote:
>> 
>>> I don't think it is. A header unit (unlike a named module) may export
>>> macros which could affect further dependencies. Consider:
>>> 
>>> import "header-unit.hpp"; // May or may not export macro FOO.
>>> 
>>> #ifdef FOO
>>> import "header-unit2.hpp";
>>> #endif
>> 
>> I agree that the header needs to be *found*, but scanning cannot require
>> a pre-existing BMI for that header.
> 
> Well, if scanning cannot require a pre-existing BMI but a pre-existing
> BMI is required to get accurate dependency information, then something
> has to give.
> 
> You hint at a potential solution in your subsequent email:
> 
>> Can't it just read the header as if it wasn't imported? AFAIU, that's
>> what GCC did in Jan 2019. I understand that CPP state is probably not
>> easy, but something to consider.
> 
> The problem with this approach is that a header import and a header
> include have subtly different semantics around macros. In particular,
> the header import does not "see" macros defined by the importer while
> the header include does. Here is an example:
> 
> // file: header-unit.hpp
> //
> #ifdef BAR
> #define FOO
> #endif
> 
> // file: importer.cpp
> //
> #define BAR
> import "header-unit.hpp";    // Should not "see" BAR.
> //#include "header-unit.hpp" // Should "see" BAR.
> 
> #ifdef FOO
> import "header-unit2.hpp";
> #endif
> 
> In this example, if you treat import of header-unit.hpp as
> include, you will get incorrect dependency information.
> 
> So to make this work correctly we will need to re-create the
> macro isolation semantics of import for include.
> 
> Even if we manage to do this, there are some implications I
> am not sure we will like: the isolated macros will contain
> inclusion guards, which means we will keep re-scanning the
> same files potentially many many time. Here is an example,
> assume each header-unitN.hpp includes or imports <functional>:
> 
> // file: importer.cpp
> //
> import <functional>; // Defined _GLIBCXX_FUNCTIONAL include
> 
> import "header-unit1.hpp"; // Ignores _GLIBCXX_FUNCTIONAL
> import "header-unit2.hpp"; // Ditto.
> import "header-unit3.hpp"; // Ditto.
> import "header-unit4.hpp"; // Ditto.

The standard has the concept of an “importable header” which is implementation-defined.

We could choose that only headers that are self-contained (i.e. unaffected by external
defines) are “importable” (thus the remaining headers would not be eligible for include-
translation).  That would mean that we could rely on processing any import by processing
the header it is created from?  Perhaps that is too great a restriction and we need to be
 more clever….

@ben, in relation to an earlier question:
https://eel.is/c++draft/cpp.import#note-4

says that predefined macro names are not introduced by #define and that the implementation
is encouraged not to treat them as if they were

IIUC, that means that -D/U (and preamble ones) are not emitted into the macro stream - however
it might well be the case that they *are* part of the module identifing hash (and preserved
as part of the captured command line).

Iain

Re: [modules] Preprocessing requires compiled header unit modules

[Ben Boeckel]

On Fri, Apr 22, 2022 at 16:08:00 +0200, Boris Kolpackov wrote:
> Ben Boeckel <ben.boeckel@kitware.com> writes:
> > I agree that the header needs to be *found*, but scanning cannot require
> > a pre-existing BMI for that header.
> 
> Well, if scanning cannot require a pre-existing BMI but a pre-existing
> BMI is required to get accurate dependency information, then something
> has to give.

If we need to know and have dependencies prepared before we can figure
out the dependencies for a TU, modules are unsolvable (without an active
build executor). If C++ implementations are really going to require
that, then not being able to perform `cl.exe *.cpp` anymore should be
the least of people's worries on the recent megathread because the
following tools are all unsuitable for C++ with header units without
major overhauls (alphabetical):

  - autoconf/automake
  - cmake
  - gn
  - gyp
  - make (not GNU make, though even that requires some active
    involvement via the socket communications)
  - meson
  - ninja

I'm sure there are many more in use out there.

> In this example, if you treat import of header-unit.hpp as
> include, you will get incorrect dependency information.
> 
> So to make this work correctly we will need to re-create the
> macro isolation semantics of import for include.

I'm aware. This is what I alluded to with "CPP state is probably not
easy".

> Even if we manage to do this, there are some implications I
> am not sure we will like: the isolated macros will contain
> inclusion guards, which means we will keep re-scanning the
> same files potentially many many time. Here is an example,
> assume each header-unitN.hpp includes or imports <functional>:

Note that scanning each module TU only happens once. Header units might
just get *read* in the course of scanning other units.

And headers are read multiple times already over the lifetime of the
build, so we're not making things worse here.

I wonder if we had had a modularized stdlib which landed with modules if
header units would have been a thing in the first place…

--Ben

Re: [modules] Preprocessing requires compiled header unit modules

[Boris Kolpackov]

Iain Sandoe <iain@sandoe.co.uk> writes:

> The standard has the concept of an “importable header” which is
> implementation-defined.

But it must contain all the C++ library headers:

https://eel.is/c++draft/headers#4


> We could choose that only headers that are self-contained (i.e. unaffected
> by external defines) are “importable” (thus the remaining headers would
> not be eligible for include-translation). That would mean that we could
> rely on processing any import by processing the header it is created from?
> Perhaps that is too great a restriction and we need to be more clever.

It will also be hard to determine whether the header (or any header
that it includes) satisfies this condition. You would probably want it
to be "meaningfully self-contained" (since pretty much every header is
not self-contained with regards to include guards) which I think will
be hard to automate.

Re: [modules] Preprocessing requires compiled header unit modules

[Boris Kolpackov]

Ben Boeckel <ben.boeckel@kitware.com> writes:

> If we need to know and have dependencies prepared before we can figure
> out the dependencies for a TU, modules are unsolvable (without an active
> build executor). If C++ implementations are really going to require
> that, then [...] the following tools are all unsuitable for C++ with
> header units without major overhauls (alphabetical):
> 
>   - autoconf/automake
>   - cmake
>   - gn
>   - gyp
>   - make (not GNU make, though even that requires some active
>     involvement via the socket communications)
>   - meson
>   - ninja

A couple of points:

1. Firstly, this only applies to header units, not named modules.

2. I am not sure what you mean by "active build executor" (but it
   does sound ominous, I will grant you that ;-)).

3. I agree some build systems may require "major overhauls" to
   support header units via the module mapper. I would like this
   not to be the case, but so far nobody has implemented an
   alternative (that I am aware of) that is correct and scalable
   and I personally have doubts such a thing is achievable.


> > Even if we manage to do this, there are some implications I
> > am not sure we will like: the isolated macros will contain
> > inclusion guards, which means we will keep re-scanning the
> > same files potentially many many time. Here is an example,
> > assume each header-unitN.hpp includes or imports <functional>:
> 
> Note that scanning each module TU only happens once. Header units might
> just get *read* in the course of scanning other units.
> 
> And headers are read multiple times already over the lifetime of the
> build, so we're not making things worse here.

I am not sure I follow. Say we have 10 TUs each include or import
10 headers each of which includes <functional>. If we use include,
then when scanning each of these 10 TUs we have to scan <functional>
once (since all the subsequent includes are suppressed by include
guards). So total of 10x1=10 scans of <functional> for the entire
build.

Now if instead of include we use import (which, during the scan, is
treated as include with macro isolation), we are looking at 10 scans
of <functional> for each TU (because the include guards are ignored).
So total of 10x10=100 scans of <functional> for the build.

What am I missing?

Re: [modules] Preprocessing requires compiled header unit modules

[Ben Boeckel]

On Mon, Apr 25, 2022 at 11:42:14 +0200, Boris Kolpackov wrote:
> 1. Firstly, this only applies to header units, not named modules.

Correct.

> 2. I am not sure what you mean by "active build executor" (but it
>    does sound ominous, I will grant you that ;-)).

One that does more than schedule builds, but is "actively" participating
in the build itself. Ninja's `dyndep` is about as close one can be to
the edge IMO. `make` is potentially active because of its behavior
around `include` of output files, but this is a niche feature.

> 3. I agree some build systems may require "major overhauls" to
>    support header units via the module mapper. I would like this
>    not to be the case, but so far nobody has implemented an
>    alternative (that I am aware of) that is correct and scalable
>    and I personally have doubts such a thing is achievable.

One the compilers can scan headers, CMake should be able to do it. MSVC
is the closest right now (though the name it wants for any given header
usage site is currently an issue).

> Ben Boeckel <ben.boeckel@kitware.com> writes:
> > Note that scanning each module TU only happens once. Header units might
> > just get *read* in the course of scanning other units.
> > 
> > And headers are read multiple times already over the lifetime of the
> > build, so we're not making things worse here.
> 
> I am not sure I follow. Say we have 10 TUs each include or import
> 10 headers each of which includes <functional>. If we use include,
> then when scanning each of these 10 TUs we have to scan <functional>
> once (since all the subsequent includes are suppressed by include
> guards). So total of 10x1=10 scans of <functional> for the entire
> build.
> 
> Now if instead of include we use import (which, during the scan, is
> treated as include with macro isolation), we are looking at 10 scans
> of <functional> for each TU (because the include guards are ignored).
> So total of 10x10=100 scans of <functional> for the build.
> 
> What am I missing?

Ew, indeed. I suppose a sufficiently smart compiler could just encode
headers as a "diff" to the preprocessor and symbol state and apply it
upon revisiting, but that seems like a high bar (re-reading if it
detects a difference in preprocessor state that "matters").

--Ben