Page MenuHomePhabricator

libc++: adjust modulemap for non-modular C
AbandonedPublic

Authored by compnerd on Apr 7 2020, 5:30 PM.

Details

Reviewers
ldionne
Group Reviewers
Restricted Project
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.

Diff Detail

Event Timeline

compnerd created this revision.Apr 7 2020, 5:30 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 7 2020, 5:30 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript

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

joerg added a subscriber: joerg.Apr 8 2020, 3:01 AM

This fixes the module build of clang for me.

This fixes the module build of clang for me.

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

@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 added a comment.Apr 8 2020, 2:36 PM

@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 - 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.

@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.

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 added a subscriber: rsmith.Apr 8 2020, 5:37 PM

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 accepted this revision as: Restricted Project.May 7 2020, 10:31 AM

LGTM since other reviewers are fine with it.

compnerd abandoned this revision.Aug 3 2020, 9:58 AM

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.