Bug 264730

Comment 1 Dimitry Andric 2022-06-18 13:39:56 UTC

I'm unsure why -fmodules changes this behavior, but __stdoutp is declared in /usr/include/stdio.h, like:

#ifndef _STDSTREAM_DECLARED
__BEGIN_DECLS
extern FILE *__stdinp;
extern FILE *__stdoutp;
extern FILE *__stderrp;
__END_DECLS
#define _STDSTREAM_DECLARED
#endif

while there is a similar, but slightly different declaration in /usr/include/wchar.h:

#ifndef _STDSTREAM_DECLARED
extern FILE *__stdinp;
extern FILE *__stdoutp;
extern FILE *__stderrp;
#define _STDSTREAM_DECLARED
#endif

Though in this file, __BEGIN_DECLS is at the top, and __END_DECLS at the bottom.

libc++'s <cstdio.h> begins by including <__config>, and then <stdio.h>. I suspect the former is pulling in <wchar.h>, or something like that?

On the other hand, you might be the very first person to ever try modules on FreeBSD. There are likely some bumps in the road. :)

Comment 2 Dimitry Andric 2022-06-18 13:55:26 UTC

This appears to fix it:

--- /usr/include/c++/v1/module.modulemap.org    2022-01-07 17:11:14.993926000 +0100
+++ /usr/include/c++/v1/module.modulemap        2022-06-18 15:52:04.275940000 +0200
@@ -180,7 +180,7 @@
       export *
     }
     module cstdio {
-      header "cstdio"
+      textual header "cstdio"
       export *
     }
     module cstdlib {

E.g. I looked at the module for <stdio.h>, and that has a comment:

    module stdio_h {
      // <stdio.h>'s __need_* macros require textual inclusion.
      textual header "stdio.h"
      export *
      export Darwin.C.stdio
    }

We don't have "__need_*" macros, but equivalent _*_DEFINED and _*_DECLARED ones.

It looks like clang modules can't work yet without the "textual" keyword on a bunch of our headers...

Well, it wasn't my choice, really ;-).  Apparently use of gmodules was enabled in LLDB's test suite on FreeBSD when the relevant tests were added — i.e. in 2016.  Unless I'm missing something, we've been doing some testing with them since then.  I'm guessing there were some recent changes to clang or libc++ that made them more strict.

Ah, sorry, I don't think the 'wchar.h' part is actually the problem — I think it just blames the wrong header.  The underlying problem is that the stdout/stderr/stdin macros aren't visible from <cstdio>.

I know very little about modules myself but FWICS the purpose here is to avoid relying on indirect includes.  I think textual header breaks that, i.e. the following test program should fail:

```
#include <cstdio>

int main() {
  FILE *f;
}
```

With the original header variant, it fails as expected (just blames the wrong header):

```
$ clang++ -fmodules test.cxx 
test.cxx:4:3: error: missing '#include <wchar.h>'; 'FILE' must be declared before it is used
  FILE *f;
  ^
/usr/include/wchar.h:109:24: note: declaration here is not visible
typedef struct __sFILE FILE;
                       ^
1 error generated.
```

Changing `header` to `textual header` makes it build, so I think that goes against the intent.  I suppose there must be a better way of solving stdout/stderr/stdin problem.

Comment 4 Dimitry Andric 2022-06-18 15:48:13 UTC

(In reply to Michał Górny from comment #3)
Well libc++'s <cstdio> simply includes <stdio.h>, so whatever applies to the latter must also apply to the former, I think?

On e.g. macOS, stdio.h is simpler with respect to __stdoutp and friends:

#include <_stdio.h>

__BEGIN_DECLS
extern FILE *__stdinp;
extern FILE *__stdoutp;
extern FILE *__stderrp;
__END_DECLS

On Linux with glibc, there are no tricks with __, they just declare:

/* Standard streams.  */
extern FILE *stdin;             /* Standard input stream.  */
extern FILE *stdout;            /* Standard output stream.  */
extern FILE *stderr;            /* Standard error output stream.  */
/* C89/C99 say they're macros.  Make them happy.  */
#define stdin stdin
#define stdout stdout
#define stderr stderr

but there are various other defines that use _need_this and _need_that.

As far as I understand the modules system, this looks like one of the as-of-yet unsolved problems in modules in general? I.e. should all system headers be rewritten to "support" -fmodules? I don't think that is going to happen on many operating systems..

As I understand it, the C++20 standard makes a distinction between:

A) C++ headers ( such as <chrono> )
vs.
B) C headers adopted by C++ ( such as <cassert> or <cstdio> )

So that:

export module NAME0;
import <chrono>
. . .

is supposed to be known to be okay. But:

export module NAME1;
import <cstdio>
. . .

is not portable and could give an error as a result
without violating the standard.

It leaves me wondering if the specific example
presented has wondered outside what is actually
supposed to be guaranteed by the C++20 standard.
(At this point I do not know.) The context is
different in the example but it might be related.

(In reply to Mark Millard from comment #5)

The C++20ish terminology here seems to be:

"importable C++ library headers"

See, for example, https://eel.is/c++draft/headers that
has a draft table listing them. It also has a separate
table for "C++ headers for C library facilities" that
are not classified as (guaranteed) importable.

<cstdio> is not guaranteed as importable in this draft
material.

I'm still unclear on if the #include use in the example
is a context for an implicit conversion to an import
according to the draft standard effort (or would be for
an importable C++ library header, anyway?). So I'm still
unclear if the example wondered outside the (draft)
guaranteed language properties.

(In reply to Mark Millard from comment #6)

If a #include lists a importable header, it is implementation
defined for if the #include is replaced by an import directive
for the header in question or not.

But <cstdio> does not have to be importable: it is not listed
in Table 21 of, for example, INCITS/ISO/IEC 14882:2020 (2021).

So it appears that definitions outside 14882:2020 need to be
used to establish the expected (but not necessarily portable)
behavior for the example.