libc++: adjust modulemap for non-modular C

[compnerd]

Summary

When building with a non-modularized C runtime, csignal would claim time.h and thus timespec rather than ctime. Adjust the modulemap to ensure that ctime claims timespec.

[dexonsmith]

This SGTM in principle, but the patch makes me wonder: which other headers from the non-modularized C runtime change ownership?

[joerg]

This fixes the module build of clang for me.

[ldionne]

In D77697#1968998, @joerg wrote:

This fixes the module build of clang for me.

@joerg When did it start failing for you? And what part of it did?

[compnerd]

@dexonsmith - yeah, sadly I dont think that there is a good way to audit that - any change to the public headers can cause issues. Furthermore, the libc headers themselves also influence this.

[joerg]

@ldionne I was updating libc++ from d42baff45d9700a199982ba0ac04dbc6c6d911bb and LLVM itself from 38aebe5c04ab4cb3695dc1bcc60b9a7b55215aff to 3d1424bc7a0e9a6f9887727e2bc93a10f50e1709 when it started failing. It likely has been present for a while.

[dexonsmith]

In D77697#1969998, @compnerd wrote:

@dexonsmith - yeah, sadly I dont think that there is a good way to audit that - any change to the public headers can cause issues. Furthermore, the libc headers themselves also influence this.

For auditing, can you use llvm-bcanalyze to see which headers are claimed by which PCM? If not, we should probably add a clang-pcm tool or something to help inspect module contents. @Bigcheese, thoughts?

I guess the obvious concern about this is that this is a game of whack-a-mole. If the headers change, you may need to shuffle module order again, at which point, which version of libc should we make libc++ work against? And different libc implementations could need different orders. But I'm just pointing it out; I don't have a problem with this patch landing.

[Bigcheese]

In D77697#1970586, @dexonsmith wrote:

In D77697#1969998, @compnerd wrote:

@dexonsmith - yeah, sadly I dont think that there is a good way to audit that - any change to the public headers can cause issues. Furthermore, the libc headers themselves also influence this.

For auditing, can you use llvm-bcanalyze to see which headers are claimed by which PCM? If not, we should probably add a clang-pcm tool or something to help inspect module contents. @Bigcheese, thoughts?

I guess the obvious concern about this is that this is a game of whack-a-mole. If the headers change, you may need to shuffle module order again, at which point, which version of libc should we make libc++ work against? And different libc implementations could need different orders. But I'm just pointing it out; I don't have a problem with this patch landing.

That would be useful for debugging this kind of issue and for testing a specific set of system headers + libc++ headers.

I'm also fine with this patch.

[compnerd]

I think that a clang-pcm like tool would be incredible. Working with modules is somewhat frustrating because there is no good supported way to see what the module structure that clang actually sees. But that goes well beyond an attempt to repair the NetBSD builds.

[rsmith]

In D77697#1968998, @joerg wrote:

This fixes the module build of clang for me.

Are you building with or without -fmodules-local-submodule-visibility (cmake -DLLVM_ENABLE_LOCAL_SUBMODULE_VISIBILITY=ON)? If you don't use that, this sort of random weirdness is going to come up a lot. And if you do use that setting, then this patch shouldn't make any difference; if it does, we should dig into why.

[ldionne]

LGTM since other reviewers are fine with it.

[compnerd]

Given that the original motivation here was to help with NetBSD, and the submodule visibility flag handling unblocks NetBSD, I think that I will hold off on this unless some other motivation arises.

libcxx/include/module.modulemap

module compat {
     module cmath {
       header "cmath"
       export *
     }
     module csetjmp {
       header "csetjmp"
       export *
     }
+    // NOTE: ensure that `ctime` preceeds `csignal` as otherwise, for a
+    // non-modular C runtime, you would end up with `csignal` providing time
+    // functions.
+    module ctime {
+      header "ctime"
+      export *
+    }
     module csignal {
       header "csignal"
       export *
     }
     // FIXME: <cstdalign> is missing.
     module cstdarg {
       header "cstdarg"
       export *
module cstring {
       export *
     }
     module ctgmath {
       header "ctgmath"
       export ccomplex
       export cmath
       export *
     }
-    module ctime {
-      header "ctime"
-      export *
-    }
     // FIXME: <cuchar> is missing.
     module cwchar {
       header "cwchar"
       export depr.stdio_h
       export *
     }
     module cwctype {
       header "cwctype"