[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)
https://github.com/benlangmuir approved this pull request. 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"}))
benlangmuir wrote:
Ah, got it. Thanks for explaining.
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"}))
benlangmuir wrote:
Are we not considering the include of `stdarg.h` a use of `_Builtin_stdarg`
here if it doesn't use the modular headers from `_Builtin_stdarg`?
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)
benlangmuir wrote: >> I'm not excited by the complexity we are moving toward with the builtin >> headers. But I don't have any alternatives. > When -fbuiltin-headers-in-system-modules goes away, things become simpler > than they were since modules were introduced. Even for the case with `-fbuiltin-headers-in-system-modules` things aren't any more complex than they were before https://reviews.llvm.org/D159064 except for the fundamental complexity of having two modes to build in, and hopefully -fbuiltin-headers-in-system-modules goes away. 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"}))
benlangmuir wrote:
Why was stdarg removed?
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] giving the __stddef_ headers their own modules can cause redeclaration errors with -fbuiltin-headers-in-system-modules (PR #84127)
vsapsai 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. 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: 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)
@@ -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)) vsapsai wrote: I was thinking about `offsetof` being defined in some header, not in the intended system one. And then we [transitively] include "\_\_stddef_offsetof.h". Does it mean that in this case "\_\_stddef_offsetof.h" is processed only when 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)
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] 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)) vsapsai wrote: What should happen when `offsetof` is defined and we are building non-`_Builtin_stddef` module? 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"}))
vsapsai wrote:
Should `_Builtin_stddef_wint_t` be a part of this check too? I don't know the
right answer, just trying to understand.
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/vsapsai commented: Still kinda confused. Have a few questions trying to improve my understanding. 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/vsapsai 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)
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)
github-actions[bot] wrote: :warning: C/C++ code formatter, clang-format found issues in your code. :warning: You can test this locally with the following command: ``bash git-clang-format --diff bf631c63d01057321c070520a56a150ede32e47d 0cc4b77fce06730f6a6a8b242384036018ebfe79 -- clang/lib/Basic/Module.cpp clang/lib/Headers/__stddef_null.h clang/lib/Headers/__stddef_nullptr_t.h clang/lib/Headers/__stddef_offsetof.h clang/lib/Headers/__stddef_ptrdiff_t.h clang/lib/Headers/__stddef_rsize_t.h clang/lib/Headers/__stddef_size_t.h clang/lib/Headers/__stddef_unreachable.h clang/lib/Headers/__stddef_wchar_t.h clang/lib/Lex/ModuleMap.cpp clang/test/Modules/no-undeclared-includes-builtins.cpp clang/test/Modules/stddef.c `` View the diff from clang-format here. ``diff diff --git a/clang/lib/Headers/__stddef_nullptr_t.h b/clang/lib/Headers/__stddef_nullptr_t.h index d724b5cba1..7f3fbe6fe0 100644 --- a/clang/lib/Headers/__stddef_nullptr_t.h +++ b/clang/lib/Headers/__stddef_nullptr_t.h @@ -11,7 +11,8 @@ * 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)) +#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 62c49c78bd..84172c6cd2 100644 --- a/clang/lib/Headers/__stddef_offsetof.h +++ b/clang/lib/Headers/__stddef_offsetof.h @@ -11,6 +11,7 @@ * 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)) +#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 31ecc00b62..fd3c893c66 100644 --- a/clang/lib/Headers/__stddef_ptrdiff_t.h +++ b/clang/lib/Headers/__stddef_ptrdiff_t.h @@ -11,7 +11,8 @@ * 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(_PTRDIFF_T) || (__has_feature(modules) && !__building_module(_Builtin_stddef)) +#if !defined(_PTRDIFF_T) || \ +(__has_feature(modules) && !__building_module(_Builtin_stddef)) #define _PTRDIFF_T typedef __PTRDIFF_TYPE__ ptrdiff_t; diff --git a/clang/lib/Headers/__stddef_rsize_t.h b/clang/lib/Headers/__stddef_rsize_t.h index 9cb9c26611..dd433d40d9 100644 --- a/clang/lib/Headers/__stddef_rsize_t.h +++ b/clang/lib/Headers/__stddef_rsize_t.h @@ -11,7 +11,8 @@ * 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(_RSIZE_T) || (__has_feature(modules) && !__building_module(_Builtin_stddef)) +#if !defined(_RSIZE_T) || \ +(__has_feature(modules) && !__building_module(_Builtin_stddef)) #define _RSIZE_T typedef __SIZE_TYPE__ rsize_t; diff --git a/clang/lib/Headers/__stddef_size_t.h b/clang/lib/Headers/__stddef_size_t.h index 01d3a0c480..3dd7b1f379 100644 --- a/clang/lib/Headers/__stddef_size_t.h +++ b/clang/lib/Headers/__stddef_size_t.h @@ -11,7 +11,8 @@ * 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(_SIZE_T) || (__has_feature(modules) && !__building_module(_Builtin_stddef)) +#if !defined(_SIZE_T) || \ +(__has_feature(modules) && !__building_module(_Builtin_stddef)) #define _SIZE_T typedef __SIZE_TYPE__ size_t; diff --git a/clang/lib/Headers/__stddef_unreachable.h b/clang/lib/Headers/__stddef_unreachable.h index 1f9254f6bb..518580c92d 100644 --- a/clang/lib/Headers/__stddef_unreachable.h +++ b/clang/lib/Headers/__stddef_unreachable.h @@ -11,6 +11,7 @@ * 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(unreachable) || (__has_feature(modules) && !__building_module(_Builtin_stddef)) +#if !defined(unreachable) || \ +(__has_feature(modules) && !__building_module(_Builtin_stddef)) #define unreachable() __builtin_unreachable() #endif diff --git a/clang/lib/Headers/__stddef_wchar_t.h b/clang/lib/Headers/__stddef_wchar_t.h index 4239f619f0..bd69f63225 100644 --- a/clang/lib/Headers/__stddef_wchar_t.h +++
[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)
llvmbot wrote:
@llvm/pr-subscribers-backend-x86
@llvm/pr-subscribers-clang
Author: Ian Anderson (ian-twilightcoder)
Changes
On Apple platforms, some of the stddef.h types are also declared in system
headers. In particular NULL has a conflicting declaration in
sys/_types/_null.h. 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.
---
Full diff: https://github.com/llvm/llvm-project/pull/84127.diff
13 Files Affected:
- (modified) clang/lib/Basic/Module.cpp (+3-4)
- (modified) clang/lib/Headers/__stddef_null.h (+1-1)
- (modified) clang/lib/Headers/__stddef_nullptr_t.h (+5-1)
- (modified) clang/lib/Headers/__stddef_offsetof.h (+5-1)
- (modified) clang/lib/Headers/__stddef_ptrdiff_t.h (+5-1)
- (modified) clang/lib/Headers/__stddef_rsize_t.h (+5-1)
- (modified) clang/lib/Headers/__stddef_size_t.h (+5-1)
- (modified) clang/lib/Headers/__stddef_unreachable.h (+5-1)
- (modified) clang/lib/Headers/__stddef_wchar_t.h (+5-1)
- (modified) clang/lib/Headers/module.modulemap (+9-11)
- (modified) clang/lib/Lex/ModuleMap.cpp (+6-3)
- (modified) clang/test/Modules/no-undeclared-includes-builtins.cpp (+1-1)
- (modified) clang/test/Modules/stddef.c (+18-14)
``diff
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 @@
*===---===
*/
-#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..31ecc00b624602 100644
--- a/clang/lib/Headers/__stddef_ptrdiff_t.h
+++ b/clang/lib/Headers/__stddef_ptrdiff_t.h
@@ -7,7 +7,11 @@
*===---===
*/
-#ifndef _PTRDIFF_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(_PTRDIFF_T) || (__has_feature(modules) &&
!__building_module(_Builtin_stddef))
#define _PTRDIFF_T
typedef
[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 @@
*===---===
*/
