[clang] [clang-tools-extra] [llvm] [clang][modules] stdarg.h and stddef.h shouldn't directly declare anything (PR #90676)
https://github.com/ian-twilightcoder closed https://github.com/llvm/llvm-project/pull/90676 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang][modules] stdarg.h and stddef.h shouldn't directly declare anything (PR #90676)
https://github.com/ian-twilightcoder updated
https://github.com/llvm/llvm-project/pull/90676
>From 12964a3632d81d500b76c1a8a5c83a6367c66e00 Mon Sep 17 00:00:00 2001
From: Ian Anderson
Date: Tue, 30 Apr 2024 15:16:38 -0700
Subject: [PATCH] [clang][modules] stdarg.h and stddef.h shouldn't directly
declare anything
stdarg.h and especially stddef.h are textual and so everything they declare
gets precompiled into all of their clients' pcm files. They shouldn't directly
declare anything though, their purpose is to select what submodules get
imported, and not to add duplicate declarations to all of their clients. Make
it so that they always ignore their header guards, even without modules, and
declare them in separate header files so that they only go into the
stdarg/stddef pcms. Still declare them in case clients rely on them.
---
.../find-all-symbols/STLPostfixHeaderMap.cpp | 2 ++
.../clangd/index/CanonicalIncludes.cpp| 2 ++
clang/lib/Headers/CMakeLists.txt | 2 ++
clang/lib/Headers/__stdarg_header_macro.h | 12
clang/lib/Headers/__stddef_header_macro.h | 12
clang/lib/Headers/module.modulemap| 9 ++
clang/lib/Headers/stdarg.h| 28 -
clang/lib/Headers/stddef.h| 30 ---
.../gn/secondary/clang/lib/Headers/BUILD.gn | 2 ++
9 files changed, 53 insertions(+), 46 deletions(-)
create mode 100644 clang/lib/Headers/__stdarg_header_macro.h
create mode 100644 clang/lib/Headers/__stddef_header_macro.h
diff --git
a/clang-tools-extra/clang-include-fixer/find-all-symbols/STLPostfixHeaderMap.cpp
b/clang-tools-extra/clang-include-fixer/find-all-symbols/STLPostfixHeaderMap.cpp
index df77bf7ea46da3..469323f0ee9d7f 100644
---
a/clang-tools-extra/clang-include-fixer/find-all-symbols/STLPostfixHeaderMap.cpp
+++
b/clang-tools-extra/clang-include-fixer/find-all-symbols/STLPostfixHeaderMap.cpp
@@ -15,9 +15,11 @@ const HeaderMapCollector::RegexHeaderMap
*getSTLPostfixHeaderMap() {
static const HeaderMapCollector::RegexHeaderMap STLPostfixHeaderMap = {
{"include/__stdarg___gnuc_va_list.h$", ""},
{"include/__stdarg___va_copy.h$", ""},
+ {"include/__stdarg_header_macro.h$", ""},
{"include/__stdarg_va_arg.h$", ""},
{"include/__stdarg_va_copy.h$", ""},
{"include/__stdarg_va_list.h$", ""},
+ {"include/__stddef_header_macro.h$", ""},
{"include/__stddef_max_align_t.h$", ""},
{"include/__stddef_null.h$", ""},
{"include/__stddef_nullptr_t.h$", ""},
diff --git a/clang-tools-extra/clangd/index/CanonicalIncludes.cpp
b/clang-tools-extra/clangd/index/CanonicalIncludes.cpp
index 42eeba36a80e43..785ec4086ea760 100644
--- a/clang-tools-extra/clangd/index/CanonicalIncludes.cpp
+++ b/clang-tools-extra/clangd/index/CanonicalIncludes.cpp
@@ -18,9 +18,11 @@ namespace {
const std::pair IncludeMappings[] = {
{"include/__stdarg___gnuc_va_list.h", ""},
{"include/__stdarg___va_copy.h", ""},
+{"include/__stdarg_header_macro.h", ""},
{"include/__stdarg_va_arg.h", ""},
{"include/__stdarg_va_copy.h", ""},
{"include/__stdarg_va_list.h", ""},
+{"include/__stddef_header_macro.h", ""},
{"include/__stddef_max_align_t.h", ""},
{"include/__stddef_null.h", ""},
{"include/__stddef_nullptr_t.h", ""},
diff --git a/clang/lib/Headers/CMakeLists.txt b/clang/lib/Headers/CMakeLists.txt
index 3416811e39de27..5f02c71f6ca51a 100644
--- a/clang/lib/Headers/CMakeLists.txt
+++ b/clang/lib/Headers/CMakeLists.txt
@@ -12,6 +12,7 @@ set(core_files
stdarg.h
__stdarg___gnuc_va_list.h
__stdarg___va_copy.h
+ __stdarg_header_macro.h
__stdarg_va_arg.h
__stdarg_va_copy.h
__stdarg_va_list.h
@@ -19,6 +20,7 @@ set(core_files
stdbool.h
stdckdint.h
stddef.h
+ __stddef_header_macro.h
__stddef_max_align_t.h
__stddef_null.h
__stddef_nullptr_t.h
diff --git a/clang/lib/Headers/__stdarg_header_macro.h
b/clang/lib/Headers/__stdarg_header_macro.h
new file mode 100644
index 00..beb92ee0252679
--- /dev/null
+++ b/clang/lib/Headers/__stdarg_header_macro.h
@@ -0,0 +1,12 @@
+/*=== __stdarg_header_macro.h --===
+ *
+ * Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+ * See https://llvm.org/LICENSE.txt for license information.
+ * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+ *
+ *===---===
+ */
+
+#ifndef __STDARG_H
+#define __STDARG_H
+#endif
diff --git a/clang/lib/Headers/__stddef_header_macro.h
b/clang/lib/Headers/__stddef_header_macro.h
new file mode 100644
index 00..db5fb3c0abc1bf
--- /dev/null
+++ b/clang/lib/Headers/__stddef_header_macro.h
@@ -0,0 +1,12 @@
+/*=== __stddef_header_macro.h --===
+ *
+ * Part of the LLVM Project, under the Apache License v2.0 with LLVM
[clang] [clang-tools-extra] [llvm] [clang][modules] stdarg.h and stddef.h shouldn't directly declare anything (PR #90676)
ian-twilightcoder wrote: > I think I like this as a solution, although I wonder if > __stdarg_guard_macro.h would be a better name. The name now somewhat implies > that it contains the macros that stddef and stdarg define. But the macros don't actually guard anything. _macro isn't the best name in the world though, maybe _header_macro.h is ok even though they aren't traditional "header macros"? https://github.com/llvm/llvm-project/pull/90676 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang][modules] stdarg.h and stddef.h shouldn't directly declare anything (PR #90676)
@@ -15,9 +15,11 @@ const HeaderMapCollector::RegexHeaderMap
*getSTLPostfixHeaderMap() {
static const HeaderMapCollector::RegexHeaderMap STLPostfixHeaderMap = {
{"include/__stdarg___gnuc_va_list.h$", ""},
{"include/__stdarg___va_copy.h$", ""},
+ {"include/__stdarg_macro.h$", ""},
{"include/__stdarg_va_copy.h$", ""},
{"include/__stdarg_va_list.h$", ""},
+ {"include/__stddef_macro.h$", "https://github.com/llvm/llvm-project/pull/90676
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [clang][modules] stdarg.h and stddef.h shouldn't directly declare anything (PR #90676)
https://github.com/ian-twilightcoder updated
https://github.com/llvm/llvm-project/pull/90676
>From 54652504cb9e56c3972270c43cc4ee8d613979b4 Mon Sep 17 00:00:00 2001
From: Ian Anderson
Date: Tue, 30 Apr 2024 15:16:38 -0700
Subject: [PATCH] [clang][modules] stdarg.h and stddef.h shouldn't directly
declare anything
stdarg.h and especially stddef.h are textual and so everything they declare
gets precompiled into all of their clients' pcm files. They shouldn't directly
declare anything though, their purpose is to select what submodules get
imported, and not to add duplicate declarations to all of their clients. Make
it so that they always ignore their header guards, even without modules, and
declare them in separate header files so that they only go into the
stdarg/stddef pcms. Still declare them in case clients rely on them.
---
.../find-all-symbols/STLPostfixHeaderMap.cpp | 2 ++
.../clangd/index/CanonicalIncludes.cpp| 2 ++
clang/lib/Headers/CMakeLists.txt | 2 ++
clang/lib/Headers/__stdarg_macro.h| 12
clang/lib/Headers/__stddef_macro.h| 12
clang/lib/Headers/module.modulemap| 9 ++
clang/lib/Headers/stdarg.h| 28 -
clang/lib/Headers/stddef.h| 30 ---
.../gn/secondary/clang/lib/Headers/BUILD.gn | 2 ++
9 files changed, 53 insertions(+), 46 deletions(-)
create mode 100644 clang/lib/Headers/__stdarg_macro.h
create mode 100644 clang/lib/Headers/__stddef_macro.h
diff --git
a/clang-tools-extra/clang-include-fixer/find-all-symbols/STLPostfixHeaderMap.cpp
b/clang-tools-extra/clang-include-fixer/find-all-symbols/STLPostfixHeaderMap.cpp
index df77bf7ea46da3..4fb8e7fff497af 100644
---
a/clang-tools-extra/clang-include-fixer/find-all-symbols/STLPostfixHeaderMap.cpp
+++
b/clang-tools-extra/clang-include-fixer/find-all-symbols/STLPostfixHeaderMap.cpp
@@ -15,9 +15,11 @@ const HeaderMapCollector::RegexHeaderMap
*getSTLPostfixHeaderMap() {
static const HeaderMapCollector::RegexHeaderMap STLPostfixHeaderMap = {
{"include/__stdarg___gnuc_va_list.h$", ""},
{"include/__stdarg___va_copy.h$", ""},
+ {"include/__stdarg_macro.h$", ""},
{"include/__stdarg_va_copy.h$", ""},
{"include/__stdarg_va_list.h$", ""},
+ {"include/__stddef_macro.h$", ""},
{"include/__stddef_null.h$", ""},
{"include/__stddef_nullptr_t.h$", ""},
diff --git a/clang-tools-extra/clangd/index/CanonicalIncludes.cpp
b/clang-tools-extra/clangd/index/CanonicalIncludes.cpp
index 42eeba36a80e43..80a9d5088ad7ce 100644
--- a/clang-tools-extra/clangd/index/CanonicalIncludes.cpp
+++ b/clang-tools-extra/clangd/index/CanonicalIncludes.cpp
@@ -18,9 +18,11 @@ namespace {
const std::pair IncludeMappings[] = {
{"include/__stdarg___gnuc_va_list.h", ""},
{"include/__stdarg___va_copy.h", ""},
+{"include/__stdarg_macro.h", ""},
{"include/__stdarg_va_arg.h", ""},
{"include/__stdarg_va_copy.h", ""},
{"include/__stdarg_va_list.h", ""},
+{"include/__stddef_macro.h", ""},
{"include/__stddef_max_align_t.h", ""},
{"include/__stddef_null.h", ""},
{"include/__stddef_nullptr_t.h", ""},
diff --git a/clang/lib/Headers/CMakeLists.txt b/clang/lib/Headers/CMakeLists.txt
index 3416811e39de27..bc0ce66f4946f2 100644
--- a/clang/lib/Headers/CMakeLists.txt
+++ b/clang/lib/Headers/CMakeLists.txt
@@ -12,6 +12,7 @@ set(core_files
stdarg.h
__stdarg___gnuc_va_list.h
__stdarg___va_copy.h
+ __stdarg_macro.h
__stdarg_va_arg.h
__stdarg_va_copy.h
__stdarg_va_list.h
@@ -19,6 +20,7 @@ set(core_files
stdbool.h
stdckdint.h
stddef.h
+ __stddef_macro.h
__stddef_max_align_t.h
__stddef_null.h
__stddef_nullptr_t.h
diff --git a/clang/lib/Headers/__stdarg_macro.h
b/clang/lib/Headers/__stdarg_macro.h
new file mode 100644
index 00..aa9c9f72f1c1ac
--- /dev/null
+++ b/clang/lib/Headers/__stdarg_macro.h
@@ -0,0 +1,12 @@
+/*=== __stdarg_macro.h -===
+ *
+ * Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+ * See https://llvm.org/LICENSE.txt for license information.
+ * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+ *
+ *===---===
+ */
+
+#ifndef __STDARG_H
+#define __STDARG_H
+#endif
diff --git a/clang/lib/Headers/__stddef_macro.h
b/clang/lib/Headers/__stddef_macro.h
new file mode 100644
index 00..dac7b1b47cd80a
--- /dev/null
+++ b/clang/lib/Headers/__stddef_macro.h
@@ -0,0 +1,12 @@
+/*=== __stddef_macro.h -===
+ *
+ * Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+ * See https://llvm.org/LICENSE.txt for license information.
+ * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+ *
+
[clang] [clang-tools-extra] [llvm] [clang][modules] stdarg.h and stddef.h shouldn't directly declare anything (PR #90676)
https://github.com/ian-twilightcoder updated
https://github.com/llvm/llvm-project/pull/90676
>From 052dfeb8d94971dadfa65147f79d5fa37910b0e0 Mon Sep 17 00:00:00 2001
From: Ian Anderson
Date: Tue, 30 Apr 2024 15:16:38 -0700
Subject: [PATCH] [clang][modules] stdarg.h and stddef.h shouldn't directly
declare anything
stdarg.h and especially stddef.h are textual and so everything they declare
gets precompiled into all of their clients' pcm files. They shouldn't directly
declare anything though, their purpose is to select what submodules get
imported, and not to add duplicate declarations to all of their clients. Make
it so that they always ignore their header guards, even without modules, and
declare them in separate header files so that they only go into the
stdarg/stddef pcms. Still declare them in case clients rely on them.
---
.../find-all-symbols/STLPostfixHeaderMap.cpp | 2 ++
.../clangd/index/CanonicalIncludes.cpp| 2 ++
clang/lib/Headers/CMakeLists.txt | 2 ++
clang/lib/Headers/__stdarg_macro.h| 12
clang/lib/Headers/__stddef_macro.h| 12
clang/lib/Headers/module.modulemap| 9 ++
clang/lib/Headers/stdarg.h| 26 -
clang/lib/Headers/stddef.h| 29 ---
.../gn/secondary/clang/lib/Headers/BUILD.gn | 2 ++
9 files changed, 51 insertions(+), 45 deletions(-)
create mode 100644 clang/lib/Headers/__stdarg_macro.h
create mode 100644 clang/lib/Headers/__stddef_macro.h
diff --git
a/clang-tools-extra/clang-include-fixer/find-all-symbols/STLPostfixHeaderMap.cpp
b/clang-tools-extra/clang-include-fixer/find-all-symbols/STLPostfixHeaderMap.cpp
index df77bf7ea46da3..4fb8e7fff497af 100644
---
a/clang-tools-extra/clang-include-fixer/find-all-symbols/STLPostfixHeaderMap.cpp
+++
b/clang-tools-extra/clang-include-fixer/find-all-symbols/STLPostfixHeaderMap.cpp
@@ -15,9 +15,11 @@ const HeaderMapCollector::RegexHeaderMap
*getSTLPostfixHeaderMap() {
static const HeaderMapCollector::RegexHeaderMap STLPostfixHeaderMap = {
{"include/__stdarg___gnuc_va_list.h$", ""},
{"include/__stdarg___va_copy.h$", ""},
+ {"include/__stdarg_macro.h$", ""},
{"include/__stdarg_va_copy.h$", ""},
{"include/__stdarg_va_list.h$", ""},
+ {"include/__stddef_macro.h$", ""},
{"include/__stddef_null.h$", ""},
{"include/__stddef_nullptr_t.h$", ""},
diff --git a/clang-tools-extra/clangd/index/CanonicalIncludes.cpp
b/clang-tools-extra/clangd/index/CanonicalIncludes.cpp
index 42eeba36a80e43..80a9d5088ad7ce 100644
--- a/clang-tools-extra/clangd/index/CanonicalIncludes.cpp
+++ b/clang-tools-extra/clangd/index/CanonicalIncludes.cpp
@@ -18,9 +18,11 @@ namespace {
const std::pair IncludeMappings[] = {
{"include/__stdarg___gnuc_va_list.h", ""},
{"include/__stdarg___va_copy.h", ""},
+{"include/__stdarg_macro.h", ""},
{"include/__stdarg_va_arg.h", ""},
{"include/__stdarg_va_copy.h", ""},
{"include/__stdarg_va_list.h", ""},
+{"include/__stddef_macro.h", ""},
{"include/__stddef_max_align_t.h", ""},
{"include/__stddef_null.h", ""},
{"include/__stddef_nullptr_t.h", ""},
diff --git a/clang/lib/Headers/CMakeLists.txt b/clang/lib/Headers/CMakeLists.txt
index e6ae4e19e81db9..0abbffe12f8030 100644
--- a/clang/lib/Headers/CMakeLists.txt
+++ b/clang/lib/Headers/CMakeLists.txt
@@ -12,6 +12,7 @@ set(core_files
stdarg.h
__stdarg___gnuc_va_list.h
__stdarg___va_copy.h
+ __stdarg_macro.h
__stdarg_va_arg.h
__stdarg_va_copy.h
__stdarg_va_list.h
@@ -19,6 +20,7 @@ set(core_files
stdbool.h
stdckdint.h
stddef.h
+ __stddef_macro.h
__stddef_max_align_t.h
__stddef_null.h
__stddef_nullptr_t.h
diff --git a/clang/lib/Headers/__stdarg_macro.h
b/clang/lib/Headers/__stdarg_macro.h
new file mode 100644
index 00..aa9c9f72f1c1ac
--- /dev/null
+++ b/clang/lib/Headers/__stdarg_macro.h
@@ -0,0 +1,12 @@
+/*=== __stdarg_macro.h -===
+ *
+ * Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+ * See https://llvm.org/LICENSE.txt for license information.
+ * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+ *
+ *===---===
+ */
+
+#ifndef __STDARG_H
+#define __STDARG_H
+#endif
diff --git a/clang/lib/Headers/__stddef_macro.h
b/clang/lib/Headers/__stddef_macro.h
new file mode 100644
index 00..dac7b1b47cd80a
--- /dev/null
+++ b/clang/lib/Headers/__stddef_macro.h
@@ -0,0 +1,12 @@
+/*=== __stddef_macro.h -===
+ *
+ * Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+ * See https://llvm.org/LICENSE.txt for license information.
+ * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+ *
+
[clang] [clang-tools-extra] [llvm] [clang][modules] stdarg.h and stddef.h shouldn't directly declare anything (PR #90676)
https://github.com/ian-twilightcoder updated
https://github.com/llvm/llvm-project/pull/90676
>From 8d8756f00297e90e0b1125d2a78d3f41c9aae6a7 Mon Sep 17 00:00:00 2001
From: Ian Anderson
Date: Tue, 30 Apr 2024 15:16:38 -0700
Subject: [PATCH] [clang][modules] stdarg.h and stddef.h shouldn't directly
declare anything
stdarg.h and especially stddef.h are textual and so everything they declare
gets precompiled into all of their clients' pcm files. They shouldn't directly
declare anything though, their purpose is to select what submodules get
imported, and not to add duplicate declarations to all of their clients. Make
it so that they always ignore their header guards, even without modules, and
declare them in separate header files so that they only go into the
stdarg/stddef pcms. Still declare them in case clients rely on them.
---
.../find-all-symbols/STLPostfixHeaderMap.cpp | 2 ++
.../clangd/index/CanonicalIncludes.cpp| 2 ++
clang/lib/Headers/CMakeLists.txt | 2 ++
clang/lib/Headers/__stdarg_macro.h| 12 +
clang/lib/Headers/__stddef_macro.h| 12 +
clang/lib/Headers/module.modulemap| 9 +++
clang/lib/Headers/stdarg.h| 24 -
clang/lib/Headers/stddef.h| 27 ---
.../gn/secondary/clang/lib/Headers/BUILD.gn | 2 ++
9 files changed, 51 insertions(+), 41 deletions(-)
create mode 100644 clang/lib/Headers/__stdarg_macro.h
create mode 100644 clang/lib/Headers/__stddef_macro.h
diff --git
a/clang-tools-extra/clang-include-fixer/find-all-symbols/STLPostfixHeaderMap.cpp
b/clang-tools-extra/clang-include-fixer/find-all-symbols/STLPostfixHeaderMap.cpp
index df77bf7ea46da3..4fb8e7fff497af 100644
---
a/clang-tools-extra/clang-include-fixer/find-all-symbols/STLPostfixHeaderMap.cpp
+++
b/clang-tools-extra/clang-include-fixer/find-all-symbols/STLPostfixHeaderMap.cpp
@@ -15,9 +15,11 @@ const HeaderMapCollector::RegexHeaderMap
*getSTLPostfixHeaderMap() {
static const HeaderMapCollector::RegexHeaderMap STLPostfixHeaderMap = {
{"include/__stdarg___gnuc_va_list.h$", ""},
{"include/__stdarg___va_copy.h$", ""},
+ {"include/__stdarg_macro.h$", ""},
{"include/__stdarg_va_copy.h$", ""},
{"include/__stdarg_va_list.h$", ""},
+ {"include/__stddef_macro.h$", ""},
{"include/__stddef_null.h$", ""},
{"include/__stddef_nullptr_t.h$", ""},
diff --git a/clang-tools-extra/clangd/index/CanonicalIncludes.cpp
b/clang-tools-extra/clangd/index/CanonicalIncludes.cpp
index 42eeba36a80e43..80a9d5088ad7ce 100644
--- a/clang-tools-extra/clangd/index/CanonicalIncludes.cpp
+++ b/clang-tools-extra/clangd/index/CanonicalIncludes.cpp
@@ -18,9 +18,11 @@ namespace {
const std::pair IncludeMappings[] = {
{"include/__stdarg___gnuc_va_list.h", ""},
{"include/__stdarg___va_copy.h", ""},
+{"include/__stdarg_macro.h", ""},
{"include/__stdarg_va_arg.h", ""},
{"include/__stdarg_va_copy.h", ""},
{"include/__stdarg_va_list.h", ""},
+{"include/__stddef_macro.h", ""},
{"include/__stddef_max_align_t.h", ""},
{"include/__stddef_null.h", ""},
{"include/__stddef_nullptr_t.h", ""},
diff --git a/clang/lib/Headers/CMakeLists.txt b/clang/lib/Headers/CMakeLists.txt
index e6ae4e19e81db9..0abbffe12f8030 100644
--- a/clang/lib/Headers/CMakeLists.txt
+++ b/clang/lib/Headers/CMakeLists.txt
@@ -12,6 +12,7 @@ set(core_files
stdarg.h
__stdarg___gnuc_va_list.h
__stdarg___va_copy.h
+ __stdarg_macro.h
__stdarg_va_arg.h
__stdarg_va_copy.h
__stdarg_va_list.h
@@ -19,6 +20,7 @@ set(core_files
stdbool.h
stdckdint.h
stddef.h
+ __stddef_macro.h
__stddef_max_align_t.h
__stddef_null.h
__stddef_nullptr_t.h
diff --git a/clang/lib/Headers/__stdarg_macro.h
b/clang/lib/Headers/__stdarg_macro.h
new file mode 100644
index 00..aa9c9f72f1c1ac
--- /dev/null
+++ b/clang/lib/Headers/__stdarg_macro.h
@@ -0,0 +1,12 @@
+/*=== __stdarg_macro.h -===
+ *
+ * Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+ * See https://llvm.org/LICENSE.txt for license information.
+ * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+ *
+ *===---===
+ */
+
+#ifndef __STDARG_H
+#define __STDARG_H
+#endif
diff --git a/clang/lib/Headers/__stddef_macro.h
b/clang/lib/Headers/__stddef_macro.h
new file mode 100644
index 00..dac7b1b47cd80a
--- /dev/null
+++ b/clang/lib/Headers/__stddef_macro.h
@@ -0,0 +1,12 @@
+/*=== __stddef_macro.h -===
+ *
+ * Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+ * See https://llvm.org/LICENSE.txt for license information.
+ * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+ *
+
[clang] [clang][modules] stdarg.h and stddef.h shouldn't directly declare anything (PR #90676)
https://github.com/ian-twilightcoder created
https://github.com/llvm/llvm-project/pull/90676
stdarg.h and especially stddef.h are textual and so everything they declare
gets precompiled into all of their clients' pcm files. They shouldn't directly
declare anything though, their purpose is to select what submodules get
imported, and not to add duplicate declarations to all of their clients. Make
it so that they always ignore their header guards, even without modules, and
declare them in separate header files so that they only go into the
stdarg/stddef pcms. Still declare them in case clients rely on them.
>From 67ed3eb8580fb94d7ec29ef07d6052dfda3b0b3e Mon Sep 17 00:00:00 2001
From: Ian Anderson
Date: Tue, 30 Apr 2024 15:16:38 -0700
Subject: [PATCH] [clang][modules] stdarg.h and stddef.h shouldn't directly
declare anything
stdarg.h and especially stddef.h are textual and so everything they declare
gets precompiled into all of their clients' pcm files. They shouldn't directly
declare anything though, their purpose is to select what submodules get
imported, and not to add duplicate declarations to all of their clients. Make
it so that they always ignore their header guards, even without modules, and
declare them in separate header files so that they only go into the
stdarg/stddef pcms. Still declare them in case clients rely on them.
---
clang/lib/Headers/__stdarg_macro.h | 12
clang/lib/Headers/__stddef_macro.h | 12
clang/lib/Headers/module.modulemap | 9 +
clang/lib/Headers/stdarg.h | 24 +---
clang/lib/Headers/stddef.h | 27 +--
5 files changed, 43 insertions(+), 41 deletions(-)
create mode 100644 clang/lib/Headers/__stdarg_macro.h
create mode 100644 clang/lib/Headers/__stddef_macro.h
diff --git a/clang/lib/Headers/__stdarg_macro.h
b/clang/lib/Headers/__stdarg_macro.h
new file mode 100644
index 00..aa9c9f72f1c1ac
--- /dev/null
+++ b/clang/lib/Headers/__stdarg_macro.h
@@ -0,0 +1,12 @@
+/*=== __stdarg_macro.h -===
+ *
+ * Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+ * See https://llvm.org/LICENSE.txt for license information.
+ * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+ *
+ *===---===
+ */
+
+#ifndef __STDARG_H
+#define __STDARG_H
+#endif
diff --git a/clang/lib/Headers/__stddef_macro.h
b/clang/lib/Headers/__stddef_macro.h
new file mode 100644
index 00..dac7b1b47cd80a
--- /dev/null
+++ b/clang/lib/Headers/__stddef_macro.h
@@ -0,0 +1,12 @@
+/*=== __stddef_macro.h -===
+ *
+ * Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+ * See https://llvm.org/LICENSE.txt for license information.
+ * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+ *
+ *===---===
+ */
+
+#ifndef __STDDEF_H
+#define __STDDEF_H
+#endif
diff --git a/clang/lib/Headers/module.modulemap
b/clang/lib/Headers/module.modulemap
index 8741968fa7f364..492d2aba8af969 100644
--- a/clang/lib/Headers/module.modulemap
+++ b/clang/lib/Headers/module.modulemap
@@ -203,6 +203,11 @@ module _Builtin_stdarg [system] {
export *
}
+ explicit module macro {
+header "__stdarg_macro.h"
+export *
+ }
+
explicit module va_arg {
header "__stdarg_va_arg.h"
export *
@@ -232,6 +237,10 @@ module _Builtin_stdbool [system] {
module _Builtin_stddef [system] {
textual header "stddef.h"
+ explicit module macro {
+header "__stddef_macro.h"
+export *
+ }
// __stddef_max_align_t.h is always in this module, even if
// -fbuiltin-headers-in-system-modules is passed.
explicit module max_align_t {
diff --git a/clang/lib/Headers/stdarg.h b/clang/lib/Headers/stdarg.h
index 94b066566f084e..753c187cb61841 100644
--- a/clang/lib/Headers/stdarg.h
+++ b/clang/lib/Headers/stdarg.h
@@ -14,29 +14,15 @@
* need to use some of its interfaces. Otherwise this header provides all of
* the expected interfaces.
*
- * When clang modules are enabled, this header is a textual header. It ignores
- * its header guard so that multiple submodules can export its interfaces.
- * Take module SM with submodules A and B, whose headers both include stdarg.h
- * When SM.A builds, __STDARG_H will be defined. When SM.B builds, the
- * definition from SM.A will leak when building without local submodule
- * visibility. stdarg.h wouldn't include any of its implementation headers, and
- * SM.B wouldn't import any of the stdarg modules, and SM.B's `export *`
- * wouldn't export any stdarg interfaces as expected. However, since stdarg.h
- * ignores its header guard when building with modules, it all works as
- * expected.
- *
- * When clang modules are not enabled, the header guards can
[clang] [clang][modules] HeaderSearch::MarkFileModuleHeader sets textual headers' HeaderFileInfo non-external when it shouldn't (PR #89005)
ian-twilightcoder wrote: >From Jan: > Maybe we can add the number of external HeaderFileInfos to > HeaderSearch::PrintStats() and have a test that checks for it. > The flag to enable that output is -show-stats I think. https://github.com/llvm/llvm-project/pull/89005 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] HeaderSearch::MarkFileModuleHeader sets textual headers' HeaderFileInfo non-external when it shouldn't (PR #89005)
ian-twilightcoder wrote: I guess https://github.com/llvm/llvm-project/pull/89441 is the real issue that https://github.com/llvm/llvm-project/pull/83660 caused, but we should still fix this for correctness, even though I'm not sure what the visible consequence is of this bug. https://github.com/llvm/llvm-project/pull/89005 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] HeaderSearch::MarkFileModuleHeader sets textual headers' HeaderFileInfo non-external when it shouldn't (PR #89005)
https://github.com/ian-twilightcoder edited https://github.com/llvm/llvm-project/pull/89005 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] HeaderSearch::MarkFileModuleHeader sets textual headers' HeaderFileInfo non-external when it shouldn't (PR #89005)
https://github.com/ian-twilightcoder updated
https://github.com/llvm/llvm-project/pull/89005
>From 45e7409a92546aab4cf8c79ce0ba40133e4dfe4c Mon Sep 17 00:00:00 2001
From: Ian Anderson
Date: Tue, 16 Apr 2024 17:08:28 -0700
Subject: [PATCH] [clang][modules] HeaderSearch::MarkFileModuleHeader sets
textual headers' HeaderFileInfo non-external when it shouldn't
HeaderSearch::MarkFileModuleHeader is no longer properly checking for
no-changes, and so sets the HeaderFileInfo for every `textual header` to
non-external.
---
clang/include/clang/Lex/HeaderSearch.h | 4 +++-
clang/lib/Lex/HeaderSearch.cpp | 14 +++---
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/clang/include/clang/Lex/HeaderSearch.h
b/clang/include/clang/Lex/HeaderSearch.h
index c5f90ef4cb3682..863878986560c4 100644
--- a/clang/include/clang/Lex/HeaderSearch.h
+++ b/clang/include/clang/Lex/HeaderSearch.h
@@ -84,7 +84,9 @@ struct HeaderFileInfo {
LLVM_PREFERRED_TYPE(bool)
unsigned isModuleHeader : 1;
- /// Whether this header is a `textual header` in a module.
+ /// Whether this header is a `textual header` in a module. If a header is
+ /// textual in one module and normal in another module, this bit will not be
+ /// set, only `isModuleHeader`.
LLVM_PREFERRED_TYPE(bool)
unsigned isTextualModuleHeader : 1;
diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp
index 0632882b296146..fd2333015b61ad 100644
--- a/clang/lib/Lex/HeaderSearch.cpp
+++ b/clang/lib/Lex/HeaderSearch.cpp
@@ -1313,11 +1313,19 @@ OptionalFileEntryRef
HeaderSearch::LookupSubframeworkHeader(
// File Info Management.
//===--===//
+static bool moduleMembershipNeedsMerge(const HeaderFileInfo *HFI,
+ ModuleMap::ModuleHeaderRole Role) {
+ if (ModuleMap::isModular(Role))
+return !HFI->isModuleHeader || HFI->isTextualModuleHeader;
+ else if (!HFI->isModuleHeader && (Role & ModuleMap::TextualHeader))
+return !HFI->isTextualModuleHeader;
+ else
+return false;
+}
+
static void mergeHeaderFileInfoModuleBits(HeaderFileInfo ,
bool isModuleHeader,
bool isTextualModuleHeader) {
- assert((!isModuleHeader || !isTextualModuleHeader) &&
- "A header can't build with a module and be textual at the same time");
HFI.isModuleHeader |= isModuleHeader;
if (HFI.isModuleHeader)
HFI.isTextualModuleHeader = false;
@@ -1432,7 +1440,7 @@ void HeaderSearch::MarkFileModuleHeader(FileEntryRef FE,
if ((Role & ModuleMap::ExcludedHeader))
return;
auto *HFI = getExistingFileInfo(FE);
-if (HFI && HFI->isModuleHeader)
+if (HFI && !moduleMembershipNeedsMerge(HFI, Role))
return;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] HeaderSearch::MarkFileModuleHeader sets textual headers' HeaderFileInfo non-external when it shouldn't (PR #89005)
https://github.com/ian-twilightcoder edited https://github.com/llvm/llvm-project/pull/89005 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Allow module maps with textual headers to be non-affecting (PR #89441)
ian-twilightcoder wrote: > > Is this a pre-existing issue, or did my patch change to make "each textual > > header gets a `HFI`"? > > My best understanding that your patch gave textual headers`HFI`s when the > module map was loaded, rather than when the header was included. This > shouldn't have mattered, but for the latent pre-existing bug: this caused the > modulemap to be "affecting" and thus serialized. (Fixed by this patch) > > Your patch changed an early-bailout condition in `markFileModuleHeader` > (1426) from `if !ModularHeader` to `if ExcludedHeader`, so now we carry on > further in case of textual headers. The next line calls `getExistingFileInfo` > which is probably fine, and then bails out if `ModularHeader` (1430). But > with textual headers, we keep going. Finally on line 1433 we call > `getFileInfo` and the `HFI` is forced to exist. > > (Having read through this a few times, I really appreciate the detail you put > in the comments and commit message, thanks for that!) Ah yeah, that makes sense. Thanks for the help (and Richard and Jan and Ilya too) in finding and fixing this. HFI's are a lot tricker than they look on the surface! https://github.com/llvm/llvm-project/pull/89441 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Allow module maps with textual headers to be non-affecting (PR #89441)
ian-twilightcoder wrote: > I updated the description of this PR, hopefully it makes more sense now. I > still need to investigate what goes wrong in > "Modules/preprocess-decluse.cpp". It seems that it assumes `%t/b.pcm` embeds > the information from "a.modulemap". Is this a pre-existing issue, or did my patch change to make "each textual header gets a `HFI`"? https://github.com/llvm/llvm-project/pull/89441 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Allow module maps with textual headers to be non-affecting (PR #89441)
ian-twilightcoder wrote: > > Hmm, this will probably only work if you compile `A` into a PCM and then > > pass it to `B`. This is not how "Modules/preprocess-decluse.cpp" is set up. > > In our case we always have a chain A <- B <- C. A.modulemap can be affecting > for B but should not be for C. (Approximately, A covers approximately clang's > headers (textual), B covers the standard library (modular). Every other > library C sees A.modulemap, B.modulemap, B.pcm. There is no PCM for A). clang's headers all have proper modules now, are you sure you still need A? https://github.com/llvm/llvm-project/pull/89441 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Allow module maps with textual headers to be non-affecting (PR #89441)
https://github.com/ian-twilightcoder approved this pull request. I don't totally understand the change, but I don't see anything wrong with it! https://github.com/llvm/llvm-project/pull/89441 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] HeaderSearch::MarkFileModuleHeader creates extra HeaderFileInfo, breaks PCM reuse (PR #89005)
ian-twilightcoder wrote: > Unfortunately with this patch I'm still seeing the same > source-location-exhausted error. I'm going to try to understand why... Damn I really thought that was going to be the one. If you have any way I could reproduce or better write a test that would be great. https://github.com/llvm/llvm-project/pull/89005 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] HeaderSearch::MarkFileModuleHeader creates extra HeaderFileInfo, breaks PCM reuse (PR #89005)
ian-twilightcoder wrote: > Ok _this_ one should fix the logic I think. @ilya-biryukov can you give this > a try please? @ilya-biryukov sorry to ping you again, just nobody else knows how to test this. https://github.com/llvm/llvm-project/pull/89005 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] HeaderSearch::MarkFileModuleHeader creates extra HeaderFileInfo, breaks PCM reuse (PR #89005)
https://github.com/ian-twilightcoder edited https://github.com/llvm/llvm-project/pull/89005 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] HeaderSearch::MarkFileModuleHeader creates extra HeaderFileInfo, breaks PCM reuse (PR #89005)
https://github.com/ian-twilightcoder edited https://github.com/llvm/llvm-project/pull/89005 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] HeaderSearch::MarkFileModuleHeader creates extra HeaderFileInfo, breaks PCM reuse (PR #89005)
@@ -1313,6 +1313,14 @@ OptionalFileEntryRef
HeaderSearch::LookupSubframeworkHeader(
// File Info Management.
//===--===//
+static bool
+headerFileInfoModuleBitsMatchRole(const HeaderFileInfo *HFI,
+ ModuleMap::ModuleHeaderRole Role) {
+ return (HFI->isModuleHeader == ModuleMap::isModular(Role)) &&
+ (HFI->isTextualModuleHeader ==
+ ((Role & ModuleMap::TextualHeader) != 0));
ian-twilightcoder wrote:
I don't think I quite understand. I think the test looked something like this.
```
module A [no_undeclared_includes] {
module one { header "one.h" }
module two { header "two" }
}
module B {
module one { textual header "one.h" }
module three { header "three.h" }
}
```
That allows one.h to include three.h even though A doesn't have a `use B`. But
wouldn't the better setup be to have the `use B`? I might not be quite
understanding what you're saying.
https://github.com/llvm/llvm-project/pull/89005
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] HeaderSearch::MarkFileModuleHeader creates extra HeaderFileInfo, breaks PCM reuse (PR #89005)
ian-twilightcoder wrote: > Also note that `ASTWriter` uses this logic in couple of places to avoid > serializing unrelated headers: > > ```c++ > if (!HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader)) > continue; > ``` > > Since textual headers from other modules have `isModuleHeader=false` and > `isCompilingModuleHeader=false` after #83660 we always serialize them, even > if we just implicitly found their module map and never entered them. I didn't > check how this patch interacts with that logic, just wanted to surface this. #83660 _shouldn't_ affect that logic. `isModuleHeader` and `isCompilingModuleHeader` _should_ always have the same values after that change. It's supposed to just add an extra `isTextualModuleHeader` without changing any of the other bits. https://github.com/llvm/llvm-project/pull/89005 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] HeaderSearch::MarkFileModuleHeader creates extra HeaderFileInfo, breaks PCM reuse (PR #89005)
ian-twilightcoder wrote: > As a test, maybe you could probe the resulting PCM with `-module-file-info`. What would I be looking for? Presumably the problem is we import the same module twice but fail to find the built pcm in the module cache and so we build it again? I don't know what else would cause runaway growth of pcm files... I actually don't really understand the problem Ilya is hitting... But Richard found (2 now!) good bugs in my code and I'm hoping fixing that will fix Ilya's problem. https://github.com/llvm/llvm-project/pull/89005 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] HeaderSearch::MarkFileModuleHeader creates extra HeaderFileInfo, breaks PCM reuse (PR #89005)
https://github.com/ian-twilightcoder edited https://github.com/llvm/llvm-project/pull/89005 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] HeaderSearch::MarkFileModuleHeader creates extra HeaderFileInfo, breaks PCM reuse (PR #89005)
@@ -84,7 +84,9 @@ struct HeaderFileInfo {
LLVM_PREFERRED_TYPE(bool)
unsigned isModuleHeader : 1;
- /// Whether this header is a `textual header` in a module.
+ /// Whether this header is a `textual header` in a module. If a header is
+ /// textual in one module and normal in another module, this bit will not be
+ /// set, only `isModuleHeader`.
ian-twilightcoder wrote:
Maybe this behavior is weird? Maybe both bits should be set in this scenario?
`HeaderSearch::ShouldEnterIncludeFile` -> `MaybeReenterImportedFile` could
change its check to `!FileInfo. isModuleHeader &&
FileInfo.isTextualModuleHeader`
https://github.com/llvm/llvm-project/pull/89005
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] HeaderSearch::MarkFileModuleHeader creates extra HeaderFileInfo, breaks PCM reuse (PR #89005)
ian-twilightcoder wrote: Ok _this_ one should fix the logic I think. @ilya-biryukov can you give this a try please? https://github.com/llvm/llvm-project/pull/89005 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] HeaderSearch::MarkFileModuleHeader creates extra HeaderFileInfo, breaks PCM reuse (PR #89005)
https://github.com/ian-twilightcoder updated
https://github.com/llvm/llvm-project/pull/89005
>From 0ea2af8066f2fb307f3bd273cf34da82189af0ab Mon Sep 17 00:00:00 2001
From: Ian Anderson
Date: Tue, 16 Apr 2024 17:08:28 -0700
Subject: [PATCH] [clang][modules] HeaderSearch::MarkFileModuleHeader creates
extra HeaderFileInfo, breaks PCM reuse
HeaderSearch::MarkFileModuleHeader is no longer properly checking for
no-changes, and so creates a new HeaderFileInfo for every `textual header`,
causes PCM use to go ballistic.
---
clang/include/clang/Lex/HeaderSearch.h | 4 +++-
clang/lib/Lex/HeaderSearch.cpp | 14 +++---
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/clang/include/clang/Lex/HeaderSearch.h
b/clang/include/clang/Lex/HeaderSearch.h
index c5f90ef4cb3682..863878986560c4 100644
--- a/clang/include/clang/Lex/HeaderSearch.h
+++ b/clang/include/clang/Lex/HeaderSearch.h
@@ -84,7 +84,9 @@ struct HeaderFileInfo {
LLVM_PREFERRED_TYPE(bool)
unsigned isModuleHeader : 1;
- /// Whether this header is a `textual header` in a module.
+ /// Whether this header is a `textual header` in a module. If a header is
+ /// textual in one module and normal in another module, this bit will not be
+ /// set, only `isModuleHeader`.
LLVM_PREFERRED_TYPE(bool)
unsigned isTextualModuleHeader : 1;
diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp
index 0632882b296146..fd2333015b61ad 100644
--- a/clang/lib/Lex/HeaderSearch.cpp
+++ b/clang/lib/Lex/HeaderSearch.cpp
@@ -1313,11 +1313,19 @@ OptionalFileEntryRef
HeaderSearch::LookupSubframeworkHeader(
// File Info Management.
//===--===//
+static bool moduleMembershipNeedsMerge(const HeaderFileInfo *HFI,
+ ModuleMap::ModuleHeaderRole Role) {
+ if (ModuleMap::isModular(Role))
+return !HFI->isModuleHeader || HFI->isTextualModuleHeader;
+ else if (!HFI->isModuleHeader && (Role & ModuleMap::TextualHeader))
+return !HFI->isTextualModuleHeader;
+ else
+return false;
+}
+
static void mergeHeaderFileInfoModuleBits(HeaderFileInfo ,
bool isModuleHeader,
bool isTextualModuleHeader) {
- assert((!isModuleHeader || !isTextualModuleHeader) &&
- "A header can't build with a module and be textual at the same time");
HFI.isModuleHeader |= isModuleHeader;
if (HFI.isModuleHeader)
HFI.isTextualModuleHeader = false;
@@ -1432,7 +1440,7 @@ void HeaderSearch::MarkFileModuleHeader(FileEntryRef FE,
if ((Role & ModuleMap::ExcludedHeader))
return;
auto *HFI = getExistingFileInfo(FE);
-if (HFI && HFI->isModuleHeader)
+if (HFI && !moduleMembershipNeedsMerge(HFI, Role))
return;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] HeaderSearch::MarkFileModuleHeader creates extra HeaderFileInfo, breaks PCM reuse (PR #89005)
@@ -1313,6 +1313,14 @@ OptionalFileEntryRef
HeaderSearch::LookupSubframeworkHeader(
// File Info Management.
//===--===//
+static bool
+headerFileInfoModuleBitsMatchRole(const HeaderFileInfo *HFI,
+ ModuleMap::ModuleHeaderRole Role) {
+ return (HFI->isModuleHeader == ModuleMap::isModular(Role)) &&
+ (HFI->isTextualModuleHeader ==
+ ((Role & ModuleMap::TextualHeader) != 0));
ian-twilightcoder wrote:
Oh I see what you're saying. I must have a HFI that says the header is
normal-modular, and a role that says it's textual-modular. When I merge in the
role it's not going to change the HFI since `isModuleHeader` never gets
downgraded, but my matching check will return false. Sneaky.
> It looks like you're considering "modular header" and "textual header" to be
> mutually exclusive
Yes, I even put an assert in the merging code. But I'm told asserts aren't
usually (ever?) compiled in, so I guess that's probably why it's not getting
hit. I did notice a test listing a header as normal in one module and textual
in another. What does that actually mean though? The test uses it to exploit a
bug in `[no_undeclared_includes]` where the compiler will use one module to
resolve the include and a different module to check for uses and bypass what
the used module says is allowed. Really what does it mean for a header to be in
multiple modules at all? Doesn't that just cause non-deterministic behavior in
header->module lookup?
https://github.com/llvm/llvm-project/pull/89005
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] HeaderSearch::MarkFileModuleHeader creates extra HeaderFileInfo, breaks PCM reuse (PR #89005)
ian-twilightcoder wrote: > > @ilya-biryukov can you check that this fixes your running out of source > > location space problem please? > > Just tried it. The patch as is did not help. I've also tried changing the > previous line to `getExistingFileInfo(, /*WantExternal=*/false)` and it > didn't help either. > > Changing from `if ((Role & ModuleMap::ExcludedHeader))` back to `if > (!ModuleMap::isModular(Role))` does help, though, but that clearly leads to > an incorrect behavior as far as the code is concerned. Does it not help because `headerFileInfoModuleBitsMatchRole` is returning `false`? The previous code was doing `WantExternal=true` so I don't think we want `getExistingLocalFileInfo()` here? I think if we used `getExistingLocalFileInfo()`, we'd get `nullptr` back more often and fall down into the `getFileInfo()` case more often wouldn't we? https://github.com/llvm/llvm-project/pull/89005 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] HeaderSearch::MarkFileModuleHeader creates extra HeaderFileInfo, breaks PCM reuse (PR #89005)
ian-twilightcoder wrote: @ilya-biryukov can you check that this fixes your running out of source location space problem please? https://github.com/llvm/llvm-project/pull/89005 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] HeaderSearch::MarkFileModuleHeader creates extra HeaderFileInfo, breaks PCM reuse (PR #89005)
ian-twilightcoder wrote: I don't really know a great way to write a test for this. If someone could point me at a similar existing test or has an idea how to write a new test that would be much appreciated. https://github.com/llvm/llvm-project/pull/89005 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Headers meant to be included multiple times can be completely invisible in clang module builds (PR #83660)
@@ -1403,94 +1421,146 @@ bool
HeaderSearch::isFileMultipleIncludeGuarded(FileEntryRef File) const {
void HeaderSearch::MarkFileModuleHeader(FileEntryRef FE,
ModuleMap::ModuleHeaderRole Role,
bool isCompilingModuleHeader) {
- bool isModularHeader = ModuleMap::isModular(Role);
-
// Don't mark the file info as non-external if there's nothing to change.
if (!isCompilingModuleHeader) {
-if (!isModularHeader)
+if ((Role & ModuleMap::ExcludedHeader))
return;
auto *HFI = getExistingFileInfo(FE);
if (HFI && HFI->isModuleHeader)
return;
}
auto = getFileInfo(FE);
- HFI.isModuleHeader |= isModularHeader;
+ HFI.mergeModuleMembership(Role);
HFI.isCompilingModuleHeader |= isCompilingModuleHeader;
ian-twilightcoder wrote:
https://github.com/llvm/llvm-project/pull/89005 and thank you!!
https://github.com/llvm/llvm-project/pull/83660
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] HeaderSearch::MarkFileModuleHeader creates extra HeaderFileInfo, breaks PCM reuse (PR #89005)
https://github.com/ian-twilightcoder created
https://github.com/llvm/llvm-project/pull/89005
HeaderSearch::MarkFileModuleHeader is no longer properly checking for
no-changes, and so creates a new HeaderFileInfo for every `textual header`,
causes PCM use to go ballistic.
>From f45cb72cb9c70d714bdc071ac51dff1a5e11502b Mon Sep 17 00:00:00 2001
From: Ian Anderson
Date: Tue, 16 Apr 2024 17:08:28 -0700
Subject: [PATCH] [clang][modules] HeaderSearch::MarkFileModuleHeader creates
extra HeaderFileInfo, breaks PCM reuse
HeaderSearch::MarkFileModuleHeader is no longer properly checking for
no-changes, and so creates a new HeaderFileInfo for every `textual header`,
causes PCM use to go ballistic.
---
clang/lib/Lex/HeaderSearch.cpp | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp
index 0632882b296146..9409b97ba0e64a 100644
--- a/clang/lib/Lex/HeaderSearch.cpp
+++ b/clang/lib/Lex/HeaderSearch.cpp
@@ -1313,6 +1313,14 @@ OptionalFileEntryRef
HeaderSearch::LookupSubframeworkHeader(
// File Info Management.
//===--===//
+static bool
+headerFileInfoModuleBitsMatchRole(const HeaderFileInfo *HFI,
+ ModuleMap::ModuleHeaderRole Role) {
+ return (HFI->isModuleHeader == ModuleMap::isModular(Role)) &&
+ (HFI->isTextualModuleHeader ==
+ ((Role & ModuleMap::TextualHeader) != 0));
+}
+
static void mergeHeaderFileInfoModuleBits(HeaderFileInfo ,
bool isModuleHeader,
bool isTextualModuleHeader) {
@@ -1432,7 +1440,7 @@ void HeaderSearch::MarkFileModuleHeader(FileEntryRef FE,
if ((Role & ModuleMap::ExcludedHeader))
return;
auto *HFI = getExistingFileInfo(FE);
-if (HFI && HFI->isModuleHeader)
+if (HFI && headerFileInfoModuleBitsMatchRole(HFI, Role))
return;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Headers meant to be included multiple times can be completely invisible in clang module builds (PR #83660)
@@ -1403,94 +1421,146 @@ bool
HeaderSearch::isFileMultipleIncludeGuarded(FileEntryRef File) const {
void HeaderSearch::MarkFileModuleHeader(FileEntryRef FE,
ModuleMap::ModuleHeaderRole Role,
bool isCompilingModuleHeader) {
- bool isModularHeader = ModuleMap::isModular(Role);
-
// Don't mark the file info as non-external if there's nothing to change.
if (!isCompilingModuleHeader) {
-if (!isModularHeader)
+if ((Role & ModuleMap::ExcludedHeader))
return;
auto *HFI = getExistingFileInfo(FE);
if (HFI && HFI->isModuleHeader)
return;
}
auto = getFileInfo(FE);
- HFI.isModuleHeader |= isModularHeader;
+ HFI.mergeModuleMembership(Role);
HFI.isCompilingModuleHeader |= isCompilingModuleHeader;
ian-twilightcoder wrote:
Ohhh. So I need to be checking the `getExistingFileInfo` to see if
`isModuleHeader` and `isTextualModuleHeader` match the given `ModuleHeaderRole`?
https://github.com/llvm/llvm-project/pull/83660
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Modules] Add -cc1 -flate-module-map-file to load module maps after PCMs (PR #88893)
ian-twilightcoder wrote: + @Bigcheese and @jansvoboda11 as I think they know this area of the code better https://github.com/llvm/llvm-project/pull/88893 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Headers meant to be included multiple times can be completely invisible in clang module builds (PR #83660)
ian-twilightcoder wrote: > To get back to my previous point about the semantics implemented by this > patch being unfortunate -- the upshot is that, now: > > ``` > #include > #define NDEBUG > #import > ``` It would be nice if we could make this work without modules too. `#pragma many` or something would do that, but I fear we'd have compatibility issues if we added a pragma that's only supported by clang to a bunch of system headers. > This doesn't make sense: with modules disabled, everything is textual -- to > say that "textual header" in a modulemap should make a header somehow "more" > textual than the default non-modular behavior is weird. With modules disabled, everything isn't textual - it's effectively excluded. `textual` was probably not the best name for that keyword in the module map. https://github.com/llvm/llvm-project/pull/83660 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Headers meant to be included multiple times can be completely invisible in clang module builds (PR #83660)
ian-twilightcoder wrote: Wall of text incoming... Sorry but our documentation in this area is poor, so I feel like I need to spell out the assumptions behind this change. First of all, this _should_ be a relatively minor change that should affect a very limited set of circumstances. 1. Only `#import` should be affected, not `#include`. 2. Only headers marked `textual header` in a module map should be affected, nothing else. 3. Headers that use `#pragma once` or have header guards should not be affected. 4. Module map reading should not be affected. That should end up being a rather narrow change, and if something else got affected I'd like to know what and fix it. `#import` is already treated differently if modules are enabled. This adds another case to the special treatment, but doesn't introduce that fact that sometimes `#import`ed headers will be re-entered when building with modules. We can say we don't like `#import` behaving differently when modules are enabled. However, it's not going to be any harder to undo this special treatment than it will to undo the other ones, if we ever care enough to try, so I don't think this is such an egregious change. `textual header`s are already treated differently as well. To better define that, there are 4 different types of headers in a clang modules world. 1. Headers covered by a `header` statement in a module map - ye olde standard include-once header. 2. Headers covered by a `textual header` statement in a module map - usually headers that are intended to be included multiple times. Generally these are X macro generator headers, or special headers like or clang's and which are designed to have different behavior based on the preprocessor environment of the includer. 3. Headers covered by an `exclude header` statement in a module map - headers that can't be used in a module. Sometimes these are just obsolete headers, and sometimes it's headers that have dependencies that can't be used in a module. 4. Headers that are not listed in any module map. These are essentially the same thing as excluded headers. There's a semantic difference between textual and excluded headers. Textual headers are anointed to be safe to use in modules, and excluded headers are not supposed to be included from any modular header. (If I had my way, `-Wnon-modular-include-in-module` would default to an error much like `-Wmodule-import-in-extern-c` does, but that can be a soap box for another day.) Textual headers are already treated differently from excluded headers in a few ways, such as they respect `[no_undeclared_includes]`/`use`, while excluded headers don't. All this to say that `#import`ed headers are already treated different when modules are enabled, and textual headers are already treated different than excluded headers. I think it's fine to tweak this different treatment, especially since it's solving a real world problem that occurs in a fully modularized system. I also think it's preferable to focus the change down to the very specific problem, which is `#import`ed textual headers, instead of trying to fix all of the submodule visibility problems we can think of. https://github.com/llvm/llvm-project/pull/83660 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [modules] allow use of ptrauth module from no_undeclared_includes system modules (PR #88432)
https://github.com/ian-twilightcoder closed https://github.com/llvm/llvm-project/pull/88432 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Headers meant to be included multiple times can be completely invisible in clang module builds (PR #83660)
ian-twilightcoder wrote: > The end result should be that #imported and #pragma once-guarded files are > treated the same way as #ifndef-guarded files. While I don't necessarily disagree with that goal in principle, it wasn't true before this change either. There was already some special casing to defeat `#import`. This change just adds another special case to the `#import` path. `#include` doesn't even go through the modified code that is `MaybeReenterImportedFile` (née `TryEnterImported`). > I know you weren't trying to fix that -- but I'm saying that's the thing that > _should_ be fixed, and that doing so would fix the _serious_ part of the > problem you described in the initial message. Modular headers including non-modular headers is a special problem. If we let every module enter the non-modular header (which Jan's patch does), its declarations will build into multiple modules/pcm files, where they will 1) conflict and cause errors in the several cases where type merging can't handle them, and 2) come from multiple modules which is undefined behavior for Swift (i.e. Swift will see `mod1.type` and `mod2.type` and won't know how to resolve un-qualified `type` in code). So I don't think that case really _can_ be fixed. https://github.com/llvm/llvm-project/pull/83660 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Headers meant to be included multiple times can be completely invisible in clang module builds (PR #83660)
ian-twilightcoder wrote: Also this change isn't trying to modify `#include` in any kind of way, _only_ `#import`. So it's quite intentional that your test behaves the same before and after. https://github.com/llvm/llvm-project/pull/83660 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Headers meant to be included multiple times can be completely invisible in clang module builds (PR #83660)
ian-twilightcoder wrote: I don't really think it's the same thing. The problem I'm trying to fix is that nobody knows when it's appropriate to use `#import` vs `#include`, and the unfortunate convention of Objective-C makes it impossible for header owners to indicate if they support being included multiple times or not, as using header guards or `#pragma once` is very "un-Objective-C". Marking the header as `textual` is a fairly reasonable way for header owners to indicate that their header is meant to be included multiple times (e.g. stddef.h), even if ObjC developers don't know that and used `#import`. (The other use of `textual` that I've seen is for headers like `unwind_arm_ehabi.h` that are only allowed to have a single includer, and aren't really standalone.) If you don't want the header to build with the module, that's what `excluded` is for is it not? And alternative solution would be @jansvoboda11's https://github.com/llvm/llvm-project/pull/71117, but I believe that will cause issues with non-modular headers building into multiple modules. https://github.com/llvm/llvm-project/pull/83660 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [modules] allow use of ptrauth module from no_undeclared_includes system modules (PR #88432)
https://github.com/ian-twilightcoder edited https://github.com/llvm/llvm-project/pull/88432 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] allow use of ptrauth module from no_undeclared_includes system modules (PR #88432)
ian-twilightcoder wrote: Upstream from Apple, followup for https://github.com/llvm/llvm-project/pull/65996. Allows Apple's Darwin module to include ptrauth.h without declaring a `use`. https://github.com/llvm/llvm-project/pull/88432 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] allow use of ptrauth module from no_undeclared_includes system modules (PR #88432)
https://github.com/ian-twilightcoder updated
https://github.com/llvm/llvm-project/pull/88432
>From 88da8be7ed10f1ee8e7e992fdd59dce52456b2ce Mon Sep 17 00:00:00 2001
From: Alex Lorenz
Date: Thu, 9 Jul 2020 15:10:49 -0700
Subject: [PATCH] [modules] allow use of ptrauth module from
no_undeclared_includes system modules
---
clang/lib/Basic/Module.cpp| 4
.../Inputs/ptrauth-include-from-darwin/module.modulemap | 8
.../Modules/Inputs/ptrauth-include-from-darwin/ptrauth.h | 1 +
.../Modules/Inputs/ptrauth-include-from-darwin/stddef.h | 1 +
clang/test/Modules/ptrauth-include-from-darwin.m | 6 ++
5 files changed, 20 insertions(+)
create mode 100644
clang/test/Modules/Inputs/ptrauth-include-from-darwin/module.modulemap
create mode 100644
clang/test/Modules/Inputs/ptrauth-include-from-darwin/ptrauth.h
create mode 100644
clang/test/Modules/Inputs/ptrauth-include-from-darwin/stddef.h
create mode 100644 clang/test/Modules/ptrauth-include-from-darwin.m
diff --git a/clang/lib/Basic/Module.cpp b/clang/lib/Basic/Module.cpp
index 256365d66bb907..bb212cde878826 100644
--- a/clang/lib/Basic/Module.cpp
+++ b/clang/lib/Basic/Module.cpp
@@ -305,6 +305,10 @@ bool Module::directlyUses(const Module *Requested) {
if (Requested->fullModuleNameIs({"_Builtin_stddef", "max_align_t"}) ||
Requested->fullModuleNameIs({"_Builtin_stddef_wint_t"}))
return true;
+ // Darwin is allowed is to use our builtin 'ptrauth.h' and its accompanying
+ // module.
+ if (!Requested->Parent && Requested->Name == "ptrauth")
+return true;
if (NoUndeclaredIncludes)
UndeclaredUses.insert(Requested);
diff --git
a/clang/test/Modules/Inputs/ptrauth-include-from-darwin/module.modulemap
b/clang/test/Modules/Inputs/ptrauth-include-from-darwin/module.modulemap
new file mode 100644
index 00..741b9bb1efc54d
--- /dev/null
+++ b/clang/test/Modules/Inputs/ptrauth-include-from-darwin/module.modulemap
@@ -0,0 +1,8 @@
+module libc [no_undeclared_includes] {
+ module stddef { header "stddef.h" export * }
+}
+
+module ptrauth {
+ header "ptrauth.h"
+ export *
+}
diff --git a/clang/test/Modules/Inputs/ptrauth-include-from-darwin/ptrauth.h
b/clang/test/Modules/Inputs/ptrauth-include-from-darwin/ptrauth.h
new file mode 100644
index 00..c8620b64b2ceef
--- /dev/null
+++ b/clang/test/Modules/Inputs/ptrauth-include-from-darwin/ptrauth.h
@@ -0,0 +1 @@
+void foo();
diff --git a/clang/test/Modules/Inputs/ptrauth-include-from-darwin/stddef.h
b/clang/test/Modules/Inputs/ptrauth-include-from-darwin/stddef.h
new file mode 100644
index 00..777a524fc67110
--- /dev/null
+++ b/clang/test/Modules/Inputs/ptrauth-include-from-darwin/stddef.h
@@ -0,0 +1 @@
+@import ptrauth;
diff --git a/clang/test/Modules/ptrauth-include-from-darwin.m
b/clang/test/Modules/ptrauth-include-from-darwin.m
new file mode 100644
index 00..72b0c36e7cb7d3
--- /dev/null
+++ b/clang/test/Modules/ptrauth-include-from-darwin.m
@@ -0,0 +1,6 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -I
%S/Inputs/ptrauth-include-from-darwin %s -verify
+// expected-no-diagnostics
+
+@import libc;
+void bar() { foo(); }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Headers meant to be included multiple times can be completely invisible in clang module builds (PR #83660)
ian-twilightcoder wrote: Wow that's a _ton_ of PCM files. Why are your headers `textual` if they're only meant to be included once? Do they need to inherit the includer's preprocessor environment or something like that? https://github.com/llvm/llvm-project/pull/83660 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] allow use of ptrauth module from no_undeclared_includes system modules (PR #88432)
https://github.com/ian-twilightcoder created
https://github.com/llvm/llvm-project/pull/88432
None
>From 43b007bfb184c6fdb5d802afca16af14e555b628 Mon Sep 17 00:00:00 2001
From: Alex Lorenz
Date: Thu, 9 Jul 2020 15:10:49 -0700
Subject: [PATCH] allow use of ptrauth module from no_undeclared_includes
system modules
---
clang/lib/Basic/Module.cpp| 4
.../Inputs/ptrauth-include-from-darwin/module.modulemap | 8
.../Modules/Inputs/ptrauth-include-from-darwin/ptrauth.h | 1 +
.../Modules/Inputs/ptrauth-include-from-darwin/stddef.h | 1 +
clang/test/Modules/ptrauth-include-from-darwin.m | 6 ++
5 files changed, 20 insertions(+)
create mode 100644
clang/test/Modules/Inputs/ptrauth-include-from-darwin/module.modulemap
create mode 100644
clang/test/Modules/Inputs/ptrauth-include-from-darwin/ptrauth.h
create mode 100644
clang/test/Modules/Inputs/ptrauth-include-from-darwin/stddef.h
create mode 100644 clang/test/Modules/ptrauth-include-from-darwin.m
diff --git a/clang/lib/Basic/Module.cpp b/clang/lib/Basic/Module.cpp
index 256365d66bb907..bb212cde878826 100644
--- a/clang/lib/Basic/Module.cpp
+++ b/clang/lib/Basic/Module.cpp
@@ -305,6 +305,10 @@ bool Module::directlyUses(const Module *Requested) {
if (Requested->fullModuleNameIs({"_Builtin_stddef", "max_align_t"}) ||
Requested->fullModuleNameIs({"_Builtin_stddef_wint_t"}))
return true;
+ // Darwin is allowed is to use our builtin 'ptrauth.h' and its accompanying
+ // module.
+ if (!Requested->Parent && Requested->Name == "ptrauth")
+return true;
if (NoUndeclaredIncludes)
UndeclaredUses.insert(Requested);
diff --git
a/clang/test/Modules/Inputs/ptrauth-include-from-darwin/module.modulemap
b/clang/test/Modules/Inputs/ptrauth-include-from-darwin/module.modulemap
new file mode 100644
index 00..741b9bb1efc54d
--- /dev/null
+++ b/clang/test/Modules/Inputs/ptrauth-include-from-darwin/module.modulemap
@@ -0,0 +1,8 @@
+module libc [no_undeclared_includes] {
+ module stddef { header "stddef.h" export * }
+}
+
+module ptrauth {
+ header "ptrauth.h"
+ export *
+}
diff --git a/clang/test/Modules/Inputs/ptrauth-include-from-darwin/ptrauth.h
b/clang/test/Modules/Inputs/ptrauth-include-from-darwin/ptrauth.h
new file mode 100644
index 00..c8620b64b2ceef
--- /dev/null
+++ b/clang/test/Modules/Inputs/ptrauth-include-from-darwin/ptrauth.h
@@ -0,0 +1 @@
+void foo();
diff --git a/clang/test/Modules/Inputs/ptrauth-include-from-darwin/stddef.h
b/clang/test/Modules/Inputs/ptrauth-include-from-darwin/stddef.h
new file mode 100644
index 00..777a524fc67110
--- /dev/null
+++ b/clang/test/Modules/Inputs/ptrauth-include-from-darwin/stddef.h
@@ -0,0 +1 @@
+@import ptrauth;
diff --git a/clang/test/Modules/ptrauth-include-from-darwin.m
b/clang/test/Modules/ptrauth-include-from-darwin.m
new file mode 100644
index 00..72b0c36e7cb7d3
--- /dev/null
+++ b/clang/test/Modules/ptrauth-include-from-darwin.m
@@ -0,0 +1,6 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -I
%S/Inputs/ptrauth-include-from-darwin %s -verify
+// expected-no-diagnostics
+
+@import libc;
+void bar() { foo(); }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Headers meant to be included multiple times can be completely invisible in clang module builds (PR #83660)
ian-twilightcoder wrote: > It's definitely caused by that change, reverting it fixes the failure. > > We use module maps so that some of our `#include` get processed as `#import`, > I believe the same code gets executed for our use-case. Because our setup is > so unusual, producing a repro is hard and make take some time. (Build system > generated module maps and `#include` is essentially turned into `#import` by > Clang). clang never transforms `#include` to `#import`. It will effectively attempt to transform `#include` and `#import` to `@import` but that's different. https://github.com/llvm/llvm-project/pull/83660 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Headers meant to be included multiple times can be completely invisible in clang module builds (PR #83660)
ian-twilightcoder wrote: Are your `textual` headers meant to be included more than once? If not, do they use header guards or `#pragma once`? https://github.com/llvm/llvm-project/pull/83660 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Headers meant to be included multiple times can be completely invisible in clang module builds (PR #83660)
ian-twilightcoder wrote: > @ian-twilightcoder this change seemed to cause our internal builds to run out > of source location space. > > ``` > remark: source manager location address space usage: [-Rsloc-usage] > note: 408559B in local locations, 2146921126B in locations loaded from AST > files, for a total of 2147329685B (99% of available space) > ``` > > The main offenders from the follow-ups are: > > * module map files for standard library and C/C++ runtime headers, each > entered a few thousand times, > * various headers without header guards, e.g. libc++'s `__config` and others, > * `arm_neon.h`. > Without looking deeper into what the change does, this might be an expected > outcome here. However, let me know if something already looks off from this > small description (e.g. the module maps entered so many times is surprising, > I'll need to see if this happened before). > > I will investigate this further, but we might not be able to change how our > builds work quickly. Would it be ok to revert this change or put the new > behavior under a flag to give us some more time to investigate? That's surprising, the code to re-enter files is `HeaderSearch::ShouldEnterIncludeFile`, and I only changed the behavior there for `#import`. The `#include` behavior is still governed by `#pragma once` and `FileInfo.getControllingMacro`, and that isn't changed. Are you sure it's this change? Do you have a repro case? https://github.com/llvm/llvm-project/pull/83660 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Headers meant to be included multiple times can be completely invisible in clang module builds (PR #83660)
https://github.com/ian-twilightcoder closed https://github.com/llvm/llvm-project/pull/83660 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Headers meant to be included multiple times can be completely invisible in clang module builds (PR #83660)
https://github.com/ian-twilightcoder updated
https://github.com/llvm/llvm-project/pull/83660
>From 63ff00ec49ac20c5ac97bd673166dabb0fb56136 Mon Sep 17 00:00:00 2001
From: Ian Anderson
Date: Fri, 1 Mar 2024 22:17:09 -0800
Subject: [PATCH] [clang][modules] Headers meant to be included multiple times
can be completely invisible in clang module builds
Once a file has been `#import`'ed, it gets stamped as if it was `#pragma once`
and will not be re-entered, even on #include. This means that any errant
#import of a file designed to be included multiple times, such as ,
will incorrectly mark it as include-once and break the multiple include
functionality. Normally this isn't a big problem, e.g. can't have
its NDEBUG mode changed after the first #import, but it is still mostly
functional. However, when clang modules are involved, this can cause the header
to be hidden entirely.
Objective-C code most often uses #import for everything, because it's required
for most Objective-C headers to prevent double inclusion and redeclaration
errors. (It's rare for Objective-C headers to use macro guards or `#pragma
once`.) The problem arises when a submodule includes a multiple-include header.
The "already included" state is global across all modules (which is necessary
so that non-modular headers don't get compiled into multiple translation units
and cause redeclaration errors). If another module or the main file #import's
the same header, it becomes invisible from then on. If the original submodule
is not imported, the include of the header will effectively do nothing and the
header will be invisible. The only way to actually get the header's
declarations is to somehow figure out which submodule consumed the header, and
import that instead. That's basically impossible since it depends on exactly
which modules were built in which order.
#import is a poor indicator of whether a header is actually include-once, as
the #import is external to the header it applies to, and requires that all
inclusions correctly and consistently use #import vs #include. When modules are
enabled, consider a header marked `textual` in its module as a stronger
indicator of multiple-include than #import's indication of include-once. This
will allow headers like to always be included when modules are
enabled, even if #import is erroneously used somewhere.
---
clang/include/clang/Lex/HeaderSearch.h | 26 ++-
clang/lib/Lex/HeaderSearch.cpp | 183 +--
clang/lib/Serialization/ASTReader.cpp| 2 +-
clang/test/Modules/builtin-import.mm | 2 +
clang/test/Modules/import-textual-noguard.mm | 6 +-
clang/test/Modules/import-textual.mm | 2 +
clang/test/Modules/multiple-import.m | 43 +
7 files changed, 201 insertions(+), 63 deletions(-)
create mode 100644 clang/test/Modules/multiple-import.m
diff --git a/clang/include/clang/Lex/HeaderSearch.h
b/clang/include/clang/Lex/HeaderSearch.h
index 705dcfa8aacc3f..855f81f775f8a8 100644
--- a/clang/include/clang/Lex/HeaderSearch.h
+++ b/clang/include/clang/Lex/HeaderSearch.h
@@ -78,11 +78,19 @@ struct HeaderFileInfo {
LLVM_PREFERRED_TYPE(bool)
unsigned External : 1;
- /// Whether this header is part of a module.
+ /// Whether this header is part of and built with a module. i.e. it is
listed
+ /// in a module map, and is not `excluded` or `textual`. (same meaning as
+ /// `ModuleMap::isModular()`).
LLVM_PREFERRED_TYPE(bool)
unsigned isModuleHeader : 1;
- /// Whether this header is part of the module that we are building.
+ /// Whether this header is a `textual header` in a module.
+ LLVM_PREFERRED_TYPE(bool)
+ unsigned isTextualModuleHeader : 1;
+
+ /// Whether this header is part of the module that we are building, even if
it
+ /// doesn't build with the module. i.e. this will include `excluded` and
+ /// `textual` headers as well as normal headers.
LLVM_PREFERRED_TYPE(bool)
unsigned isCompilingModuleHeader : 1;
@@ -128,13 +136,20 @@ struct HeaderFileInfo {
HeaderFileInfo()
: isImport(false), isPragmaOnce(false), DirInfo(SrcMgr::C_User),
-External(false), isModuleHeader(false), isCompilingModuleHeader(false),
-Resolved(false), IndexHeaderMapHeader(false), IsValid(false) {}
+External(false), isModuleHeader(false), isTextualModuleHeader(false),
+isCompilingModuleHeader(false), Resolved(false),
+IndexHeaderMapHeader(false), IsValid(false) {}
/// Retrieve the controlling macro for this header file, if
/// any.
const IdentifierInfo *
getControllingMacro(ExternalPreprocessorSource *External);
+
+ /// Update the module membership bits based on the header role.
+ ///
+ /// isModuleHeader will potentially be set, but not cleared.
+ /// isTextualModuleHeader will be set or cleared based on the role update.
+ void mergeModuleMembership(ModuleMap::ModuleHeaderRole Role);
};
/// An external source of
[clang] [Headers] Don't declare unreachable() from stddef.h in C++ (PR #86748)
https://github.com/ian-twilightcoder closed https://github.com/llvm/llvm-project/pull/86748 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Headers] Don't declare unreachable() from stddef.h in C++ (PR #86748)
ian-twilightcoder wrote: > > Is that still worth mention in the release notes do you think? > > Oh duh, that's right, this requires someone to define the `__need` macro to > get it, so no, I don't think it needs a release note. Sorry for the noise! thanks for bearing with me on this! https://github.com/llvm/llvm-project/pull/86748 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Headers] Don't declare unreachable() from stddef.h in C++ (PR #86748)
ian-twilightcoder wrote: > LGTM, though we should have a release note about the change because we've > been exposing the macro since Clang 17. I don't think this warrant a > potentially breaking change notice, though, just a regular bugfix one. I think we weren't exposing `unreachable` in C++ at all in Clang 17, and it's only exposed in Clang 18 if something sets the new `__needs_unreachable`? I can still release note it if you think it's worth calling out. https://github.com/llvm/llvm-project/pull/86748 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Headers] Don't declare unreachable() from stddef.h in C++ (PR #86748)
ian-twilightcoder wrote: > > > FWIW, I did verify that it's very unlikely the changes in this PR will > > > break existing code: > > > https://sourcegraph.com/search?q=context:global+__need_unreachable+-file:.*clang.*=keyword=0, > > > so that's a good thing. > > > > I do wonder if we could have the broader builtin headers discussion > > > > independent of this patch? Is everyone happy with this patch? We can > > > > keep talking about the builtin headers in here independent of merging > > > > right? > > > > > > > > > I guess I don't see these as independent topics; if we decide that C++ > > > mode should not have observable differences in C headers, then the > > > changes here are incorrect. I think we should have this discussion in a > > > broader context (like Discourse) before moving forward with these changes. > > > Also, I'd still like an explanation for [this > > > question](https://github.com/llvm/llvm-project/pull/86748#issuecomment-2023145043): > > > > I don't understand why this is making into C++ builds at all: > > > > > > > > > because it may turn out we don't need these changes in the first place > > > because the issue is elsewhere. > > > > > > Right now I just noticed that in a C++ test I was writing that stddef.h > > alone doesn't give me unreachable, but __needs_unreachable does. And that's > > probably wrong because unreachable belongs to in C++ and _not_ stddef.h. > > The C++ standard has some frustrating divergence with the C standard as to > > what the c stdlib headers declare... > > I don't yet agree that it's wrong -- you define the macro saying you want > `unreachable` from `stddef.h`, so you get `unreachable` from `stddef.h`. > Morally, it's very similar to: > > ``` > #define unreachable "I am doing something which causes myself pain" > #include > ``` I was thinking that a lot of the low level Apple SDK C headers use the `__need_` macros to add things they use, and that shouldn't mess up C++ clients. Although I guess if they actually use `unreachable` then they need to have a `__cplusplus` in there to include \ instead of just not getting `unreachable` at all. So, I guess there's a problem either way. However, I still don't really like the idea that someone can make a mistake in the SDK and break the C++ declaration of `unreachable`. I also don't like that `unreachable` is currently inconsistent with `nullptr_t`, which we almost never declare in C++ mode either, for similar reasons. https://github.com/llvm/llvm-project/pull/86748 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Headers] Don't declare unreachable() from stddef.h in C++ (PR #86748)
ian-twilightcoder wrote: > FWIW, I did verify that it's very unlikely the changes in this PR will break > existing code: > https://sourcegraph.com/search?q=context:global+__need_unreachable+-file:.*clang.*=keyword=0, > so that's a good thing. > > > I do wonder if we could have the broader builtin headers discussion > > independent of this patch? Is everyone happy with this patch? We can keep > > talking about the builtin headers in here independent of merging right? > > I guess I don't see these as independent topics; if we decide that C++ mode > should not have observable differences in C headers, then the changes here > are incorrect. I think we should have this discussion in a broader context > (like Discourse) before moving forward with these changes. > > Also, I'd still like an explanation for [this > question](https://github.com/llvm/llvm-project/pull/86748#issuecomment-2023145043): > > > I don't understand why this is making into C++ builds at all: > > because it may turn out we don't need these changes in the first place > because the issue is elsewhere. Right now I just noticed that in a C++ test I was writing that stddef.h alone doesn't give me unreachable, but __needs_unreachable does. And that's probably wrong because unreachable belongs to in C++ and _not_ stddef.h. The C++ standard has some frustrating divergence with the C standard as to what the c stdlib headers declare... https://github.com/llvm/llvm-project/pull/86748 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Headers] Don't declare unreachable() from stddef.h in C++ (PR #86748)
ian-twilightcoder wrote: I do wonder if we could have the broader builtin headers discussion independent of this patch? Is everyone happy with this patch? We can keep talking about the builtin headers in here independent of merging right? https://github.com/llvm/llvm-project/pull/86748 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Headers] Don't declare unreachable() from stddef.h in C++ (PR #86748)
ian-twilightcoder wrote: @ldionne @AaronBallman is there anything I need to do before merging this? https://github.com/llvm/llvm-project/pull/86748 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Headers] Don't declare unreachable() from stddef.h in C++ (PR #86748)
ian-twilightcoder wrote: > I still don't understand how that works in case you do end up including a > header from the platform that (re)defines `unreachable`, for example. > > The same problem also applies today to e.g. `size_t` and anything else > provided by the Clang builtin headers. If a platform decides to define > `size_t` differently than what the Clang builtin headers define it to, I > guess we run into issues? I assume it's not something that happens often > because it's pretty unambiguous what these typedefs should be, but still. We do indeed run into issues, the redeclarations cause two problems. 1. Modules get upset when different modules declare the same name somewhat differently. 2. Swift uses the module name as part of the type name, and ambiguities arise when different modules define the same type, even identically. We're needing to carefully remove our duplications in the Apple headers and always use the clang builtins. The coupling is unfortunate, but I don't know of any practical way around it. https://github.com/llvm/llvm-project/pull/86748 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Headers] Don't declare unreachable() from stddef.h in C++ (PR #86748)
ian-twilightcoder wrote: > (for example, we now include `string.h` and `stdbit.h` in freestanding, both > of which provide a lot of function interfaces) We do? I don't see those in lib/Headers https://github.com/llvm/llvm-project/pull/86748 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Headers] Don't declare unreachable() from stddef.h in C++ (PR #86748)
ian-twilightcoder wrote: > I'm checking with the C and C++ Compatibility study group (SG22) for what's > expected here. Prior to adding `__need_unreachable` in LLVM 18, clang would never declare `unreachable()` in C++ mode, but would defer to the C++ library to do that. I think we should keep with that behavior and let libc++'s and possibly headers handle declaring `unreachable()`. https://github.com/llvm/llvm-project/pull/86748 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Headers] Don't declare unreachable() from stddef.h in C++ (PR #86748)
https://github.com/ian-twilightcoder updated https://github.com/llvm/llvm-project/pull/86748 >From e67c757c7cdd1837008e573295e87e3ebec5382d Mon Sep 17 00:00:00 2001 From: Ian Anderson Date: Tue, 26 Mar 2024 16:19:38 -0700 Subject: [PATCH] [Headers] Don't declare unreachable() from stddef.h in C++ Even if __need_unreachable is set, stddef.h should not declare unreachable() in C++ because it conflicts with the declaration in . --- clang/lib/Headers/__stddef_unreachable.h | 4 1 file changed, 4 insertions(+) diff --git a/clang/lib/Headers/__stddef_unreachable.h b/clang/lib/Headers/__stddef_unreachable.h index 518580c92d3f5d..61df43e9732f8a 100644 --- a/clang/lib/Headers/__stddef_unreachable.h +++ b/clang/lib/Headers/__stddef_unreachable.h @@ -7,6 +7,8 @@ *===---=== */ +#ifndef __cplusplus + /* * When -fbuiltin-headers-in-system-modules is set this is a non-modular header * and needs to behave as if it was textual. @@ -15,3 +17,5 @@ (__has_feature(modules) && !__building_module(_Builtin_stddef)) #define unreachable() __builtin_unreachable() #endif + +#endif ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Headers] Don't declare unreachable() from stddef.h in C++ (PR #86748)
https://github.com/ian-twilightcoder edited https://github.com/llvm/llvm-project/pull/86748 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Headers] Don't declare unreachable() from stddef.h in C++ (PR #86748)
https://github.com/ian-twilightcoder created https://github.com/llvm/llvm-project/pull/86748 Even if __need_unreachable is set, stddef.h should not declare unreachable() in C++ because it conflicts with the declaration in . >From bfa16401ee26425492ce52daabe4144ea70da973 Mon Sep 17 00:00:00 2001 From: Ian Anderson Date: Tue, 26 Mar 2024 16:19:38 -0700 Subject: [PATCH] [Headers] Don't declare unreachable() from stddef.h in C++ Even if __need_unreachable is set, stddef.h should not declare unreachable() in C++ because it conflicts with the declaration in . --- clang/lib/Headers/__stddef_unreachable.h | 4 1 file changed, 4 insertions(+) diff --git a/clang/lib/Headers/__stddef_unreachable.h b/clang/lib/Headers/__stddef_unreachable.h index 518580c92d3f5d..61df43e9732f8a 100644 --- a/clang/lib/Headers/__stddef_unreachable.h +++ b/clang/lib/Headers/__stddef_unreachable.h @@ -7,6 +7,8 @@ *===---=== */ +#ifndef __cplusplus + /* * When -fbuiltin-headers-in-system-modules is set this is a non-modular header * and needs to behave as if it was textual. @@ -15,3 +17,5 @@ (__has_feature(modules) && !__building_module(_Builtin_stddef)) #define unreachable() __builtin_unreachable() #endif + +#endif ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] giving the __stddef_ headers their own modules can cause redeclaration errors with -fbuiltin-headers-in-system-modules (PR #84127)
https://github.com/ian-twilightcoder closed https://github.com/llvm/llvm-project/pull/84127 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] giving the __stddef_ headers their own modules can cause redeclaration errors with -fbuiltin-headers-in-system-modules (PR #84127)
@@ -301,10 +301,9 @@ bool Module::directlyUses(const Module *Requested) {
if (Requested->isSubModuleOf(Use))
return true;
- // Anyone is allowed to use our builtin stdarg.h and stddef.h and their
- // accompanying modules.
- if (Requested->getTopLevelModuleName() == "_Builtin_stdarg" ||
- Requested->getTopLevelModuleName() == "_Builtin_stddef")
+ // Anyone is allowed to use our builtin stddef.h and its accompanying
modules.
+ if (Requested->fullModuleNameIs({"_Builtin_stddef", "max_align_t"}) ||
+ Requested->fullModuleNameIs({"_Builtin_stddef_wint_t"}))
ian-twilightcoder wrote:
When `-fbuiltin-headers-in-system-modules` is set, the module map parser
doesn't add any headers to the `_Builtin` modules except for
__stddef_max_align_t.h and __stddef_wint_t.h.
https://github.com/llvm/llvm-project/pull/84127
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] giving the __stddef_ headers their own modules can cause redeclaration errors with -fbuiltin-headers-in-system-modules (PR #84127)
@@ -301,10 +301,9 @@ bool Module::directlyUses(const Module *Requested) {
if (Requested->isSubModuleOf(Use))
return true;
- // Anyone is allowed to use our builtin stdarg.h and stddef.h and their
- // accompanying modules.
- if (Requested->getTopLevelModuleName() == "_Builtin_stdarg" ||
- Requested->getTopLevelModuleName() == "_Builtin_stddef")
+ // Anyone is allowed to use our builtin stddef.h and its accompanying
modules.
+ if (Requested->fullModuleNameIs({"_Builtin_stddef", "max_align_t"}) ||
+ Requested->fullModuleNameIs({"_Builtin_stddef_wint_t"}))
ian-twilightcoder wrote:
We're taking `[no_undeclared_includes]` to imply
`-fbuiltin-headers-in-system-modules`, where `_Builtin_stdarg` is empty. The
assumption is that `[no_undeclared_includes]` is a workaround for the C++
headers not layering when the C stdlib headers are all in the same module,
which is also what `-fbuiltin-headers-in-system-modules` is for. If we don't
think that's a good assumption, then maybe we should add all of the clang C
stdlib modules here (but it's been fine so far keeping the assumption).
https://github.com/llvm/llvm-project/pull/84127
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] giving the __stddef_ headers their own modules can cause redeclaration errors with -fbuiltin-headers-in-system-modules (PR #84127)
ian-twilightcoder wrote: > I'm not excited by the complexity we are moving toward with the builtin > headers. But I don't have any alternatives. > > Given the situation we are in, I think the change is ok. But I'd like someone > else to look at it as it is tricky to reason about it. No blockers from me > but want more people to review the change. It's not my favorite. Hopefully Apple will be able to move off of `-fbuiltin-headers-in-system-modules` in a few years, including the Linux and Windows module maps in Swift. There may not be any other users, I haven't heard any other feedback about the new modules behavior yet. When `-fbuiltin-headers-in-system-modules` goes away, things become simpler than they were since modules were introduced. https://github.com/llvm/llvm-project/pull/84127 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Headers meant to be included multiple times can be completely invisible in clang module builds (PR #83660)
ian-twilightcoder wrote: > > Sometimes it does confuse clang, at least I saw problems with a `typedef > > enum` when I made an include-once header `textual`. > > Ok, I see. I would just consider it a bug, not a design decision. > > > That's correct. `#import` is an external source - often it comes from the > > users of the header and not the author, and the users might not be > > consistent with each other. `textual` comes from the author and a much > > stronger indicator of intent. > > Hmm, I need to think more about it. Usually I prefer for modular builds to > act like non-modular as it minimizes the surprises and allows to have a > portable code. But I haven't really considered the implications of such > preference. Yes generally, but modules build more headers than non-modular builds, nothing we can do about that. > I see why you prefer to give more priority to `textual`. To give a different > perspective, what if a developer wants to use a newer library, for example, > ncurses. The rest of SDK expects an older version and the developer might > need to do some header gymnastics to make it work. So there are cases when > developers need to go against library author's intent. I'm thinking that > library authors aren't able to predict all the ways their library is used, so > it is useful to have some flexibility around that, even at the cost of > misusing the library. But this is a general opinion, I don't have specific > data about the needs to tweak libraries or the risks of a library misuse. So > the opinion isn't particularly strong. That one seems like an edge case, and using `#import` wouldn't necessarily help you because the SDK module could build first before your `#import` is seen. https://github.com/llvm/llvm-project/pull/83660 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] giving the __stddef_ headers their own modules can cause redeclaration errors with -fbuiltin-headers-in-system-modules (PR #84127)
@@ -7,6 +7,11 @@ *===---=== */ -#ifndef offsetof +/* + * When -fbuiltin-headers-in-system-modules is set this is a non-modular header + * and needs to behave as if it was textual. + */ +#if !defined(offsetof) || \ +(__has_feature(modules) && !__building_module(_Builtin_stddef)) ian-twilightcoder wrote: Oh I see what you mean. When the Darwin module builds, it will build which will include <__stddef_offsetof.h>. It will bypass the header guard and (re)define `offsetof`. The Darwin module will also build , so which one gets used is undefined, but the Darwin module will be the single definer of `offsetof`. It's a little weird, but this actually matches the LLVM 17 behavior. https://github.com/llvm/llvm-project/pull/84127 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] giving the __stddef_ headers their own modules can cause redeclaration errors with -fbuiltin-headers-in-system-modules (PR #84127)
https://github.com/ian-twilightcoder edited https://github.com/llvm/llvm-project/pull/84127 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] giving the __stddef_ headers their own modules can cause redeclaration errors with -fbuiltin-headers-in-system-modules (PR #84127)
@@ -7,6 +7,11 @@ *===---=== */ -#ifndef offsetof +/* + * When -fbuiltin-headers-in-system-modules is set this is a non-modular header + * and needs to behave as if it was textual. + */ +#if !defined(offsetof) || \ +(__has_feature(modules) && !__building_module(_Builtin_stddef)) ian-twilightcoder wrote: This header shouldn't be seen at all, and `_Builtin_stddef.offset` should instead be imported/made visible https://github.com/llvm/llvm-project/pull/84127 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] giving the __stddef_ headers their own modules can cause redeclaration errors with -fbuiltin-headers-in-system-modules (PR #84127)
@@ -2498,9 +2498,12 @@ void ModuleMapParser::parseHeaderDecl(MMToken::TokenKind
LeadingToken,
}
bool NeedsFramework = false;
- // Don't add the top level headers to the builtin modules if the builtin
headers
- // belong to the system modules.
- if (!Map.LangOpts.BuiltinHeadersInSystemModules ||
ActiveModule->isSubModule() || !isBuiltInModuleName(ActiveModule->Name))
+ // Don't add headers to the builtin modules if the builtin headers belong to
+ // the system modules, with the exception of __stddef_max_align_t.h which
+ // always had its own module.
+ if (!Map.LangOpts.BuiltinHeadersInSystemModules ||
+ !isBuiltInModuleName(ActiveModule->getTopLevelModuleName()) ||
+ ActiveModule->fullModuleNameIs({"_Builtin_stddef", "max_align_t"}))
ian-twilightcoder wrote:
I don't really know the right answer, __stddef_wint_t.h is a weird one.
Strictly speaking it wasn't modular so anyone could import it previously. But
then it's not really supposed to be part of stddef.h, and you have to
specifically opt into seeing it., i.e. if you just include stddef.h you never
got __stddef_wint_t.h. So maybe it's ok that it's unconditionally in its own
module. Or maybe it needs to be added to `isBuiltInModuleName`.
https://github.com/llvm/llvm-project/pull/84127
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Headers meant to be included multiple times can be completely invisible in clang module builds (PR #83660)
ian-twilightcoder wrote: > To clarify a little bit > > > [...] The "already included" state is global across all modules (which is > > necessary so that non-modular headers don't get compiled into multiple > > translation units and cause redeclaration errors). > > The necessity isn't actually true. The same definition in multiple modules > shouldn't confuse clang as long as these definitions are identical. But this > is a side issue. Sometimes it does confuse clang, at least I saw problems with a `typedef enum` when I made an include-once header `textual`. > To re-iterate what this change is about: > > 1. `#import` implies a file is a single-include > 2. Textual header in a module map implies a file is a multi-include (aka > re-entrant) > 3. When we have both `#import` and the header marked as textual, `#import` > "wins", i.e. the file is single-include > 4. You want to make that when we have both `#import` and a textual header, > textual should "win", i.e. the file should be multi-include > > Is it correct? That's correct. `#import` is an external source - often it comes from the users of the header and not the author, and the users might not be consistent with each other. `textual` comes from the author and a much stronger indicator of intent. https://github.com/llvm/llvm-project/pull/83660 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Headers meant to be included multiple times can be completely invisible in clang module builds (PR #83660)
https://github.com/ian-twilightcoder updated
https://github.com/llvm/llvm-project/pull/83660
>From 1cb3d459f3a9ae73ac98bf8c06b905d788be954f Mon Sep 17 00:00:00 2001
From: Ian Anderson
Date: Fri, 1 Mar 2024 22:17:09 -0800
Subject: [PATCH] [clang][modules] Headers meant to be included multiple times
can be completely invisible in clang module builds
Once a file has been `#import`'ed, it gets stamped as if it was `#pragma once`
and will not be re-entered, even on #include. This means that any errant
#import of a file designed to be included multiple times, such as ,
will incorrectly mark it as include-once and break the multiple include
functionality. Normally this isn't a big problem, e.g. can't have
its NDEBUG mode changed after the first #import, but it is still mostly
functional. However, when clang modules are involved, this can cause the header
to be hidden entirely.
Objective-C code most often uses #import for everything, because it's required
for most Objective-C headers to prevent double inclusion and redeclaration
errors. (It's rare for Objective-C headers to use macro guards or `#pragma
once`.) The problem arises when a submodule includes a multiple-include header.
The "already included" state is global across all modules (which is necessary
so that non-modular headers don't get compiled into multiple translation units
and cause redeclaration errors). If another module or the main file #import's
the same header, it becomes invisible from then on. If the original submodule
is not imported, the include of the header will effectively do nothing and the
header will be invisible. The only way to actually get the header's
declarations is to somehow figure out which submodule consumed the header, and
import that instead. That's basically impossible since it depends on exactly
which modules were built in which order.
#import is a poor indicator of whether a header is actually include-once, as
the #import is external to the header it applies to, and requires that all
inclusions correctly and consistently use #import vs #include. When modules are
enabled, consider a header marked `textual` in its module as a stronger
indicator of multiple-include than #import's indication of include-once. This
will allow headers like to always be included when modules are
enabled, even if #import is erroneously used somewhere.
---
clang/include/clang/Lex/HeaderSearch.h | 26 ++-
clang/lib/Lex/HeaderSearch.cpp | 183 +--
clang/lib/Serialization/ASTReader.cpp| 2 +-
clang/test/Modules/builtin-import.mm | 2 +
clang/test/Modules/import-textual-noguard.mm | 6 +-
clang/test/Modules/import-textual.mm | 2 +
clang/test/Modules/multiple-import.m | 43 +
7 files changed, 201 insertions(+), 63 deletions(-)
create mode 100644 clang/test/Modules/multiple-import.m
diff --git a/clang/include/clang/Lex/HeaderSearch.h
b/clang/include/clang/Lex/HeaderSearch.h
index 705dcfa8aacc3f..4c9fb58fbd35ef 100644
--- a/clang/include/clang/Lex/HeaderSearch.h
+++ b/clang/include/clang/Lex/HeaderSearch.h
@@ -78,11 +78,19 @@ struct HeaderFileInfo {
LLVM_PREFERRED_TYPE(bool)
unsigned External : 1;
- /// Whether this header is part of a module.
+ /// Whether this header is part of and built with a module
+ /// (`isTextualModuleHeader` will be `false`).
LLVM_PREFERRED_TYPE(bool)
unsigned isModuleHeader : 1;
- /// Whether this header is part of the module that we are building.
+ /// Whether this header is a textual header in a module (`isModuleHeader`
will
+ /// be `false`).
+ LLVM_PREFERRED_TYPE(bool)
+ unsigned isTextualModuleHeader : 1;
+
+ /// Whether this header is part of the module that we are building
+ /// (independent of `isModuleHeader` and `isTextualModuleHeader`, they can
+ /// both be `false`).
LLVM_PREFERRED_TYPE(bool)
unsigned isCompilingModuleHeader : 1;
@@ -128,13 +136,20 @@ struct HeaderFileInfo {
HeaderFileInfo()
: isImport(false), isPragmaOnce(false), DirInfo(SrcMgr::C_User),
-External(false), isModuleHeader(false), isCompilingModuleHeader(false),
-Resolved(false), IndexHeaderMapHeader(false), IsValid(false) {}
+External(false), isModuleHeader(false), isTextualModuleHeader(false),
+isCompilingModuleHeader(false), Resolved(false),
+IndexHeaderMapHeader(false), IsValid(false) {}
/// Retrieve the controlling macro for this header file, if
/// any.
const IdentifierInfo *
getControllingMacro(ExternalPreprocessorSource *External);
+
+ /// Update the module membership bits based on the header role.
+ ///
+ /// isModuleHeader will potentially be set, but not cleared.
+ /// isTextualModuleHeader will be set or cleared based on the role update.
+ void mergeModuleMembership(ModuleMap::ModuleHeaderRole Role);
};
/// An external source of header file information, which may supply
@@ -522,6 +537,9 @@ class HeaderSearch {
[clang] [clang][modules] giving the __stddef_ headers their own modules can cause redeclaration errors with -fbuiltin-headers-in-system-modules (PR #84127)
ian-twilightcoder wrote: > Still need to check the code but > > > Make the __stddef_ headers be non-modular in > > -fbuiltin-headers-in-system-modules and restore them back to not respecting > > their header guards. Still define the header guards though. > > sounds quite questionable. They were non-modular prior to https://reviews.llvm.org/D159064 https://github.com/llvm/llvm-project/pull/84127 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] giving the __stddef_ headers their own modules can cause redeclaration errors with -fbuiltin-headers-in-system-modules (PR #84127)
https://github.com/ian-twilightcoder edited https://github.com/llvm/llvm-project/pull/84127 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] giving the __stddef_ headers their own modules can cause redeclaration errors with -fbuiltin-headers-in-system-modules (PR #84127)
https://github.com/ian-twilightcoder edited https://github.com/llvm/llvm-project/pull/84127 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] giving the __stddef_ headers their own modules can cause redeclaration errors with -fbuiltin-headers-in-system-modules (PR #84127)
https://github.com/ian-twilightcoder updated
https://github.com/llvm/llvm-project/pull/84127
>From e34ccad2b82b75c050d12bbb987529c320c0df9d Mon Sep 17 00:00:00 2001
From: Ian Anderson
Date: Tue, 5 Mar 2024 22:56:36 -0800
Subject: [PATCH] [clang][modules] giving the __stddef_ headers their own
modules can cause redeclaration errors with
-fbuiltin-headers-in-system-modules
On Apple platforms, some of the stddef.h types are also declared in system
headers. In particular NULL has a conflicting declaration in
. When that's in a different module from <__stddef_null.h>,
redeclaration errors can occur.
Make the __stddef_ headers be non-modular in
-fbuiltin-headers-in-system-modules and restore them back to not respecting
their header guards. Still define the header guards though.
__stddef_max_align_t.h was in _Builtin_stddef_max_align_t prior to the addition
of _Builtin_stddef, and it needs to stay in a module because struct's can't be
type merged. __stddef_wint_t.h didn't used to have a module, but leave it in it
current module since it doesn't really belong to stddef.h.
---
clang/lib/Basic/Module.cpp| 7 ++--
clang/lib/Headers/__stddef_null.h | 2 +-
clang/lib/Headers/__stddef_nullptr_t.h| 7 +++-
clang/lib/Headers/__stddef_offsetof.h | 7 +++-
clang/lib/Headers/__stddef_ptrdiff_t.h| 7 +++-
clang/lib/Headers/__stddef_rsize_t.h | 7 +++-
clang/lib/Headers/__stddef_size_t.h | 7 +++-
clang/lib/Headers/__stddef_unreachable.h | 7 +++-
clang/lib/Headers/__stddef_wchar_t.h | 7 +++-
clang/lib/Headers/module.modulemap| 20 ++--
clang/lib/Lex/ModuleMap.cpp | 9 --
.../no-undeclared-includes-builtins.cpp | 2 +-
clang/test/Modules/stddef.c | 32 +++
13 files changed, 80 insertions(+), 41 deletions(-)
diff --git a/clang/lib/Basic/Module.cpp b/clang/lib/Basic/Module.cpp
index 1c5043a618fff3..f68a556fb455bf 100644
--- a/clang/lib/Basic/Module.cpp
+++ b/clang/lib/Basic/Module.cpp
@@ -301,10 +301,9 @@ bool Module::directlyUses(const Module *Requested) {
if (Requested->isSubModuleOf(Use))
return true;
- // Anyone is allowed to use our builtin stdarg.h and stddef.h and their
- // accompanying modules.
- if (Requested->getTopLevelModuleName() == "_Builtin_stdarg" ||
- Requested->getTopLevelModuleName() == "_Builtin_stddef")
+ // Anyone is allowed to use our builtin stddef.h and its accompanying
modules.
+ if (Requested->fullModuleNameIs({"_Builtin_stddef", "max_align_t"}) ||
+ Requested->fullModuleNameIs({"_Builtin_stddef_wint_t"}))
return true;
if (NoUndeclaredIncludes)
diff --git a/clang/lib/Headers/__stddef_null.h
b/clang/lib/Headers/__stddef_null.h
index 7336fdab389723..c10bd2d7d9887c 100644
--- a/clang/lib/Headers/__stddef_null.h
+++ b/clang/lib/Headers/__stddef_null.h
@@ -7,7 +7,7 @@
*===---===
*/
-#if !defined(NULL) || !__has_feature(modules)
+#if !defined(NULL) || !__building_module(_Builtin_stddef)
/* linux/stddef.h will define NULL to 0. glibc (and other) headers then define
* __need_NULL and rely on stddef.h to redefine NULL to the correct value
again.
diff --git a/clang/lib/Headers/__stddef_nullptr_t.h
b/clang/lib/Headers/__stddef_nullptr_t.h
index 183d394d56c1b7..7f3fbe6fe0d3a8 100644
--- a/clang/lib/Headers/__stddef_nullptr_t.h
+++ b/clang/lib/Headers/__stddef_nullptr_t.h
@@ -7,7 +7,12 @@
*===---===
*/
-#ifndef _NULLPTR_T
+/*
+ * When -fbuiltin-headers-in-system-modules is set this is a non-modular header
+ * and needs to behave as if it was textual.
+ */
+#if !defined(_NULLPTR_T) ||
\
+(__has_feature(modules) && !__building_module(_Builtin_stddef))
#define _NULLPTR_T
#ifdef __cplusplus
diff --git a/clang/lib/Headers/__stddef_offsetof.h
b/clang/lib/Headers/__stddef_offsetof.h
index 3b347b3b92f62c..84172c6cd27352 100644
--- a/clang/lib/Headers/__stddef_offsetof.h
+++ b/clang/lib/Headers/__stddef_offsetof.h
@@ -7,6 +7,11 @@
*===---===
*/
-#ifndef offsetof
+/*
+ * When -fbuiltin-headers-in-system-modules is set this is a non-modular header
+ * and needs to behave as if it was textual.
+ */
+#if !defined(offsetof) ||
\
+(__has_feature(modules) && !__building_module(_Builtin_stddef))
#define offsetof(t, d) __builtin_offsetof(t, d)
#endif
diff --git a/clang/lib/Headers/__stddef_ptrdiff_t.h
b/clang/lib/Headers/__stddef_ptrdiff_t.h
index 3ea6d7d2852e1c..fd3c893c66c979 100644
--- a/clang/lib/Headers/__stddef_ptrdiff_t.h
+++ b/clang/lib/Headers/__stddef_ptrdiff_t.h
@@ -7,7 +7,12 @@
[clang] [clang][modules] giving the __stddef_ headers their own modules can cause redeclaration errors with -fbuiltin-headers-in-system-modules (PR #84127)
@@ -7,7 +7,7 @@ *===---=== */ -#if !defined(NULL) || !__has_feature(modules) +#if !defined(NULL) || !__building_module(_Builtin_stddef) ian-twilightcoder wrote: ``` #if !defined(NULL) || !__has_feature(modules) || (__has_feature(modules) && !__building_module(_Builtin_stddef)) ``` If `!__has_feature(modules)` then `__building_module(anything)` is `0`. So you can make these substitutions without changing the logic. ``` !__has_feature(modules) !__has_feature(modules) && !__building_module(_Builtin_stddef) !__has_feature(modules) || (__has_feature(modules) && !__building_module(_Builtin_stddef)) (!__has_feature(modules) && !__building_module(_Builtin_stddef)) || (__has_feature(modules) && !__building_module(_Builtin_stddef)) (!a && b) || (a && b) b (!__has_feature(modules) && !__building_module(_Builtin_stddef)) || (__has_feature(modules) && !__building_module(_Builtin_stddef)) !__building_module(_Builtin_stddef) ``` https://github.com/llvm/llvm-project/pull/84127 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] giving the __stddef_ headers their own modules can cause redeclaration errors with -fbuiltin-headers-in-system-modules (PR #84127)
ian-twilightcoder wrote: We should consider this for LLVM 18, it's a regression from https://reviews.llvm.org/D159064 https://github.com/llvm/llvm-project/pull/84127 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] giving the __stddef_ headers their own modules can cause redeclaration errors with -fbuiltin-headers-in-system-modules (PR #84127)
https://github.com/ian-twilightcoder created
https://github.com/llvm/llvm-project/pull/84127
On Apple platforms, some of the stddef.h types are also declared in system
headers. In particular NULL has a conflicting declaration in
. When that's in a different module from <__stddef_null.h>,
redeclaration errors can occur.
Make the __stddef_ headers be non-modular in
-fbuiltin-headers-in-system-modules and restore them back to not respecting
their header guards. Still define the header guards though.
__stddef_max_align_t.h was in _Builtin_stddef_max_align_t prior to the addition
of _Builtin_stddef, and it needs to stay in a module because struct's can't be
type merged. __stddef_wint_t.h didn't used to have a module, but leave it in it
current module since it doesn't really belong to stddef.h.
>From 0cc4b77fce06730f6a6a8b242384036018ebfe79 Mon Sep 17 00:00:00 2001
From: Ian Anderson
Date: Tue, 5 Mar 2024 22:56:36 -0800
Subject: [PATCH] [clang][modules] giving the __stddef_ headers their own
modules can cause redeclaration errors with
-fbuiltin-headers-in-system-modules
On Apple platforms, some of the stddef.h types are also declared in system
headers. In particular NULL has a conflicting declaration in
. When that's in a different module from <__stddef_null.h>,
redeclaration errors can occur.
Make the __stddef_ headers be non-modular in
-fbuiltin-headers-in-system-modules and restore them back to not respecting
their header guards. Still define the header guards though.
__stddef_max_align_t.h was in _Builtin_stddef_max_align_t prior to the addition
of _Builtin_stddef, and it needs to stay in a module because struct's can't be
type merged. __stddef_wint_t.h didn't used to have a module, but leave it in it
current module since it doesn't really belong to stddef.h.
---
clang/lib/Basic/Module.cpp| 7 ++--
clang/lib/Headers/__stddef_null.h | 2 +-
clang/lib/Headers/__stddef_nullptr_t.h| 6 +++-
clang/lib/Headers/__stddef_offsetof.h | 6 +++-
clang/lib/Headers/__stddef_ptrdiff_t.h| 6 +++-
clang/lib/Headers/__stddef_rsize_t.h | 6 +++-
clang/lib/Headers/__stddef_size_t.h | 6 +++-
clang/lib/Headers/__stddef_unreachable.h | 6 +++-
clang/lib/Headers/__stddef_wchar_t.h | 6 +++-
clang/lib/Headers/module.modulemap| 20 ++--
clang/lib/Lex/ModuleMap.cpp | 9 --
.../no-undeclared-includes-builtins.cpp | 2 +-
clang/test/Modules/stddef.c | 32 +++
13 files changed, 73 insertions(+), 41 deletions(-)
diff --git a/clang/lib/Basic/Module.cpp b/clang/lib/Basic/Module.cpp
index 1c5043a618fff3..f68a556fb455bf 100644
--- a/clang/lib/Basic/Module.cpp
+++ b/clang/lib/Basic/Module.cpp
@@ -301,10 +301,9 @@ bool Module::directlyUses(const Module *Requested) {
if (Requested->isSubModuleOf(Use))
return true;
- // Anyone is allowed to use our builtin stdarg.h and stddef.h and their
- // accompanying modules.
- if (Requested->getTopLevelModuleName() == "_Builtin_stdarg" ||
- Requested->getTopLevelModuleName() == "_Builtin_stddef")
+ // Anyone is allowed to use our builtin stddef.h and its accompanying
modules.
+ if (Requested->fullModuleNameIs({"_Builtin_stddef", "max_align_t"}) ||
+ Requested->fullModuleNameIs({"_Builtin_stddef_wint_t"}))
return true;
if (NoUndeclaredIncludes)
diff --git a/clang/lib/Headers/__stddef_null.h
b/clang/lib/Headers/__stddef_null.h
index 7336fdab389723..c10bd2d7d9887c 100644
--- a/clang/lib/Headers/__stddef_null.h
+++ b/clang/lib/Headers/__stddef_null.h
@@ -7,7 +7,7 @@
*===---===
*/
-#if !defined(NULL) || !__has_feature(modules)
+#if !defined(NULL) || !__building_module(_Builtin_stddef)
/* linux/stddef.h will define NULL to 0. glibc (and other) headers then define
* __need_NULL and rely on stddef.h to redefine NULL to the correct value
again.
diff --git a/clang/lib/Headers/__stddef_nullptr_t.h
b/clang/lib/Headers/__stddef_nullptr_t.h
index 183d394d56c1b7..d724b5cba18294 100644
--- a/clang/lib/Headers/__stddef_nullptr_t.h
+++ b/clang/lib/Headers/__stddef_nullptr_t.h
@@ -7,7 +7,11 @@
*===---===
*/
-#ifndef _NULLPTR_T
+/*
+ * When -fbuiltin-headers-in-system-modules is set this is a non-modular header
+ * and needs to behave as if it was textual.
+ */
+#if !defined(_NULLPTR_T) || (__has_feature(modules) &&
!__building_module(_Builtin_stddef))
#define _NULLPTR_T
#ifdef __cplusplus
diff --git a/clang/lib/Headers/__stddef_offsetof.h
b/clang/lib/Headers/__stddef_offsetof.h
index 3b347b3b92f62c..62c49c78bd0516 100644
--- a/clang/lib/Headers/__stddef_offsetof.h
+++ b/clang/lib/Headers/__stddef_offsetof.h
@@ -7,6 +7,10 @@
*===---===
*/
[clang] [clang][modules] Headers meant to be included multiple times can be completely invisible in clang module builds (PR #83660)
ian-twilightcoder wrote: > Thanks to @Bigcheese for helping with this patch. > > https://reviews.llvm.org/D26267 looks like the most recent significant update > to this code that we could find. It had an additional allowance where clang > headers that were _not_ marked `textual` could be re-entered. I think this > was to allow stddef.h (which is actually a multiple-include file despite it > not being `textual`) to be entered multiple times while building the Darwin > module. That shouldn't be necessary since nothing in the Darwin module or > libc++ defines the `__need_` macros. > > The unit tests added in D26267 still pass, so I don't think the builtin > special case is needed anymore, but I'm still doing testing. @bcardosolopes as far as I can tell the builtin special case isn't needed. Can you think of anything else I might need to test? https://github.com/llvm/llvm-project/pull/83660 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Headers meant to be included multiple times can be completely invisible in clang module builds (PR #83660)
https://github.com/ian-twilightcoder updated
https://github.com/llvm/llvm-project/pull/83660
>From f90fe8b1b7c73b68614ade3cf7e1c292f02d3573 Mon Sep 17 00:00:00 2001
From: Ian Anderson
Date: Fri, 1 Mar 2024 22:17:09 -0800
Subject: [PATCH] [clang][modules] Headers meant to be included multiple times
can be completely invisible in clang module builds
Once a file has been `#import`'ed, it gets stamped as if it was `#pragma once`
and will not be re-entered, even on #include. This means that any errant
#import of a file designed to be included multiple times, such as ,
will incorrectly mark it as include-once and break the multiple include
functionality. Normally this isn't a big problem, e.g. can't have
its NDEBUG mode changed after the first #import, but it is still mostly
functional. However, when clang modules are involved, this can cause the header
to be hidden entirely.
Objective-C code most often uses #import for everything, because it's required
for most Objective-C headers to prevent double inclusion and redeclaration
errors. (It's rare for Objective-C headers to use macro guards or `#pragma
once`.) The problem arises when a submodule includes a multiple-include header.
The "already included" state is global across all modules (which is necessary
so that non-modular headers don't get compiled into multiple translation units
and cause redeclaration errors). If another module or the main file #import's
the same header, it becomes invisible from then on. If the original submodule
is not imported, the include of the header will effectively do nothing and the
header will be invisible. The only way to actually get the header's
declarations is to somehow figure out which submodule consumed the header, and
import that instead. That's basically impossible since it depends on exactly
which modules were built in which order.
#import is a poor indicator of whether a header is actually include-once, as
the #import is external to the header it applies to, and requires that all
inclusions correctly and consistently use #import vs #include. When modules are
enabled, consider a header marked `textual` in its module as a stronger
indicator of multiple-include than #import's indication of include-once. This
will allow headers like to always be included when modules are
enabled, even if #import is erroneously used somewhere.
---
clang/include/clang/Lex/HeaderSearch.h | 26 +++-
clang/lib/Lex/HeaderSearch.cpp | 154 ---
clang/lib/Serialization/ASTReader.cpp| 2 +-
clang/test/Modules/builtin-import.mm | 2 +
clang/test/Modules/import-textual-noguard.mm | 6 +-
clang/test/Modules/import-textual.mm | 2 +
clang/test/Modules/multiple-import.m | 43 ++
7 files changed, 173 insertions(+), 62 deletions(-)
create mode 100644 clang/test/Modules/multiple-import.m
diff --git a/clang/include/clang/Lex/HeaderSearch.h
b/clang/include/clang/Lex/HeaderSearch.h
index 705dcfa8aacc3f8..4c9fb58fbd35ef1 100644
--- a/clang/include/clang/Lex/HeaderSearch.h
+++ b/clang/include/clang/Lex/HeaderSearch.h
@@ -78,11 +78,19 @@ struct HeaderFileInfo {
LLVM_PREFERRED_TYPE(bool)
unsigned External : 1;
- /// Whether this header is part of a module.
+ /// Whether this header is part of and built with a module
+ /// (`isTextualModuleHeader` will be `false`).
LLVM_PREFERRED_TYPE(bool)
unsigned isModuleHeader : 1;
- /// Whether this header is part of the module that we are building.
+ /// Whether this header is a textual header in a module (`isModuleHeader`
will
+ /// be `false`).
+ LLVM_PREFERRED_TYPE(bool)
+ unsigned isTextualModuleHeader : 1;
+
+ /// Whether this header is part of the module that we are building
+ /// (independent of `isModuleHeader` and `isTextualModuleHeader`, they can
+ /// both be `false`).
LLVM_PREFERRED_TYPE(bool)
unsigned isCompilingModuleHeader : 1;
@@ -128,13 +136,20 @@ struct HeaderFileInfo {
HeaderFileInfo()
: isImport(false), isPragmaOnce(false), DirInfo(SrcMgr::C_User),
-External(false), isModuleHeader(false), isCompilingModuleHeader(false),
-Resolved(false), IndexHeaderMapHeader(false), IsValid(false) {}
+External(false), isModuleHeader(false), isTextualModuleHeader(false),
+isCompilingModuleHeader(false), Resolved(false),
+IndexHeaderMapHeader(false), IsValid(false) {}
/// Retrieve the controlling macro for this header file, if
/// any.
const IdentifierInfo *
getControllingMacro(ExternalPreprocessorSource *External);
+
+ /// Update the module membership bits based on the header role.
+ ///
+ /// isModuleHeader will potentially be set, but not cleared.
+ /// isTextualModuleHeader will be set or cleared based on the role update.
+ void mergeModuleMembership(ModuleMap::ModuleHeaderRole Role);
};
/// An external source of header file information, which may supply
@@ -522,6 +537,9 @@ class HeaderSearch
[clang] [clang][modules] Headers meant to be included multiple times can be completely invisible in clang module builds (PR #83660)
https://github.com/ian-twilightcoder updated
https://github.com/llvm/llvm-project/pull/83660
>From 08681ff77f432806316109146ab4fef058137648 Mon Sep 17 00:00:00 2001
From: Ian Anderson
Date: Fri, 1 Mar 2024 22:17:09 -0800
Subject: [PATCH] [clang][modules] Headers meant to be included multiple times
can be completely invisible in clang module builds
Once a file has been `#import`'ed, it gets stamped as if it was `#pragma once`
and will not be re-entered, even on #include. This means that any errant
#import of a file designed to be included multiple times, such as ,
will incorrectly mark it as include-once and break the multiple include
functionality. Normally this isn't a big problem, e.g. can't have
its NDEBUG mode changed after the first #import, but it is still mostly
functional. However, when clang modules are involved, this can cause the header
to be hidden entirely.
Objective-C code most often uses #import for everything, because it's required
for most Objective-C headers to prevent double inclusion and redeclaration
errors. (It's rare for Objective-C headers to use macro guards or `#pragma
once`.) The problem arises when a submodule includes a multiple-include header.
The "already included" state is global across all modules (which is necessary
so that non-modular headers don't get compiled into multiple translation units
and cause redeclaration errors). If another module or the main file #import's
the same header, it becomes invisible from then on. If the original submodule
is not imported, the include of the header will effectively do nothing and the
header will be invisible. The only way to actually get the header's
declarations is to somehow figure out which submodule consumed the header, and
import that instead. That's basically impossible since it depends on exactly
which modules were built in which order.
#import is a poor indicator of whether a header is actually include-once, as
the #import is external to the header it applies to, and requires that all
inclusions correctly and consistently use #import vs #include. When modules are
enabled, consider a header marked `textual` in its module as a stronger
indicator of multiple-include than #import's indication of include-once. This
will allow headers like to always be included when modules are
enabled, even if #import is erroneously used somewhere.
---
clang/include/clang/Lex/HeaderSearch.h | 26 +++-
clang/lib/Lex/HeaderSearch.cpp | 154 ---
clang/lib/Serialization/ASTReader.cpp| 2 +-
clang/test/Modules/builtin-import.mm | 2 +
clang/test/Modules/import-textual-noguard.mm | 6 +-
clang/test/Modules/import-textual.mm | 2 +
clang/test/Modules/multiple-import.m | 43 ++
7 files changed, 173 insertions(+), 62 deletions(-)
create mode 100644 clang/test/Modules/multiple-import.m
diff --git a/clang/include/clang/Lex/HeaderSearch.h
b/clang/include/clang/Lex/HeaderSearch.h
index 705dcfa8aacc3f8..4c9fb58fbd35ef1 100644
--- a/clang/include/clang/Lex/HeaderSearch.h
+++ b/clang/include/clang/Lex/HeaderSearch.h
@@ -78,11 +78,19 @@ struct HeaderFileInfo {
LLVM_PREFERRED_TYPE(bool)
unsigned External : 1;
- /// Whether this header is part of a module.
+ /// Whether this header is part of and built with a module
+ /// (`isTextualModuleHeader` will be `false`).
LLVM_PREFERRED_TYPE(bool)
unsigned isModuleHeader : 1;
- /// Whether this header is part of the module that we are building.
+ /// Whether this header is a textual header in a module (`isModuleHeader`
will
+ /// be `false`).
+ LLVM_PREFERRED_TYPE(bool)
+ unsigned isTextualModuleHeader : 1;
+
+ /// Whether this header is part of the module that we are building
+ /// (independent of `isModuleHeader` and `isTextualModuleHeader`, they can
+ /// both be `false`).
LLVM_PREFERRED_TYPE(bool)
unsigned isCompilingModuleHeader : 1;
@@ -128,13 +136,20 @@ struct HeaderFileInfo {
HeaderFileInfo()
: isImport(false), isPragmaOnce(false), DirInfo(SrcMgr::C_User),
-External(false), isModuleHeader(false), isCompilingModuleHeader(false),
-Resolved(false), IndexHeaderMapHeader(false), IsValid(false) {}
+External(false), isModuleHeader(false), isTextualModuleHeader(false),
+isCompilingModuleHeader(false), Resolved(false),
+IndexHeaderMapHeader(false), IsValid(false) {}
/// Retrieve the controlling macro for this header file, if
/// any.
const IdentifierInfo *
getControllingMacro(ExternalPreprocessorSource *External);
+
+ /// Update the module membership bits based on the header role.
+ ///
+ /// isModuleHeader will potentially be set, but not cleared.
+ /// isTextualModuleHeader will be set or cleared based on the role update.
+ void mergeModuleMembership(ModuleMap::ModuleHeaderRole Role);
};
/// An external source of header file information, which may supply
@@ -522,6 +537,9 @@ class HeaderSearch
[clang] [clang][modules] Headers meant to be included multiple times can be completely invisible in clang module builds (PR #83660)
ian-twilightcoder wrote: > > Once a file has been `#import`'ed, it gets stamped as if it was `#pragma > > once` and will not be re-entered, even on #include. > > Can you explain how this is happening? The only place where > `HeaderFileInfo::isPragmaOnce` is set to `true` is in > `HeaderSearch::MarkFileIncludeOnce()`, only called from > `Preprocessor::HandlePragmaOnce()`, which seems correct. It gets `HeaderFileInfo::isImport` which causes it to be treated the same as `#pragma once` from then on. > > The problem arises when a submodule includes a multiple-include header. The > > "already included" state is global across all modules (which is necessary > > so that non-modular headers don't get compiled into multiple translation > > units and cause redeclaration errors). If another module or the main file > > #import's the same header, it becomes invisible from then on. If the > > original submodule is not imported, the include of the header will > > effectively do nothing and the header will be invisible. The only way to > > actually get the header's declarations is to somehow figure out which > > submodule consumed the header, and import that instead. That's basically > > impossible since it depends on exactly which modules were built in which > > order. > > Could this be also solved by tracking the "already included" state [per > submodule](https://github.com/llvm/llvm-project/pull/71117)? That would fix headers that can be included multiple times (which is what I'm trying to fix here). But I think that would break headers that can only be included once and are not part of a module, since their declarations would go into multiple pcms. https://github.com/llvm/llvm-project/pull/83660 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Headers meant to be included multiple times can be completely invisible in clang module builds (PR #83660)
ian-twilightcoder wrote: ``` error: 'expected-error' diagnostics seen but not expected: File /var/lib/buildkite-agent/builds/linux-56-59b8f5d88-6mhbj-1/llvm-project/clang-ci/clang/test/Modules/explicit-build-overlap.cpp Line 11: module use does not directly depend on a module exporting 'a.h', which is part of indirectly-used module b 1 error generated. ``` https://github.com/llvm/llvm-project/pull/83660 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Headers meant to be included multiple times can be completely invisible in clang module builds (PR #83660)
https://github.com/ian-twilightcoder updated
https://github.com/llvm/llvm-project/pull/83660
>From 771c0740dcc438d99baf4ad9555bc57e963001df Mon Sep 17 00:00:00 2001
From: Ian Anderson
Date: Fri, 1 Mar 2024 22:17:09 -0800
Subject: [PATCH] [clang][modules] Headers meant to be included multiple times
can be completely invisible in clang module builds
Once a file has been `#import`'ed, it gets stamped as if it was `#pragma once`
and will not be re-entered, even on #include. This means that any errant
#import of a file designed to be included multiple times, such as ,
will incorrectly mark it as include-once and break the multiple include
functionality. Normally this isn't a big problem, e.g. can't have
its NDEBUG mode changed after the first #import, but it is still mostly
functional. However, when clang modules are involved, this can cause the header
to be hidden entirely.
Objective-C code most often uses #import for everything, because it's required
for most Objective-C headers to prevent double inclusion and redeclaration
errors. (It's rare for Objective-C headers to use macro guards or `#pragma
once`.) The problem arises when a submodule includes a multiple-include header.
The "already included" state is global across all modules (which is necessary
so that non-modular headers don't get compiled into multiple translation units
and cause redeclaration errors). If another module or the main file #import's
the same header, it becomes invisible from then on. If the original submodule
is not imported, the include of the header will effectively do nothing and the
header will be invisible. The only way to actually get the header's
declarations is to somehow figure out which submodule consumed the header, and
import that instead. That's basically impossible since it depends on exactly
which modules were built in which order.
#import is a poor indicator of whether a header is actually include-once, as
the #import is external to the header it applies to, and requires that all
inclusions correctly and consistently use #import vs #include. When modules are
enabled, consider a header marked `textual` in its module as a stronger
indicator of multiple-include than #import's indication of include-once. This
will allow headers like to always be included when modules are
enabled, even if #import is erroneously used somewhere.
---
clang/include/clang/Lex/HeaderSearch.h | 24 ++-
clang/lib/Lex/HeaderSearch.cpp | 157 ---
clang/lib/Serialization/ASTReader.cpp| 2 +-
clang/test/Modules/builtin-import.mm | 2 +
clang/test/Modules/import-textual-noguard.mm | 6 +-
clang/test/Modules/import-textual.mm | 2 +
clang/test/Modules/multiple-import.m | 43 +
7 files changed, 173 insertions(+), 63 deletions(-)
create mode 100644 clang/test/Modules/multiple-import.m
diff --git a/clang/include/clang/Lex/HeaderSearch.h
b/clang/include/clang/Lex/HeaderSearch.h
index 705dcfa8aacc3f..bf8981b94d1b57 100644
--- a/clang/include/clang/Lex/HeaderSearch.h
+++ b/clang/include/clang/Lex/HeaderSearch.h
@@ -78,11 +78,17 @@ struct HeaderFileInfo {
LLVM_PREFERRED_TYPE(bool)
unsigned External : 1;
- /// Whether this header is part of a module.
+ /// Whether this header is part of and built with a module.
LLVM_PREFERRED_TYPE(bool)
unsigned isModuleHeader : 1;
- /// Whether this header is part of the module that we are building.
+ /// Whether this header is a textual header in a module (isModuleHeader will
+ /// be false)
+ LLVM_PREFERRED_TYPE(bool)
+ unsigned isTextualModuleHeader : 1;
+
+ /// Whether this header is part of and built with the module that we are
+ /// building.
LLVM_PREFERRED_TYPE(bool)
unsigned isCompilingModuleHeader : 1;
@@ -128,13 +134,20 @@ struct HeaderFileInfo {
HeaderFileInfo()
: isImport(false), isPragmaOnce(false), DirInfo(SrcMgr::C_User),
-External(false), isModuleHeader(false), isCompilingModuleHeader(false),
-Resolved(false), IndexHeaderMapHeader(false), IsValid(false) {}
+External(false), isModuleHeader(false), isTextualModuleHeader(false),
+isCompilingModuleHeader(false), Resolved(false),
+IndexHeaderMapHeader(false), IsValid(false) {}
/// Retrieve the controlling macro for this header file, if
/// any.
const IdentifierInfo *
getControllingMacro(ExternalPreprocessorSource *External);
+
+ /// Update the module membership bits based on the header role.
+ ///
+ /// isModuleHeader will potentially be set, but not cleared.
+ /// isTextualModuleHeader will be set or cleared based on the role update.
+ void mergeModuleMembership(ModuleMap::ModuleHeaderRole Role);
};
/// An external source of header file information, which may supply
@@ -522,6 +535,9 @@ class HeaderSearch {
///
/// \return false if \#including the file will have no effect or true
/// if we should include it.
+ ///
+ /// \param M
[clang] [clang][modules] Headers meant to be included multiple times can be completely invisible in clang module builds (PR #83660)
ian-twilightcoder wrote: Thanks to @Bigcheese for helping with this patch. https://reviews.llvm.org/D26267 looks like the most recent significant update to this code that we could find. It had an additional allowance where clang headers that were _not_ marked `textual` could be re-entered. I think this was to allow stddef.h (which is actually a multiple-include file despite it not being `textual`) to be entered multiple times while building the Darwin module. That shouldn't be necessary since nothing in the Darwin module or libc++ defines the `__need_` macros. The unit tests added in D26267 still pass, so I don't think the builtin special case is needed anymore, but I'm still doing testing. https://github.com/llvm/llvm-project/pull/83660 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Headers meant to be included multiple times can be completely invisible in clang module builds (PR #83660)
https://github.com/ian-twilightcoder created https://github.com/llvm/llvm-project/pull/83660 Once a file has been `#import`'ed, it gets stamped as if it was `#pragma once` and will not be re-entered, even on #include. This means that any errant #import of a file designed to be included multiple times, such as , will incorrectly mark it as include-once and break the multiple include functionality. Normally this isn't a big problem, e.g. can't have its NDEBUG mode changed after the first #import, but it is still mostly functional. However, when clang modules are involved, this can cause the header to be hidden entirely. Objective-C code most often uses #import for everything, because it's required for most Objective-C headers to prevent double inclusion and redeclaration errors. (It's rare for Objective-C headers to use macro guards or `#pragma once`.) The problem arises when a submodule includes a multiple-include header. The "already included" state is global across all modules (which is necessary so that non-modular headers don't get compiled into multiple translation units and cause redeclaration errors). If another module or the main file #import's the same header, it becomes invisible from then on. If the original submodule is not imported, the include of the header will effectively do nothing and the header will be invisible. The only way to actually get the header's declarations is to somehow figure out which submodule consumed the header, and import that instead. That's basically impossible since it depends on exactly which modules were built in which order. #import is a poor indicator of whether a header is actually include-once, as the #import is external to the header it applies to, and requires that all inclusions correctly and consistently use #import vs #include. When modules are enabled, consider a header marked `textual` in its module as a stronger indicator of multiple-include than #import's indication of include-once. This will allow headers like to always be included when modules are enabled, even if #import is erroneously used somewhere. >From 171d0e299dd676ce29583e16fdf8c3e6f3dd7925 Mon Sep 17 00:00:00 2001 From: Ian Anderson Date: Fri, 1 Mar 2024 22:17:09 -0800 Subject: [PATCH] [clang][modules] Headers meant to be included multiple times can be completely invisible in clang module builds Once a file has been `#import`'ed, it gets stamped as if it was `#pragma once` and will not be re-entered, even on #include. This means that any errant #import of a file designed to be included multiple times, such as , will incorrectly mark it as include-once and break the multiple include functionality. Normally this isn't a big problem, e.g. can't have its NDEBUG mode changed after the first #import, but it is still mostly functional. However, when clang modules are involved, this can cause the header to be hidden entirely. Objective-C code most often uses #import for everything, because it's required for most Objective-C headers to prevent double inclusion and redeclaration errors. (It's rare for Objective-C headers to use macro guards or `#pragma once`.) The problem arises when a submodule includes a multiple-include header. The "already included" state is global across all modules (which is necessary so that non-modular headers don't get compiled into multiple translation units and cause redeclaration errors). If another module or the main file #import's the same header, it becomes invisible from then on. If the original submodule is not imported, the include of the header will effectively do nothing and the header will be invisible. The only way to actually get the header's declarations is to somehow figure out which submodule consumed the header, and import that instead. That's basically impossible since it depends on exactly which modules were built in which order. #import is a poor indicator of whether a header is actually include-once, as the #import is external to the header it applies to, and requires that all inclusions correctly and consistently use #import vs #include. When modules are enabled, consider a header marked `textual` in its module as a stronger indicator of multiple-include than #import's indication of include-once. This will allow headers like to always be included when modules are enabled, even if #import is erroneously used somewhere. --- clang/include/clang/Lex/HeaderSearch.h | 24 ++- clang/lib/Lex/HeaderSearch.cpp | 156 --- clang/lib/Serialization/ASTReader.cpp| 2 +- clang/test/Modules/builtin-import.mm | 2 + clang/test/Modules/import-textual-noguard.mm | 6 +- clang/test/Modules/import-textual.mm | 2 + clang/test/Modules/multiple-import.m | 43 + 7 files changed, 172 insertions(+), 63 deletions(-) create mode 100644 clang/test/Modules/multiple-import.m diff --git a/clang/include/clang/Lex/HeaderSearch.h
[clang] [Docs] Add release note about Clang-defined target OS macros (PR #79879)
https://github.com/ian-twilightcoder approved this pull request. https://github.com/llvm/llvm-project/pull/79879 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Remove the builtin_headers_in_system_modules feature (PR #75262)
https://github.com/ian-twilightcoder closed https://github.com/llvm/llvm-project/pull/75262 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Remove the builtin_headers_in_system_modules feature (PR #75262)
https://github.com/ian-twilightcoder created
https://github.com/llvm/llvm-project/pull/75262
__has_feature(builtin_headers_in_system_modules) was added in
https://reviews.llvm.org/D159483 to be used in the stdarg/stddef implementation
headers. It ended up being unnecessary, but I forgot to remove the feature
definition.
>From 120be30b2056b6f03d692d1e79a2f4ef60d5e5f4 Mon Sep 17 00:00:00 2001
From: Ian Anderson
Date: Tue, 12 Dec 2023 16:33:33 -0800
Subject: [PATCH] Remove the builtin_headers_in_system_modules feature
__has_feature(builtin_headers_in_system_modules) was added in
https://reviews.llvm.org/D159483 to be used in the stdarg/stddef implementation
headers. It ended up being unnecessary, but I forgot to remove the feature
definition.
---
clang/include/clang/Basic/Features.def | 1 -
clang/test/Driver/darwin-builtin-modules.c | 16
2 files changed, 17 deletions(-)
diff --git a/clang/include/clang/Basic/Features.def
b/clang/include/clang/Basic/Features.def
index 7473e00a7bd86b..06efac0cf1abd7 100644
--- a/clang/include/clang/Basic/Features.def
+++ b/clang/include/clang/Basic/Features.def
@@ -282,7 +282,6 @@ EXTENSION(matrix_types_scalar_division, true)
EXTENSION(cxx_attributes_on_using_declarations, LangOpts.CPlusPlus11)
EXTENSION(datasizeof, LangOpts.CPlusPlus)
-FEATURE(builtin_headers_in_system_modules,
LangOpts.BuiltinHeadersInSystemModules)
FEATURE(cxx_abi_relative_vtable, LangOpts.CPlusPlus &&
LangOpts.RelativeCXXABIVTables)
// CUDA/HIP Features
diff --git a/clang/test/Driver/darwin-builtin-modules.c
b/clang/test/Driver/darwin-builtin-modules.c
index 215f2b8d2c142a..1c56e13bfb9293 100644
--- a/clang/test/Driver/darwin-builtin-modules.c
+++ b/clang/test/Driver/darwin-builtin-modules.c
@@ -9,19 +9,3 @@
// RUN: %clang -isysroot %S/Inputs/MacOSX99.0.sdk -target
x86_64-apple-macos98.0 -### %s 2>&1 | FileCheck --check-prefix=CHECK_FUTURE %s
// RUN: %clang -isysroot %S/Inputs/MacOSX99.0.sdk -target
x86_64-apple-macos99.0 -### %s 2>&1 | FileCheck --check-prefix=CHECK_FUTURE %s
// CHECK_FUTURE-NOT: -fbuiltin-headers-in-system-modules
-
-
-// Check that builtin_headers_in_system_modules is only set if
-fbuiltin-headers-in-system-modules and -fmodules are both set.
-
-// RUN: %clang -isysroot %S/Inputs/iPhoneOS13.0.sdk -target
arm64-apple-ios13.0 -fsyntax-only %s -Xclang -verify=no-feature
-// RUN: %clang -isysroot %S/Inputs/iPhoneOS13.0.sdk -target
arm64-apple-ios13.0 -fsyntax-only %s -fmodules -Xclang -verify=yes-feature
-// RUN: %clang -isysroot %S/Inputs/MacOSX99.0.sdk -target
x86_64-apple-macos99.0 -fsyntax-only %s -Xclang -verify=no-feature
-// RUN: %clang -isysroot %S/Inputs/MacOSX99.0.sdk -target
x86_64-apple-macos99.0 -fsyntax-only %s -fmodules -Xclang -verify=no-feature
-
-#if __has_feature(builtin_headers_in_system_modules)
-#error "has builtin_headers_in_system_modules"
-// yes-feature-error@-1 {{}}
-#else
-#error "no builtin_headers_in_system_modules"
-// no-feature-error@-1 {{}}
-#endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Fix] Disable fdefine-target-os-macros for now (PR #74886)
https://github.com/ian-twilightcoder approved this pull request. https://github.com/llvm/llvm-project/pull/74886 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][PP] Add extension to predefine target OS macros (PR #74676)
https://github.com/ian-twilightcoder approved this pull request. https://github.com/llvm/llvm-project/pull/74676 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][PP] Add extension to predefine target OS macros (PR #74676)
https://github.com/ian-twilightcoder approved this pull request. https://github.com/llvm/llvm-project/pull/74676 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][PP] Add extension to predefine target OS macros (PR #74676)
@@ -0,0 +1,131 @@ +// RUN: %clang -### --target=arm64-apple-darwin %s 2>&1 | FileCheck %s --check-prefix=DEFAULT-OPTION + +// RUN: %clang -dM -E --target=arm64-apple-macos %s 2>&1 \ +// RUN: | FileCheck %s -DMAC=1 \ +// RUN:-DOSX=1 \ +// RUN:-DIPHONE=0 \ +// RUN:-DIOS=0 \ +// RUN:-DTV=0 \ +// RUN:-DWATCH=0 \ +// RUN:-DDRIVERKIT=0 \ +// RUN:-DMACCATALYST=0 \ +// RUN:-DEMBEDDED=0\ +// RUN:-DSIMULATOR=0 + +// RUN: %clang -dM -E --target=arm64-apple-ios %s 2>&1 \ +// RUN: | FileCheck %s -DMAC=1 \ +// RUN:-DOSX=0 \ +// RUN:-DIPHONE=1 \ +// RUN:-DIOS=1 \ +// RUN:-DTV=0 \ +// RUN:-DWATCH=0 \ +// RUN:-DDRIVERKIT=0 \ +// RUN:-DMACCATALYST=0 \ +// RUN:-DEMBEDDED=1\ +// RUN:-DSIMULATOR=0 + +// RUN: %clang -dM -E --target=arm64-apple-ios-macabi %s 2>&1 \ +// RUN: | FileCheck %s -DMAC=1 \ +// RUN:-DOSX=0 \ +// RUN:-DIPHONE=1 \ +// RUN:-DIOS=1 \ +// RUN:-DTV=0 \ +// RUN:-DWATCH=0 \ +// RUN:-DDRIVERKIT=0 \ +// RUN:-DMACCATALYST=1 \ +// RUN:-DEMBEDDED=0\ +// RUN:-DSIMULATOR=0 + +// RUN: %clang -dM -E --target=arm64-apple-ios-simulator %s 2>&1 \ +// RUN: | FileCheck %s -DMAC=1 \ +// RUN:-DOSX=0 \ +// RUN:-DIPHONE=1 \ +// RUN:-DIOS=1 \ +// RUN:-DTV=0 \ +// RUN:-DWATCH=0 \ +// RUN:-DDRIVERKIT=0 \ +// RUN:-DMACCATALYST=0 \ +// RUN:-DEMBEDDED=0\ +// RUN:-DSIMULATOR=1 + +// RUN: %clang -dM -E --target=arm64-apple-tvos %s 2>&1 \ +// RUN: | FileCheck %s -DMAC=1 \ +// RUN:-DOSX=0 \ +// RUN:-DIPHONE=1 \ +// RUN:-DIOS=0 \ +// RUN:-DTV=1 \ +// RUN:-DWATCH=0 \ +// RUN:-DDRIVERKIT=0 \ +// RUN:-DMACCATALYST=0 \ +// RUN:-DEMBEDDED=1\ +// RUN:-DSIMULATOR=0 + +// RUN: %clang -dM -E --target=arm64-apple-tvos-simulator %s 2>&1 \ +// RUN: | FileCheck %s -DMAC=1 \ +// RUN:-DOSX=0 \ +// RUN:-DIPHONE=1 \ +// RUN:-DIOS=0 \ +// RUN:-DTV=1 \ +// RUN:-DWATCH=0 \ +// RUN:-DDRIVERKIT=0 \ +// RUN:-DMACCATALYST=0 \ +// RUN:-DEMBEDDED=0\ +// RUN:-DSIMULATOR=1 + +// RUN: %clang -dM -E --target=arm64-apple-watchos %s 2>&1 \ +// RUN: | FileCheck %s -DMAC=1 \ +// RUN:-DOSX=0 \ +// RUN:-DIPHONE=1 \ +// RUN:-DIOS=0 \ +// RUN:-DTV=0 \ +// RUN:-DWATCH=1 \ +// RUN:-DDRIVERKIT=0 \ +// RUN:-DMACCATALYST=0 \ +// RUN:-DEMBEDDED=1\ +// RUN:-DSIMULATOR=0 + +// RUN: %clang -dM -E --target=arm64-apple-watchos-simulator %s 2>&1 \ +// RUN: | FileCheck %s -DMAC=1 \ +// RUN:-DOSX=0 \ +// RUN:-DIPHONE=1 \ +// RUN:-DIOS=0 \ +// RUN:-DTV=0 \ +// RUN:-DWATCH=1 \ +// RUN:-DDRIVERKIT=0 \ +// RUN:-DMACCATALYST=0 \ +// RUN:-DEMBEDDED=0\ +// RUN:-DSIMULATOR=1 + +// RUN: %clang -dM -E --target=arm64-apple-driverkit %s 2>&1 \ +// RUN: | FileCheck %s -DMAC=1 \ +// RUN:-DOSX=0 \ +// RUN:-DIPHONE=0 \ +// RUN:-DIOS=0 \ +// RUN:-DTV=0 \ +// RUN:-DWATCH=0 \ +// RUN:-DDRIVERKIT=1 \ +// RUN:-DMACCATALYST=0 \ +// RUN:-DEMBEDDED=0\ +// RUN:-DSIMULATOR=0 + +// DEFAULT-OPTION: "-fdefine-target-os-macros" + +// CHECK-DAG: #define TARGET_OS_MAC [[MAC]] +// CHECK-DAG: #define TARGET_OS_OSX [[OSX]] +// CHECK-DAG: #define TARGET_OS_IPHONE [[IPHONE]] +// CHECK-DAG: #define TARGET_OS_IOS [[IOS]] +// CHECK-DAG: #define TARGET_OS_TV [[TV]] +// CHECK-DAG: #define TARGET_OS_WATCH [[WATCH]] +// CHECK-DAG: #define TARGET_OS_DRIVERKIT [[DRIVERKIT]] +// CHECK-DAG: #define TARGET_OS_MACCATALYST [[MACCATALYST]] +// CHECK-DAG: #define TARGET_OS_SIMULATOR [[SIMULATOR]] +// Deprecated
[clang] [Modules] textual headers in submodules never resolve their `use`s (PR #69651)
https://github.com/ian-twilightcoder closed https://github.com/llvm/llvm-project/pull/69651 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Modules] textual headers in submodules never resolve their `use`s (PR #69651)
https://github.com/ian-twilightcoder updated
https://github.com/llvm/llvm-project/pull/69651
>From 7b88bf3102240d1734feab8919a049f8a92ca0e0 Mon Sep 17 00:00:00 2001
From: Ian Anderson
Date: Thu, 19 Oct 2023 15:22:11 -0700
Subject: [PATCH] [Modules] textual headers in submodules never resolve their
`use`s
When an include from a textual header is resolved, the textual header's
submodule is used as the requesting module. The submodule's uses are resolved,
but that doesn't work because only top level modules have uses, and only the
top level module uses are used for checking uses in Module::directlyUses.
ModuleMap::resolveUses to resolve the top level module instead of the submodule.
---
clang/lib/Lex/ModuleMap.cpp | 13 +
clang/test/Modules/no-undeclared-includes.c | 31 +
2 files changed, 38 insertions(+), 6 deletions(-)
create mode 100644 clang/test/Modules/no-undeclared-includes.c
diff --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp
index f65a5f145c04395..259c97796ae19f2 100644
--- a/clang/lib/Lex/ModuleMap.cpp
+++ b/clang/lib/Lex/ModuleMap.cpp
@@ -1398,16 +1398,17 @@ bool ModuleMap::resolveExports(Module *Mod, bool
Complain) {
}
bool ModuleMap::resolveUses(Module *Mod, bool Complain) {
- auto Unresolved = std::move(Mod->UnresolvedDirectUses);
- Mod->UnresolvedDirectUses.clear();
+ auto *Top = Mod->getTopLevelModule();
+ auto Unresolved = std::move(Top->UnresolvedDirectUses);
+ Top->UnresolvedDirectUses.clear();
for (auto : Unresolved) {
-Module *DirectUse = resolveModuleId(UDU, Mod, Complain);
+Module *DirectUse = resolveModuleId(UDU, Top, Complain);
if (DirectUse)
- Mod->DirectUses.push_back(DirectUse);
+ Top->DirectUses.push_back(DirectUse);
else
- Mod->UnresolvedDirectUses.push_back(UDU);
+ Top->UnresolvedDirectUses.push_back(UDU);
}
- return !Mod->UnresolvedDirectUses.empty();
+ return !Top->UnresolvedDirectUses.empty();
}
bool ModuleMap::resolveConflicts(Module *Mod, bool Complain) {
diff --git a/clang/test/Modules/no-undeclared-includes.c
b/clang/test/Modules/no-undeclared-includes.c
new file mode 100644
index 000..83a654f6ed77539
--- /dev/null
+++ b/clang/test/Modules/no-undeclared-includes.c
@@ -0,0 +1,31 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: %clang_cc1 -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -I
%t %t/no-undeclared-includes.c -verify
+
+//--- no-undeclared-includes.c
+// expected-no-diagnostics
+#include
+
+//--- assert.h
+#include
+
+//--- base.h
+#ifndef base_h
+#define base_h
+
+
+
+#endif /* base_h */
+
+//--- module.modulemap
+module cstd [system] [no_undeclared_includes] {
+ use base
+ module assert {
+textual header "assert.h"
+ }
+}
+
+module base [system] {
+ header "base.h"
+ export *
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
