[PATCH] D27439: [libcxx][modules] Fix <stddef.h>'s module definition

Eric Fiselier via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Dec 5 17:40:45 PST 2016


EricWF created this revision.
EricWF added reviewers: rsmith, manmanren, bruno.
EricWF added subscribers: cfe-commits, rsmith.

Currently libc++'s `stddef.h` is treated as a textual header by the module map, and technically this is correct because the header is non-modular (at least during normal compilation).
Unfortunatly this means that the stddef module doesn't own any of the definitions it should, like `size_t` and `ptrdiff_t`. which end up getting owned by random STL headers.

Fortunately I believe that  libc++'s `stddef.h` *is* modular due to Clang internals which handle modules containing `stddef.h` differently. Specifically Clang translates the module definition `module stddef_h { header "stddef.h" export *}` into

  module stddef_h {
    textual header "clang/builtin-includes/.../stddef.h"
    header "libcxx/include/stddef.h"
  }

This means that the `__need_*` macros will always be resolved and `#undef`-ed by Clang's `stddef.h` header before processing libc++'s. Therefore we should be able to treat libc++'s  header as non-textual. This allows libc++ to actually build a `stddef_h` module containing the correct definitions.

I've tested this change both with and without local sub-module visibility and on both Linux and OS X.

@rsmith What do you think? Does the above rational make sense?


https://reviews.llvm.org/D27439

Files:
  include/module.modulemap
  test/libcxx/modules/cstddef_h_exports.sh.cpp
  test/libcxx/modules/stddef_h_exports.sh.cpp
  test/libcxx/modules/stdlib_h_exports.sh.cpp


Index: test/libcxx/modules/stdlib_h_exports.sh.cpp
===================================================================
--- /dev/null
+++ test/libcxx/modules/stdlib_h_exports.sh.cpp
@@ -0,0 +1,20 @@
+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// REQUIRES: modules-support, verify-support
+
+// Test that stddef.h provides size_t
+
+// RUN: %build_module
+
+#include <stdlib.h>
+
+int main() {
+  size_t x;
+}
Index: test/libcxx/modules/stddef_h_exports.sh.cpp
===================================================================
--- /dev/null
+++ test/libcxx/modules/stddef_h_exports.sh.cpp
@@ -0,0 +1,20 @@
+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// REQUIRES: modules-support, verify-support
+
+// Test that stddef.h provides size_t
+
+// RUN: %compile_module %verify
+
+#include <ciso646>
+
+int main() {
+  size_t x; // expected-error {{missing '#include <stddef.h>'}}
+}
Index: test/libcxx/modules/cstddef_h_exports.sh.cpp
===================================================================
--- /dev/null
+++ test/libcxx/modules/cstddef_h_exports.sh.cpp
@@ -0,0 +1,21 @@
+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// REQUIRES: modules-support
+
+// Test that stddef.h provides size_t
+
+// RUN: %build_module
+
+#include <cstddef>
+
+int main() {
+  size_t x; ((void)x);
+  std::size_t y; ((void)y);
+}
Index: include/module.modulemap
===================================================================
--- include/module.modulemap
+++ include/module.modulemap
@@ -42,8 +42,15 @@
     // <stdarg.h> provided by compiler.
     // <stdbool.h> provided by compiler.
     module stddef_h {
-      // <stddef.h>'s __need_* macros require textual inclusion.
-      textual header "stddef.h"
+      // NOTE: <stddef.h> should technically be a textual header due to the
+      // __need_* macros. However Clang transforms this module definition into:
+      //   module stddef_h {
+      //      textual header "clang/builtin-includes/stddef.h"
+      //      header "libcxx/include/stddef.h"
+      //  }
+      // Therefore Clang's stddef.h resolves the __need_* macros before
+      // building libc++'s header as a non-textual module.
+      header "stddef.h"
     }
     module stdint_h {
       header "stdint.h"


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D27439.80350.patch
Type: text/x-patch
Size: 3187 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20161206/c2deaa63/attachment.bin>


More information about the cfe-commits mailing list