[clang] 260fb2b - [clang][modules] Don't hard code [no_undeclared_includes] for the Darwin module
Author: Ian Anderson
Date: 2022-08-30T14:57:15-07:00
New Revision: 260fb2bc3f79019cae4e182a64c8752d3d25049e
URL:
https://github.com/llvm/llvm-project/commit/260fb2bc3f79019cae4e182a64c8752d3d25049e
DIFF:
https://github.com/llvm/llvm-project/commit/260fb2bc3f79019cae4e182a64c8752d3d25049e.diff
LOG: [clang][modules] Don't hard code [no_undeclared_includes] for the Darwin
module
The Darwin module has specified [no_undeclared_includes] for at least five
years now, there's no need to hard code it in the compiler.
Reviewed By: ributzka, Bigcheese
Differential Revision: https://reviews.llvm.org/D132971
Added:
Modified:
clang/lib/Lex/ModuleMap.cpp
Removed:
diff --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp
index 3698a8c932d72..87a90bc3e704f 100644
--- a/clang/lib/Lex/ModuleMap.cpp
+++ b/clang/lib/Lex/ModuleMap.cpp
@@ -2026,8 +2026,7 @@ void ModuleMapParser::parseModuleDecl() {
ActiveModule->IsSystem = true;
if (Attrs.IsExternC)
ActiveModule->IsExternC = true;
- if (Attrs.NoUndeclaredIncludes ||
- (!ActiveModule->Parent && ModuleName == "Darwin"))
+ if (Attrs.NoUndeclaredIncludes)
ActiveModule->NoUndeclaredIncludes = true;
ActiveModule->Directory = Directory;
___
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] [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] [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] 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] 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] [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)
@@ -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)
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] [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] [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 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)
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)
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 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: @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 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: > 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.*&patternType=keyword&sm=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: > > > 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.*&patternType=keyword&sm=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] [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 @@
*===---===
*/
-#ifndef
[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)
@@ -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)
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)
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)
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] 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] 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] 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)) 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)
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: 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] 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)
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)
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 b/clang/incl
[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 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 Th
[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)
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)
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)
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)
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] [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] [Modules] textual headers in submodules never resolve their `use`s (PR #69651)
https://github.com/ian-twilightcoder created
https://github.com/llvm/llvm-project/pull/69651
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.
>From 1f6f9023fd11c7413f814e249bf77933d289fb73 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 +++--
.../Modules/Inputs/no-undeclared-includes/assert.h | 1 +
.../Modules/Inputs/no-undeclared-includes/base.h| 6 ++
.../Inputs/no-undeclared-includes/module.modulemap | 11 +++
clang/test/Modules/no-undeclared-includes.c | 5 +
5 files changed, 30 insertions(+), 6 deletions(-)
create mode 100644 clang/test/Modules/Inputs/no-undeclared-includes/assert.h
create mode 100644 clang/test/Modules/Inputs/no-undeclared-includes/base.h
create mode 100644
clang/test/Modules/Inputs/no-undeclared-includes/module.modulemap
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 &UDU : 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/Inputs/no-undeclared-includes/assert.h
b/clang/test/Modules/Inputs/no-undeclared-includes/assert.h
new file mode 100644
index 000..6ca1ffb2ad7ea6e
--- /dev/null
+++ b/clang/test/Modules/Inputs/no-undeclared-includes/assert.h
@@ -0,0 +1 @@
+#include
diff --git a/clang/test/Modules/Inputs/no-undeclared-includes/base.h
b/clang/test/Modules/Inputs/no-undeclared-includes/base.h
new file mode 100644
index 000..e559063bef43311
--- /dev/null
+++ b/clang/test/Modules/Inputs/no-undeclared-includes/base.h
@@ -0,0 +1,6 @@
+#ifndef base_h
+#define base_h
+
+
+
+#endif /* base_h */
diff --git a/clang/test/Modules/Inputs/no-undeclared-includes/module.modulemap
b/clang/test/Modules/Inputs/no-undeclared-includes/module.modulemap
new file mode 100644
index 000..23d04305b3becbd
--- /dev/null
+++ b/clang/test/Modules/Inputs/no-undeclared-includes/module.modulemap
@@ -0,0 +1,11 @@
+module cstd [system] [no_undeclared_includes] {
+ use base
+ module assert {
+textual header "assert.h"
+ }
+}
+
+module base [system] {
+ header "base.h"
+ export *
+}
diff --git a/clang/test/Modules/no-undeclared-includes.c
b/clang/test/Modules/no-undeclared-includes.c
new file mode 100644
index 000..613418ef0041328
--- /dev/null
+++ b/clang/test/Modules/no-undeclared-includes.c
@@ -0,0 +1,5 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -I
%S/Inputs/no-undeclared-includes %s -verify
+
+// expected-no-diagnostics
+#include
___
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)
ian-twilightcoder wrote: Somewhat indirectly caused by https://reviews.llvm.org/D132779 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 &UDU : 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
[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
[libunwind] 6408e6c - [libunwind] Add module maps for libunwind
Author: Ian Anderson
Date: 2022-10-26T22:39:46-07:00
New Revision: 6408e6c99d42af3bf866f92931d1f17d97bb7d37
URL:
https://github.com/llvm/llvm-project/commit/6408e6c99d42af3bf866f92931d1f17d97bb7d37
DIFF:
https://github.com/llvm/llvm-project/commit/6408e6c99d42af3bf866f92931d1f17d97bb7d37.diff
LOG: [libunwind] Add module maps for libunwind
Add module maps for the libunwind headers. unwind_arm_ehabi.h and
unwind_itanium.h aren't covered because they don't get installed on all
platforms.
Reviewed By: #libunwind, MaskRay
Differential Revision: https://reviews.llvm.org/D135345
Added:
libunwind/include/libunwind.modulemap
libunwind/include/mach-o/compact_unwind_encoding.modulemap
Modified:
libunwind/include/CMakeLists.txt
Removed:
diff --git a/libunwind/include/CMakeLists.txt
b/libunwind/include/CMakeLists.txt
index adf1766c44cbc..51065d68afd4e 100644
--- a/libunwind/include/CMakeLists.txt
+++ b/libunwind/include/CMakeLists.txt
@@ -1,7 +1,9 @@
set(files
__libunwind_config.h
libunwind.h
+libunwind.modulemap
mach-o/compact_unwind_encoding.h
+mach-o/compact_unwind_encoding.modulemap
unwind_arm_ehabi.h
unwind_itanium.h
unwind.h
diff --git a/libunwind/include/libunwind.modulemap
b/libunwind/include/libunwind.modulemap
new file mode 100644
index 0..162fe1d279a3c
--- /dev/null
+++ b/libunwind/include/libunwind.modulemap
@@ -0,0 +1,10 @@
+module libunwind [system] {
+ header "libunwind.h"
+ export *
+}
+
+module unwind [system] {
+ header "__libunwind_config.h"
+ header "unwind.h"
+ export *
+}
diff --git a/libunwind/include/mach-o/compact_unwind_encoding.modulemap
b/libunwind/include/mach-o/compact_unwind_encoding.modulemap
new file mode 100644
index 0..6eae657d31b5c
--- /dev/null
+++ b/libunwind/include/mach-o/compact_unwind_encoding.modulemap
@@ -0,0 +1,4 @@
+module MachO.compact_unwind_encoding [system] {
+ header "compact_unwind_encoding.h"
+ export *
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 0ea3d88 - [Modules] Add a flag to control builtin headers being in system modules
Author: Ian Anderson
Date: 2023-09-28T11:30:09-07:00
New Revision: 0ea3d88bdb16aa6e9a0043cc3ed93dcb88a89dea
URL:
https://github.com/llvm/llvm-project/commit/0ea3d88bdb16aa6e9a0043cc3ed93dcb88a89dea
DIFF:
https://github.com/llvm/llvm-project/commit/0ea3d88bdb16aa6e9a0043cc3ed93dcb88a89dea.diff
LOG: [Modules] Add a flag to control builtin headers being in system modules
Including select builtin headers in system modules is a workaround for module
cycles, primarily in Apple's Darwin module that includes all of its C standard
library headers. The workaround is problematic because it doesn't include all
of the builtin headers (inttypes.h is notably absent), and it also doesn't
include C++ headers. The straightforward for for this is to make top level
modules for all of the C standard library headers and unwind.h in C++, clang,
and the OS.
However, doing so in clang before the OS modules are ready re-introduces the
module cycles. Add a -fbuiltin-headers-in-system-modules option to control if
the special builtin headers belong to system modules or builtin modules. Pass
the option by default for Apple.
Reviewed By: ChuanqiXu, Bigcheese, benlangmuir
Differential Revision: https://reviews.llvm.org/D159483
Added:
clang/test/Driver/Inputs/MacOSX99.0.sdk/SDKSettings.json
clang/test/Driver/darwin-builtin-modules.c
Modified:
clang/include/clang/Basic/Features.def
clang/include/clang/Basic/LangOptions.def
clang/include/clang/Driver/Options.td
clang/include/clang/Lex/ModuleMap.h
clang/lib/Driver/ToolChains/Darwin.cpp
clang/lib/Lex/ModuleMap.cpp
clang/test/Modules/Werror-Wsystem-headers.m
clang/test/Modules/context-hash.c
clang/test/Modules/crash-vfs-include-pch.m
clang/test/Modules/cstd.m
clang/test/Modules/pch-used.m
clang/test/Modules/shadowed-submodule.m
clang/test/Modules/stddef.c
clang/test/Modules/stddef.m
Removed:
diff --git a/clang/include/clang/Basic/Features.def
b/clang/include/clang/Basic/Features.def
index cbeb92fbe4fdd19..cf626d0120cc7c7 100644
--- a/clang/include/clang/Basic/Features.def
+++ b/clang/include/clang/Basic/Features.def
@@ -278,6 +278,7 @@ EXTENSION(matrix_types, LangOpts.MatrixTypes)
EXTENSION(matrix_types_scalar_division, true)
EXTENSION(cxx_attributes_on_using_declarations, LangOpts.CPlusPlus11)
+FEATURE(builtin_headers_in_system_modules,
LangOpts.BuiltinHeadersInSystemModules)
FEATURE(cxx_abi_relative_vtable, LangOpts.CPlusPlus &&
LangOpts.RelativeCXXABIVTables)
// CUDA/HIP Features
diff --git a/clang/include/clang/Basic/LangOptions.def
b/clang/include/clang/Basic/LangOptions.def
index e18b5b80a34e718..28c9bcec3ee60f1 100644
--- a/clang/include/clang/Basic/LangOptions.def
+++ b/clang/include/clang/Basic/LangOptions.def
@@ -173,6 +173,7 @@ LANGOPT(MathErrno , 1, 1, "errno in math functions")
BENIGN_LANGOPT(HeinousExtensions , 1, 0, "extensions that we really don't like
and may be ripped out at any time")
LANGOPT(Modules , 1, 0, "modules semantics")
COMPATIBLE_LANGOPT(CPlusPlusModules, 1, 0, "C++ modules syntax")
+LANGOPT(BuiltinHeadersInSystemModules, 1, 0, "builtin headers belong to system
modules, and _Builtin_ modules are ignored for cstdlib headers")
BENIGN_ENUM_LANGOPT(CompilingModule, CompilingModuleKind, 3, CMK_None,
"compiling a module interface")
BENIGN_LANGOPT(CompilingPCH, 1, 0, "building a pch")
diff --git a/clang/include/clang/Driver/Options.td
b/clang/include/clang/Driver/Options.td
index f8143651cd3c151..31ecf98b806b843 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2952,6 +2952,13 @@ defm modules : BoolFOption<"modules",
"Enable the 'modules' language feature">,
NegFlag, BothFlags<
[NoXarchOption], [ClangOption, CLOption]>>;
+def fbuiltin_headers_in_system_modules : Flag <["-"],
"fbuiltin-headers-in-system-modules">,
+ Group,
+ Flags<[NoXarchOption]>,
+ Visibility<[CC1Option]>,
+ ShouldParseIf,
+ HelpText<"builtin headers belong to system modules, and _Builtin_ modules
are ignored for cstdlib headers">,
+ MarshallingInfoFlag>;
def fmodule_maps : Flag <["-"], "fmodule-maps">,
Visibility<[ClangOption, CLOption]>, Alias;
def fmodule_name_EQ : Joined<["-"], "fmodule-name=">, Group,
diff --git a/clang/include/clang/Lex/ModuleMap.h
b/clang/include/clang/Lex/ModuleMap.h
index 086b37a78c2927b..494baf0e57b3b9b 100644
--- a/clang/include/clang/Lex/ModuleMap.h
+++ b/clang/include/clang/Lex/ModuleMap.h
@@ -416,7 +416,6 @@ class ModuleMap {
}
/// Is this a compiler builtin header?
- static bool isBuiltinHeader(StringRef FileName);
bool isBuiltinHeader(const FileEntry *File);
/// Add a module map callback.
diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp
b/clang/lib/Driver/ToolChains/Darwin.cpp
index 0068ca33a9fcb7e..c8700a7d159aa47 100644
---
[clang] 9a7a6dd - [Modules] Make clang modules for the C standard library headers
Author: Ian Anderson
Date: 2023-10-03T12:41:11-07:00
New Revision: 9a7a6dd3c358ca7becef75c0a9581dcfa3e6b5f4
URL:
https://github.com/llvm/llvm-project/commit/9a7a6dd3c358ca7becef75c0a9581dcfa3e6b5f4
DIFF:
https://github.com/llvm/llvm-project/commit/9a7a6dd3c358ca7becef75c0a9581dcfa3e6b5f4.diff
LOG: [Modules] Make clang modules for the C standard library headers
Make top level modules for all the C standard library headers.
The `__stddef` implementation headers need header guards now that they're all
modular. stdarg.h and stddef.h will be textual headers in the builtin modules,
and so need to be repeatedly included in both the system and builtin module
case. Define their header guards for consistency, but ignore them when building
with modules.
`__stddef_null.h` needs to ignore its header guard when modules aren't being
used to fulfill its redefinition obligation.
`__stddef_nullptr_t.h` needs to add a guard for C23 so that `_Builtin_stddef`
can compile in C17 and earlier modes. `_Builtin_stddef.nullptr_t` can't require
C23 because it also needs to be usable from C++.
Reviewed By: Bigcheese
Differential Revision: https://reviews.llvm.org/D159064
Added:
clang/test/Modules/Inputs/System/usr/include/complex.h
clang/test/Modules/Inputs/System/usr/include/inttypes.h
clang/test/Modules/Inputs/System/usr/include/math.h
Modified:
clang/lib/Basic/Module.cpp
clang/lib/Headers/__stddef_max_align_t.h
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/Headers/__stddef_wint_t.h
clang/lib/Headers/module.modulemap
clang/lib/Headers/stdarg.h
clang/lib/Headers/stddef.h
clang/test/Headers/stdarg.c
clang/test/Headers/stdargneeds.c
clang/test/Headers/stddef.c
clang/test/Headers/stddefneeds.c
clang/test/Modules/Inputs/System/usr/include/module.map
clang/test/Modules/Inputs/System/usr/include/stdint.h
clang/test/Modules/compiler_builtins.m
clang/test/Modules/stddef.c
clang/test/Modules/stddef.m
Removed:
diff --git a/clang/lib/Basic/Module.cpp b/clang/lib/Basic/Module.cpp
index 0455304ef7f2b1a..0fd9c1dca39984d 100644
--- a/clang/lib/Basic/Module.cpp
+++ b/clang/lib/Basic/Module.cpp
@@ -299,8 +299,9 @@ bool Module::directlyUses(const Module *Requested) {
if (Requested->isSubModuleOf(Use))
return true;
- // Anyone is allowed to use our builtin stddef.h and its accompanying module.
- if (!Requested->Parent && Requested->Name == "_Builtin_stddef_max_align_t")
+ // Anyone is allowed to use our builtin stdarg.h and stddef.h and their
+ // accompanying modules.
+ if (!Requested->Parent && (Requested->Name == "_Builtin_stdarg" ||
Requested->Name == "_Builtin_stddef"))
return true;
if (NoUndeclaredIncludes)
diff --git a/clang/lib/Headers/__stddef_max_align_t.h
b/clang/lib/Headers/__stddef_max_align_t.h
index e3b439285d0fd08..512606a877288bf 100644
--- a/clang/lib/Headers/__stddef_max_align_t.h
+++ b/clang/lib/Headers/__stddef_max_align_t.h
@@ -1,4 +1,4 @@
-/*=== __stddef_max_align_t.h - Definition of max_align_t for modules ---===
+/*=== __stddef_max_align_t.h - Definition of max_align_t ---===
*
* Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
* See https://llvm.org/LICENSE.txt for license information.
diff --git a/clang/lib/Headers/__stddef_null.h
b/clang/lib/Headers/__stddef_null.h
index 77d7a54025380b6..7336fdab389723d 100644
--- a/clang/lib/Headers/__stddef_null.h
+++ b/clang/lib/Headers/__stddef_null.h
@@ -7,7 +7,15 @@
*===---===
*/
+#if !defined(NULL) || !__has_feature(modules)
+
+/* 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.
+ * Modules don't support redefining macros like that, but support that pattern
+ * in the non-modules case.
+ */
#undef NULL
+
#ifdef __cplusplus
#if !defined(__MINGW32__) && !defined(_MSC_VER)
#define NULL __null
@@ -17,3 +25,5 @@
#else
#define NULL ((void*)0)
#endif
+
+#endif
diff --git a/clang/lib/Headers/__stddef_nullptr_t.h
b/clang/lib/Headers/__stddef_nullptr_t.h
index 8d23ed6dc8c697b..183d394d56c1b70 100644
--- a/clang/lib/Headers/__stddef_nullptr_t.h
+++ b/clang/lib/Headers/__stddef_nullptr_t.h
@@ -7,11 +7,9 @@
*===---===
*/
-#if !defined(_NULLPTR_T) || __has_feature(modules)
-/* Always define nullptr_t when modules are available. */
-#if !__has_feature(modules)
+#ifndef _NULL
[clang] Fix the Modules/compiler_builtins.m test (PR #68163)
https://github.com/ian-twilightcoder created https://github.com/llvm/llvm-project/pull/68163 Sometimes unwind.h needs uint32_t also. >From cd2cc782937f9b0415733c48dcd1d9d8ab19ea48 Mon Sep 17 00:00:00 2001 From: Ian Anderson Date: Tue, 3 Oct 2023 14:51:58 -0700 Subject: [PATCH] Fix the Modules/compiler_builtins.m test Sometimes unwind.h needs uint32_t also. --- clang/test/Modules/Inputs/System/usr/include/stdint.h | 1 + 1 file changed, 1 insertion(+) diff --git a/clang/test/Modules/Inputs/System/usr/include/stdint.h b/clang/test/Modules/Inputs/System/usr/include/stdint.h index e3592fe359a4a32..209d54cd411ad55 100644 --- a/clang/test/Modules/Inputs/System/usr/include/stdint.h +++ b/clang/test/Modules/Inputs/System/usr/include/stdint.h @@ -30,6 +30,7 @@ typedef unsigned int uintmax_t; // additional types for unwind.h +typedef unsigned int uint32_t; typedef unsigned long long uint64_t; #endif /* STDINT_H */ ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix the Modules/compiler_builtins.m test (PR #68163)
ian-twilightcoder wrote: Fix for https://lab.llvm.org/buildbot/#/builders/186/builds/12412 https://github.com/llvm/llvm-project/pull/68163 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix the Modules/compiler_builtins.m test (PR #68163)
https://github.com/ian-twilightcoder closed https://github.com/llvm/llvm-project/pull/68163 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Modules] no_undeclared_includes modules (Apple Darwin) don't work the clang modules (PR #68241)
https://github.com/ian-twilightcoder created
https://github.com/llvm/llvm-project/pull/68241
All of the _Builtin_stdarg and _Builtin_stddef submodules need to be allowed
from [no_undeclared_includes] modules. Split the builtin headers tests out from
the compiler_builtins test so that the testing modules can be modified without
affecting the other many tests that use Inputs/System/usr/include.
>From 389a174b0e8462ed15d9ca1e1b223618aec99d21 Mon Sep 17 00:00:00 2001
From: Ian Anderson
Date: Wed, 4 Oct 2023 10:52:33 -0700
Subject: [PATCH] [Modules] no_undeclared_includes modules (Apple Darwin) don't
work the clang modules
All of the _Builtin_stdarg and _Builtin_stddef submodules need to be allowed
from [no_undeclared_includes] modules.
Split the builtin headers tests out from the compiler_builtins test so that the
testing modules can be modified without affecting the other many tests that use
Inputs/System/usr/include.
---
clang/lib/Basic/Module.cpp| 2 +-
.../Inputs/System/usr/include/inttypes.h | 0
.../Modules/Inputs/System/usr/include/math.h | 0
.../Inputs/System/usr/include/module.map | 15
.../Inputs/System/usr/include/stdint.h| 35 -
.../builtin-headers/builtin-modules.modulemap | 29
.../builtin-headers/c++/module.modulemap | 4 ++
.../Inputs/builtin-headers/c++/stdint.h | 6 ++
.../Modules/Inputs/builtin-headers/complex.h | 1 +
.../complex.h => builtin-headers/float.h} | 0
.../Modules/Inputs/builtin-headers/inttypes.h | 12
.../Modules/Inputs/builtin-headers/limits.h | 1 +
.../Modules/Inputs/builtin-headers/math.h | 1 +
.../Modules/Inputs/builtin-headers/stdint.h | 34 +
.../builtin-headers/system-modules.modulemap | 71 +++
clang/test/Modules/builtin-headers.mm | 41 +++
clang/test/Modules/compiler_builtins.m| 22 +-
17 files changed, 204 insertions(+), 70 deletions(-)
delete mode 100644 clang/test/Modules/Inputs/System/usr/include/inttypes.h
delete mode 100644 clang/test/Modules/Inputs/System/usr/include/math.h
create mode 100644
clang/test/Modules/Inputs/builtin-headers/builtin-modules.modulemap
create mode 100644
clang/test/Modules/Inputs/builtin-headers/c++/module.modulemap
create mode 100644 clang/test/Modules/Inputs/builtin-headers/c++/stdint.h
create mode 100644 clang/test/Modules/Inputs/builtin-headers/complex.h
rename clang/test/Modules/Inputs/{System/usr/include/complex.h =>
builtin-headers/float.h} (100%)
create mode 100644 clang/test/Modules/Inputs/builtin-headers/inttypes.h
create mode 100644 clang/test/Modules/Inputs/builtin-headers/limits.h
create mode 100644 clang/test/Modules/Inputs/builtin-headers/math.h
create mode 100644 clang/test/Modules/Inputs/builtin-headers/stdint.h
create mode 100644
clang/test/Modules/Inputs/builtin-headers/system-modules.modulemap
create mode 100644 clang/test/Modules/builtin-headers.mm
diff --git a/clang/lib/Basic/Module.cpp b/clang/lib/Basic/Module.cpp
index 0fd9c1dca39984d..ad353e6089a8920 100644
--- a/clang/lib/Basic/Module.cpp
+++ b/clang/lib/Basic/Module.cpp
@@ -301,7 +301,7 @@ bool Module::directlyUses(const Module *Requested) {
// Anyone is allowed to use our builtin stdarg.h and stddef.h and their
// accompanying modules.
- if (!Requested->Parent && (Requested->Name == "_Builtin_stdarg" ||
Requested->Name == "_Builtin_stddef"))
+ if (Requested->getTopLevelModuleName() == "_Builtin_stdarg" ||
Requested->getTopLevelModuleName() == "_Builtin_stddef")
return true;
if (NoUndeclaredIncludes)
diff --git a/clang/test/Modules/Inputs/System/usr/include/inttypes.h
b/clang/test/Modules/Inputs/System/usr/include/inttypes.h
deleted file mode 100644
index e69de29bb2d1d64..000
diff --git a/clang/test/Modules/Inputs/System/usr/include/math.h
b/clang/test/Modules/Inputs/System/usr/include/math.h
deleted file mode 100644
index e69de29bb2d1d64..000
diff --git a/clang/test/Modules/Inputs/System/usr/include/module.map
b/clang/test/Modules/Inputs/System/usr/include/module.map
index 5a15c70a394d2e2..1d88ca380f49a50 100644
--- a/clang/test/Modules/Inputs/System/usr/include/module.map
+++ b/clang/test/Modules/Inputs/System/usr/include/module.map
@@ -1,24 +1,9 @@
module cstd [system] {
- // Only in system headers directory
- module complex {
-header "complex.h"
- }
-
// Only in compiler support directory
module float_constants {
header "float.h"
}
- // In both directories (compiler support version wins, forwards)
- module inttypes {
-header "inttypes.h"
- }
-
- // Only in system headers directory
- module math {
-header "math.h"
- }
-
// Only in system headers directory
module stdio {
header "stdio.h"
diff --git a/clang/test/Modules/Inputs/System/usr/include/stdint.h
b/clang/test/Modules/Inputs/System/usr/include/stdint.h
index 209d54cd411ad55..e8e50f90290ced7 100644
--- a/clang/test/Modules
[clang] [Modules] no_undeclared_includes modules (Apple Darwin) don't work the clang modules (PR #68241)
https://github.com/ian-twilightcoder updated
https://github.com/llvm/llvm-project/pull/68241
>From 2c6f1a7b618f00f6fae2fddaab94a60a01f3ce90 Mon Sep 17 00:00:00 2001
From: Ian Anderson
Date: Wed, 4 Oct 2023 10:52:33 -0700
Subject: [PATCH] [Modules] no_undeclared_includes modules (Apple Darwin) don't
work the clang modules
All of the _Builtin_stdarg and _Builtin_stddef submodules need to be allowed
from [no_undeclared_includes] modules.
Split the builtin headers tests out from the compiler_builtins test so that the
testing modules can be modified without affecting the other many tests that use
Inputs/System/usr/include.
---
clang/lib/Basic/Module.cpp| 3 +-
.../Inputs/System/usr/include/inttypes.h | 0
.../Modules/Inputs/System/usr/include/math.h | 0
.../Inputs/System/usr/include/module.map | 15
.../Inputs/System/usr/include/stdint.h| 35 -
.../builtin-headers/builtin-modules.modulemap | 29
.../builtin-headers/c++/module.modulemap | 4 ++
.../Inputs/builtin-headers/c++/stdint.h | 6 ++
.../Modules/Inputs/builtin-headers/complex.h | 1 +
.../complex.h => builtin-headers/float.h} | 0
.../Modules/Inputs/builtin-headers/inttypes.h | 12
.../Modules/Inputs/builtin-headers/limits.h | 1 +
.../Modules/Inputs/builtin-headers/math.h | 1 +
.../Modules/Inputs/builtin-headers/stdint.h | 34 +
.../builtin-headers/system-modules.modulemap | 71 +++
clang/test/Modules/builtin-headers.mm | 41 +++
clang/test/Modules/compiler_builtins.m| 22 +-
17 files changed, 205 insertions(+), 70 deletions(-)
delete mode 100644 clang/test/Modules/Inputs/System/usr/include/inttypes.h
delete mode 100644 clang/test/Modules/Inputs/System/usr/include/math.h
create mode 100644
clang/test/Modules/Inputs/builtin-headers/builtin-modules.modulemap
create mode 100644
clang/test/Modules/Inputs/builtin-headers/c++/module.modulemap
create mode 100644 clang/test/Modules/Inputs/builtin-headers/c++/stdint.h
create mode 100644 clang/test/Modules/Inputs/builtin-headers/complex.h
rename clang/test/Modules/Inputs/{System/usr/include/complex.h =>
builtin-headers/float.h} (100%)
create mode 100644 clang/test/Modules/Inputs/builtin-headers/inttypes.h
create mode 100644 clang/test/Modules/Inputs/builtin-headers/limits.h
create mode 100644 clang/test/Modules/Inputs/builtin-headers/math.h
create mode 100644 clang/test/Modules/Inputs/builtin-headers/stdint.h
create mode 100644
clang/test/Modules/Inputs/builtin-headers/system-modules.modulemap
create mode 100644 clang/test/Modules/builtin-headers.mm
diff --git a/clang/lib/Basic/Module.cpp b/clang/lib/Basic/Module.cpp
index 0fd9c1dca39984d..7879a11179b6d4b 100644
--- a/clang/lib/Basic/Module.cpp
+++ b/clang/lib/Basic/Module.cpp
@@ -301,7 +301,8 @@ bool Module::directlyUses(const Module *Requested) {
// Anyone is allowed to use our builtin stdarg.h and stddef.h and their
// accompanying modules.
- if (!Requested->Parent && (Requested->Name == "_Builtin_stdarg" ||
Requested->Name == "_Builtin_stddef"))
+ if (Requested->getTopLevelModuleName() == "_Builtin_stdarg" ||
+ Requested->getTopLevelModuleName() == "_Builtin_stddef")
return true;
if (NoUndeclaredIncludes)
diff --git a/clang/test/Modules/Inputs/System/usr/include/inttypes.h
b/clang/test/Modules/Inputs/System/usr/include/inttypes.h
deleted file mode 100644
index e69de29bb2d1d64..000
diff --git a/clang/test/Modules/Inputs/System/usr/include/math.h
b/clang/test/Modules/Inputs/System/usr/include/math.h
deleted file mode 100644
index e69de29bb2d1d64..000
diff --git a/clang/test/Modules/Inputs/System/usr/include/module.map
b/clang/test/Modules/Inputs/System/usr/include/module.map
index 5a15c70a394d2e2..1d88ca380f49a50 100644
--- a/clang/test/Modules/Inputs/System/usr/include/module.map
+++ b/clang/test/Modules/Inputs/System/usr/include/module.map
@@ -1,24 +1,9 @@
module cstd [system] {
- // Only in system headers directory
- module complex {
-header "complex.h"
- }
-
// Only in compiler support directory
module float_constants {
header "float.h"
}
- // In both directories (compiler support version wins, forwards)
- module inttypes {
-header "inttypes.h"
- }
-
- // Only in system headers directory
- module math {
-header "math.h"
- }
-
// Only in system headers directory
module stdio {
header "stdio.h"
diff --git a/clang/test/Modules/Inputs/System/usr/include/stdint.h
b/clang/test/Modules/Inputs/System/usr/include/stdint.h
index 209d54cd411ad55..e8e50f90290ced7 100644
--- a/clang/test/Modules/Inputs/System/usr/include/stdint.h
+++ b/clang/test/Modules/Inputs/System/usr/include/stdint.h
@@ -1,36 +1 @@
-#ifndef STDINT_H
-#define STDINT_H
-
typedef int my_awesome_nonstandard_integer_type;
-
-// types needed by stdatomic.h
-
-typedef char int_least8_t;
-typedef short int_least16_t;
-typedef
[clang] [Modules] no_undeclared_includes modules (Apple Darwin) don't work the clang modules (PR #68241)
ian-twilightcoder wrote: I could also revert most of the test changes and augment no-undeclared-includes-builtins.cpp, but I liked having a dedicated test that's not necessarily specific to Apple or glibc to test the intended system module setups with `-fbuiltin-headers-in-system-modules` set and unset. Let me know if you feel like the tests here are too redundant. https://github.com/llvm/llvm-project/pull/68241 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Modules] no_undeclared_includes modules (Apple Darwin) don't work the clang modules (PR #68241)
https://github.com/ian-twilightcoder closed https://github.com/llvm/llvm-project/pull/68241 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 62f769a - [Headers] Add missing __need_ macros to stdarg.h
Author: Ian Anderson
Date: 2023-08-23T23:55:30-07:00
New Revision: 62f769aa82f668a076fed2633e425631a4598a47
URL:
https://github.com/llvm/llvm-project/commit/62f769aa82f668a076fed2633e425631a4598a47
DIFF:
https://github.com/llvm/llvm-project/commit/62f769aa82f668a076fed2633e425631a4598a47.diff
LOG: [Headers] Add missing __need_ macros to stdarg.h
Apple needs a `__need_` macro for va_list. Add one, and also ones for
va_start/va_arg/va_end, __va_copy, va_copy.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D157793
Added:
clang/test/Headers/stdarg.c
clang/test/Headers/stdargneeds.c
Modified:
clang/lib/Headers/stdarg.h
Removed:
diff --git a/clang/lib/Headers/stdarg.h b/clang/lib/Headers/stdarg.h
index 13742eea61f631..521c4733d55589 100644
--- a/clang/lib/Headers/stdarg.h
+++ b/clang/lib/Headers/stdarg.h
@@ -7,22 +7,45 @@
*===---===
*/
-#ifndef __STDARG_H
+#if !defined(__STDARG_H) || defined(__need___va_list) ||
\
+defined(__need_va_list) || defined(__need_va_arg) ||
\
+defined(__need___va_copy) || defined(__need_va_copy)
+#if !defined(__need___va_list) && !defined(__need_va_list) &&
\
+!defined(__need_va_arg) && !defined(__need___va_copy) &&
\
+!defined(__need_va_copy)
+#define __STDARG_H
+#define __need___va_list
+#define __need_va_list
+#define __need_va_arg
+#define __need___va_copy
+/* GCC always defines __va_copy, but does not define va_copy unless in c99 mode
+ * or -ansi is not specified, since it was not part of C90.
+ */
+#if (defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L) ||
\
+(defined(__cplusplus) && __cplusplus >= 201103L) ||
\
+!defined(__STRICT_ANSI__)
+#define __need_va_copy
+#endif
+#endif
+
+#ifdef __need___va_list
#ifndef __GNUC_VA_LIST
#define __GNUC_VA_LIST
typedef __builtin_va_list __gnuc_va_list;
#endif
-
-#ifdef __need___va_list
#undef __need___va_list
-#else
-#define __STDARG_H
+#endif /* defined(__need___va_list) */
+
+#ifdef __need_va_list
#ifndef _VA_LIST
typedef __builtin_va_list va_list;
#define _VA_LIST
#endif
+#undef __need_va_list
+#endif /* defined(__need_va_list) */
+#ifdef __need_va_arg
#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 202311L
/* C23 does not require the second parameter for va_start. */
#define va_start(ap, ...) __builtin_va_start(ap, 0)
@@ -32,18 +55,17 @@ typedef __builtin_va_list va_list;
#endif
#define va_end(ap) __builtin_va_end(ap)
#define va_arg(ap, type)__builtin_va_arg(ap, type)
+#undef __need_va_arg
+#endif /* defined(__need_va_arg) */
-/* GCC always defines __va_copy, but does not define va_copy unless in c99 mode
- * or -ansi is not specified, since it was not part of C90.
- */
+#ifdef __need___va_copy
#define __va_copy(d,s) __builtin_va_copy(d,s)
+#undef __need___va_copy
+#endif /* defined(__need___va_copy) */
-#if (defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L) ||
\
-(defined(__cplusplus) && __cplusplus >= 201103L) ||
\
-!defined(__STRICT_ANSI__)
+#ifdef __need_va_copy
#define va_copy(dest, src) __builtin_va_copy(dest, src)
-#endif
-
-#endif /* __STDARG_H */
+#undef __need_va_copy
+#endif /* defined(__need_va_copy) */
-#endif /* not __STDARG_H */
+#endif
diff --git a/clang/test/Headers/stdarg.c b/clang/test/Headers/stdarg.c
new file mode 100644
index 00..49df42caa3300a
--- /dev/null
+++ b/clang/test/Headers/stdarg.c
@@ -0,0 +1,37 @@
+// RUN: split-file %s %t
+// RUN: %clang_cc1 -fsyntax-only -verify=c89
-Werror=implicit-function-declaration -std=c89 %t/stdarg0.c
+// RUN: %clang_cc1 -fsyntax-only -verify=c99
-Werror=implicit-function-declaration -std=c99 %t/stdarg0.c
+// RUN: %clang_cc1 -fsyntax-only -verify=c89
-Werror=implicit-function-declaration -std=c89 %t/stdarg1.c
+// RUN: %clang_cc1 -fsyntax-only -verify=c99
-Werror=implicit-function-declaration -std=c99 %t/stdarg1.c
+
+// Split the file so that the "implicitly declaring library function" errors
get repeated.
+
+//--- stdarg0.c
+static void f(int p, ...) {
+__gnuc_va_list g; // c89-error{{undeclared identifier '__gnuc_va_list'}}
c99-error{{undeclared identifier}}
+va_list v; // c89-error{{undeclared identifier 'va_list'}}
c99-error{{undeclared identifier}}
+va_start(v, p); // c89-error{{implicitly declaring library function
'va_start'}} c89-note{{include the header or explicitly provide a
declaration for 'va_start'}} c89-error{{undeclared identifier 'v'}} \
+ c99-error{{call to undeclared library function
'va_start'}} c99-note{{provide a declaration}} c99-error{{undeclared
identifier}}
+int i = va_arg(v, int); // c89-error{{implicit declaration of function
'
[clang] 72da678 - [Headers] Replace __need_STDDEF_H_misc with specific __need_ macros
Author: Ian Anderson
Date: 2023-08-23T23:55:51-07:00
New Revision: 72da678d8c84c2cc442940a026ca50d0bb5c2b8e
URL:
https://github.com/llvm/llvm-project/commit/72da678d8c84c2cc442940a026ca50d0bb5c2b8e
DIFF:
https://github.com/llvm/llvm-project/commit/72da678d8c84c2cc442940a026ca50d0bb5c2b8e.diff
LOG: [Headers] Replace __need_STDDEF_H_misc with specific __need_ macros
Apple needs __need_ macros for rsize_t and offsetof. Add those, and also ones
for nullptr_t, unreachable, max_align_t.
Reviewed By: aaron.ballman, efriedma, #libc, ldionne
Differential Revision: https://reviews.llvm.org/D157757
Added:
clang/test/Headers/stddef.c
clang/test/Headers/stddefneeds.c
Modified:
clang/lib/Headers/stddef.h
Removed:
diff --git a/clang/lib/Headers/stddef.h b/clang/lib/Headers/stddef.h
index dedf4e59a977eb..dc720d44fc9b73 100644
--- a/clang/lib/Headers/stddef.h
+++ b/clang/lib/Headers/stddef.h
@@ -8,22 +8,46 @@
*/
#if !defined(__STDDEF_H) || defined(__need_ptr
diff _t) || \
-defined(__need_size_t) || defined(__need_wchar_t) ||
\
-defined(__need_NULL) || defined(__need_wint_t)
+defined(__need_size_t) || defined(__need_rsize_t) ||
\
+defined(__need_wchar_t) || defined(__need_NULL) ||
\
+defined(__need_nullptr_t) || defined(__need_unreachable) ||
\
+defined(__need_max_align_t) || defined(__need_offsetof) ||
\
+defined(__need_wint_t) ||
\
+(defined(__STDC_WANT_LIB_EXT1__) && __STDC_WANT_LIB_EXT1__ >= 1)
#if !defined(__need_ptr
diff _t) && !defined(__need_size_t) && \
-!defined(__need_wchar_t) && !defined(__need_NULL) &&
\
-!defined(__need_wint_t)
+!defined(__need_rsize_t) && !defined(__need_wchar_t) &&
\
+!defined(__need_NULL) && !defined(__need_nullptr_t) &&
\
+!defined(__need_unreachable) && !defined(__need_max_align_t) &&
\
+!defined(__need_offsetof) && !defined(__need_wint_t)
/* Always define miscellaneous pieces when modules are available. */
#if !__has_feature(modules)
#define __STDDEF_H
#endif
#define __need_ptr
diff _t
#define __need_size_t
+/* ISO9899:2011 7.20 (C11 Annex K): Define rsize_t if __STDC_WANT_LIB_EXT1__ is
+ * enabled. */
+#if defined(__STDC_WANT_LIB_EXT1__) && __STDC_WANT_LIB_EXT1__ >= 1
+#define __need_rsize_t
+#endif
#define __need_wchar_t
#define __need_NULL
-#define __need_STDDEF_H_misc
-/* __need_wint_t is intentionally not defined here. */
+#if (defined(__STDC_VERSION__) && __STDC_VERSION__ >= 202311L) ||
\
+defined(__cplusplus)
+#define __need_nullptr_t
+#endif
+#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 202311L
+#define __need_unreachable
+#endif
+#if (defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L) ||
\
+(defined(__cplusplus) && __cplusplus >= 201103L)
+#define __need_max_align_t
+#endif
+#define __need_offsetof
+/* wint_t is provided by and not . It's here
+ * for compatibility, but must be explicitly requested. Therefore
+ * __need_wint_t is intentionally not defined here. */
#endif
#if defined(__need_ptr
diff _t)
@@ -48,18 +72,16 @@ typedef __SIZE_TYPE__ size_t;
#undef __need_size_t
#endif /*defined(__need_size_t) */
-#if defined(__need_STDDEF_H_misc)
-/* ISO9899:2011 7.20 (C11 Annex K): Define rsize_t if __STDC_WANT_LIB_EXT1__ is
- * enabled. */
-#if (defined(__STDC_WANT_LIB_EXT1__) && __STDC_WANT_LIB_EXT1__ >= 1 && \
- !defined(_RSIZE_T)) || __has_feature(modules)
+#if defined(__need_rsize_t)
+#if !defined(_RSIZE_T) || __has_feature(modules)
/* Always define rsize_t when modules are available. */
#if !__has_feature(modules)
#define _RSIZE_T
#endif
typedef __SIZE_TYPE__ rsize_t;
#endif
-#endif /* defined(__need_STDDEF_H_misc) */
+#undef __need_rsize_t
+#endif /* defined(__need_rsize_t) */
#if defined(__need_wchar_t)
#if !defined(__cplusplus) || (defined(_MSC_VER) && !_NATIVE_WCHAR_T_DEFINED)
@@ -88,32 +110,37 @@ typedef __WCHAR_TYPE__ wchar_t;
#else
# define NULL ((void*)0)
#endif
+#undef __need_NULL
+#endif /* defined(__need_NULL) */
+
+#if defined(__need_nullptr_t)
#ifdef __cplusplus
#if defined(_MSC_EXTENSIONS) && defined(_NATIVE_NULLPTR_SUPPORTED)
-namespace std { typedef decltype(nullptr) nullptr_t; }
+namespace std {
+typedef decltype(nullptr) nullptr_t;
+}
using ::std::nullptr_t;
#endif
-#endif
-#undef __need_NULL
-#endif /* defined(__need_NULL) */
-
-#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 202311L
+#else
typedef typeof(nullptr) nullptr_t;
-#endif /* defined(__STDC_VERSION__) && __STDC_VERSION__ >= 202311L */
+#endif
+#undef __need_nullptr_t
+#endif /* defined(__need_nullptr_t) */
-#if defined(__need_STDDEF_H_misc) && defined(__STDC_VER
[clang] adb68c9 - [clang][modules] Add a c23 module feature
Author: Ian Anderson
Date: 2023-08-29T11:11:14-07:00
New Revision: adb68c979d846a55bef2ea8208a69565e8c1a833
URL:
https://github.com/llvm/llvm-project/commit/adb68c979d846a55bef2ea8208a69565e8c1a833
DIFF:
https://github.com/llvm/llvm-project/commit/adb68c979d846a55bef2ea8208a69565e8c1a833.diff
LOG: [clang][modules] Add a c23 module feature
Add a c23 module feature for `requires`.
Reviewed By: ChuanqiXu, v.g.vassilev, aaron.ballman
Differential Revision: https://reviews.llvm.org/D159018
Added:
Modified:
clang/docs/Modules.rst
clang/docs/ReleaseNotes.rst
clang/lib/Basic/Module.cpp
clang/test/Modules/Inputs/DependsOnModule.framework/module.map
clang/test/Modules/requires.m
Removed:
diff --git a/clang/docs/Modules.rst b/clang/docs/Modules.rst
index a54850f5457bd4..06294e3c58a4f8 100644
--- a/clang/docs/Modules.rst
+++ b/clang/docs/Modules.rst
@@ -588,6 +588,9 @@ c11
c17
C17 support is available.
+c23
+ C23 support is available.
+
freestanding
A freestanding environment is available.
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 9cd95005d7e059..6f97acc09bc934 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -107,6 +107,7 @@ C23 Feature Support
and the ``__STDC_VERSION__`` macro now expands to ``202311L`` instead of its
previous placeholder value. Clang continues to accept ``-std=c2x`` and
``-std=gnu2x`` as aliases for C23 and GNU C23, respectively.
+- Clang now supports `requires c23` for module maps.
Non-comprehensive list of changes in this release
-
diff --git a/clang/lib/Basic/Module.cpp b/clang/lib/Basic/Module.cpp
index 8ec68237a0fc08..4ffdb6a5f717cf 100644
--- a/clang/lib/Basic/Module.cpp
+++ b/clang/lib/Basic/Module.cpp
@@ -113,6 +113,7 @@ static bool hasFeature(StringRef Feature, const LangOptions
&LangOpts,
.Case("c99", LangOpts.C99)
.Case("c11", LangOpts.C11)
.Case("c17", LangOpts.C17)
+.Case("c23", LangOpts.C23)
.Case("freestanding", LangOpts.Freestanding)
.Case("gnuinlineasm", LangOpts.GNUAsm)
.Case("objc", LangOpts.ObjC)
diff --git a/clang/test/Modules/Inputs/DependsOnModule.framework/module.map
b/clang/test/Modules/Inputs/DependsOnModule.framework/module.map
index 9cede40c45c336..c4813b679f0efd 100644
--- a/clang/test/Modules/Inputs/DependsOnModule.framework/module.map
+++ b/clang/test/Modules/Inputs/DependsOnModule.framework/module.map
@@ -64,4 +64,7 @@ framework module DependsOnModule {
explicit module C17 {
requires c17
}
+ explicit module C23 {
+requires c23
+ }
}
diff --git a/clang/test/Modules/requires.m b/clang/test/Modules/requires.m
index b370fa31ee0d95..77f00a0e47c182 100644
--- a/clang/test/Modules/requires.m
+++ b/clang/test/Modules/requires.m
@@ -35,4 +35,6 @@
@import DependsOnModule.C11; // expected-note {{module imported here}}
// expected-error@DependsOnModule.framework/module.map:64 {{module
'DependsOnModule.C17' requires feature 'c17'}}
@import DependsOnModule.C17; // expected-note {{module imported here}}
+// expected-error@DependsOnModule.framework/module.map:67 {{module
'DependsOnModule.C23' requires feature 'c23'}}
+@import DependsOnModule.C23; // expected-note {{module imported here}}
#endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 680da4b - [Headers][Modules] Make separate headers for the stdarg.h and stddef.h pieces so that they can be modularized
Author: Ian Anderson
Date: 2023-08-30T11:41:12-07:00
New Revision: 680da4b5d7ec05a1e6656f2a7603a7b843268bab
URL:
https://github.com/llvm/llvm-project/commit/680da4b5d7ec05a1e6656f2a7603a7b843268bab
DIFF:
https://github.com/llvm/llvm-project/commit/680da4b5d7ec05a1e6656f2a7603a7b843268bab.diff
LOG: [Headers][Modules] Make separate headers for the stdarg.h and stddef.h
pieces so that they can be modularized
stdarg.h and stddef.h have to be textual headers in their upcoming modules to
support their `__needs_xxx` macros. That means that they won't get precompiled
into their modules' pcm, and instead their declarations will go into every
other pcm that uses them. For now that's ok since the type merger can handle
the declarations in these headers, but it's suboptimal at best. Make separate
headers for all of the pieces so that they can be properly modularized.
Reviewed By: aaron.ballman, ChuanqiXu
Differential Revision: https://reviews.llvm.org/D158709
Added:
clang/lib/Headers/__stdarg___gnuc_va_list.h
clang/lib/Headers/__stdarg___va_copy.h
clang/lib/Headers/__stdarg_va_arg.h
clang/lib/Headers/__stdarg_va_copy.h
clang/lib/Headers/__stdarg_va_list.h
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/Headers/__stddef_wint_t.h
Modified:
clang-tools-extra/clang-include-fixer/find-all-symbols/STLPostfixHeaderMap.cpp
clang-tools-extra/clangd/index/CanonicalIncludes.cpp
clang/lib/Headers/CMakeLists.txt
clang/lib/Headers/stdarg.h
clang/lib/Headers/stddef.h
clang/test/Headers/stddef.c
clang/test/Headers/stddefneeds.c
clang/test/Modules/stddef.c
compiler-rt/lib/gwp_asan/guarded_pool_allocator.h
llvm/utils/gn/secondary/clang/lib/Headers/BUILD.gn
Removed:
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 5245243bac0816..df77bf7ea46da3 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
@@ -13,7 +13,21 @@ namespace find_all_symbols {
const HeaderMapCollector::RegexHeaderMap *getSTLPostfixHeaderMap() {
static const HeaderMapCollector::RegexHeaderMap STLPostfixHeaderMap = {
+ {"include/__stdarg___gnuc_va_list.h$", ""},
+ {"include/__stdarg___va_copy.h$", ""},
+ {"include/__stdarg_va_arg.h$", ""},
+ {"include/__stdarg_va_copy.h$", ""},
+ {"include/__stdarg_va_list.h$", ""},
{"include/__stddef_max_align_t.h$", ""},
+ {"include/__stddef_null.h$", ""},
+ {"include/__stddef_nullptr_t.h$", ""},
+ {"include/__stddef_offsetof.h$", ""},
+ {"include/__stddef_ptr
diff _t.h$", ""},
+ {"include/__stddef_rsize_t.h$", ""},
+ {"include/__stddef_size_t.h$", ""},
+ {"include/__stddef_unreachable.h$", ""},
+ {"include/__stddef_wchar_t.h$", ""},
+ {"include/__stddef_wint_t.h$", ""},
{"include/__wmmintrin_aes.h$", ""},
{"include/__wmmintrin_pclmul.h$", ""},
{"include/adxintrin.h$", ""},
diff --git a/clang-tools-extra/clangd/index/CanonicalIncludes.cpp
b/clang-tools-extra/clangd/index/CanonicalIncludes.cpp
index 3e3ba4d9f2b58d..9d6c09cd2ab4b7 100644
--- a/clang-tools-extra/clangd/index/CanonicalIncludes.cpp
+++ b/clang-tools-extra/clangd/index/CanonicalIncludes.cpp
@@ -16,7 +16,21 @@ namespace clang {
namespace clangd {
namespace {
const std::pair IncludeMappings[] = {
+{"include/__stdarg___gnuc_va_list.h", ""},
+{"include/__stdarg___va_copy.h", ""},
+{"include/__stdarg_va_arg.h", ""},
+{"include/__stdarg_va_copy.h", ""},
+{"include/__stdarg_va_list.h", ""},
{"include/__stddef_max_align_t.h", ""},
+{"include/__stddef_null.h", ""},
+{"include/__stddef_nullptr_t.h", ""},
+{"include/__stddef_offsetof.h", ""},
+{"include/__stddef_ptr
diff _t.h", ""},
+{"include/__stddef_rsize_t.h", ""},
+{"include/__stddef_size_t.h", ""},
+{"include/__stddef_unreachable.h", ""},
+{"include/__stddef_wchar_t.h", ""},
+{"include/__stddef_wint_t.h", ""},
{"include/__wmmintrin_aes.h", ""},
{"include/__wmmintrin_pclmul.h", ""},
{"include/adxintrin.h", ""},
diff --git a/clang/lib/Headers/CMakeLists.txt
b/clang/lib/Headers/CMakeLists.txt
index 05bb740f306727..e6154d7ecd80ea 100644
--- a/clang/lib/Headers/CMakeLists.txt
+++ b/clang/lib/Headers/CMakeLists.txt
@@ -10,10 +10,24 @@ set(core_files
module.modulemap
stdalign.h
stdarg.h
+ __stdarg___gnuc_va_list.h
+ __stdarg___
[libunwind] 1187d8a - [libunwind][Modules] Add unwind_arm_ehabi.h and unwind_itanium.h to the unwind module)
Author: Ian Anderson
Date: 2023-03-20T15:13:14-07:00
New Revision: 1187d8a62ba288e2221731f1795fa184571cd854
URL:
https://github.com/llvm/llvm-project/commit/1187d8a62ba288e2221731f1795fa184571cd854
DIFF:
https://github.com/llvm/llvm-project/commit/1187d8a62ba288e2221731f1795fa184571cd854.diff
LOG: [libunwind][Modules] Add unwind_arm_ehabi.h and unwind_itanium.h to the
unwind module)
Add unwind_arm_ehabi.h and unwind_itanium.h to the unwind module and use angle
includes to include them.
Reviewed By: ldionne, #libunwind
Differential Revision: https://reviews.llvm.org/D144323
Added:
Modified:
libunwind/include/libunwind.modulemap
libunwind/include/unwind.h
Removed:
diff --git a/libunwind/include/libunwind.modulemap
b/libunwind/include/libunwind.modulemap
index 162fe1d279a3..775841ecb5f1 100644
--- a/libunwind/include/libunwind.modulemap
+++ b/libunwind/include/libunwind.modulemap
@@ -6,5 +6,8 @@ module libunwind [system] {
module unwind [system] {
header "__libunwind_config.h"
header "unwind.h"
+ private textual header "unwind_arm_ehabi.h"
+ private textual header "unwind_itanium.h"
+
export *
}
diff --git a/libunwind/include/unwind.h b/libunwind/include/unwind.h
index 26cdef22207e..b1775d3a3dec 100644
--- a/libunwind/include/unwind.h
+++ b/libunwind/include/unwind.h
@@ -56,9 +56,9 @@ typedef enum {
typedef struct _Unwind_Context _Unwind_Context; // opaque
#if defined(_LIBUNWIND_ARM_EHABI)
-#include "unwind_arm_ehabi.h"
+#include
#else
-#include "unwind_itanium.h"
+#include
#endif
typedef _Unwind_Reason_Code (*_Unwind_Stop_Fn)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Adjust modulemap to mark mm3dnow as textual header. (PR #107155)
ian-twilightcoder wrote: Does anything rely on the header guard? It would be good if we could remove it since textual headers are not generally expected to declare anything, even macros. https://github.com/llvm/llvm-project/pull/107155 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Adjust modulemap to mark mm3dnow as textual header. (PR #107155)
ian-twilightcoder wrote: That's true. Probably nobody's using this header anymore anyway so either way should be fine I think. https://github.com/llvm/llvm-project/pull/107155 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Enable built-in modules for the upcoming Apple releases (PR #102239)
ian-twilightcoder wrote: /cherry-pick 961639962251de7428c3fe93fa17cfa6ab3c561a 0f1361baf650641a59aaa1710d7a0b7b02f2e56d https://github.com/llvm/llvm-project/pull/102239 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix a unit test input file (PR #102567)
https://github.com/ian-twilightcoder created
https://github.com/llvm/llvm-project/pull/102567
I forgot to update the version info in the SDKSettings file when I updated it
to the real version relevant to the test. The version in the SDKSettings file
isn't actually used for anything in the test, but might as well have realistic
values.
>From 23f2ad9507fb6a8edb0f2194448e0eb2ac8c2652 Mon Sep 17 00:00:00 2001
From: Ian Anderson
Date: Thu, 8 Aug 2024 21:54:40 -0700
Subject: [PATCH] Fix a unit test input file
I forgot to update the version info in the SDKSettings file when I updated it
to the real version relevant to the test. The version in the SDKSettings file
isn't actually used for anything in the test, but might as well have realistic
values.
---
clang/test/Driver/Inputs/MacOSX15.0.sdk/SDKSettings.json | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/test/Driver/Inputs/MacOSX15.0.sdk/SDKSettings.json
b/clang/test/Driver/Inputs/MacOSX15.0.sdk/SDKSettings.json
index 77b70e1a83c19c..ced45d5c219962 100644
--- a/clang/test/Driver/Inputs/MacOSX15.0.sdk/SDKSettings.json
+++ b/clang/test/Driver/Inputs/MacOSX15.0.sdk/SDKSettings.json
@@ -1 +1 @@
-{"Version":"990.0", "MaximumDeploymentTarget": "99.0.99"}
+{"Version":"15.0", "MaximumDeploymentTarget": "15.0.99"}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix a unit test input file (PR #102567)
https://github.com/ian-twilightcoder updated
https://github.com/llvm/llvm-project/pull/102567
>From 4bc7053e1383e6aa9e1a60017be296e305b8da5a Mon Sep 17 00:00:00 2001
From: Ian Anderson
Date: Thu, 8 Aug 2024 21:54:40 -0700
Subject: [PATCH] Fix a unit test input file
I forgot to update the version info in the SDKSettings file when I updated it
to the real version relevant to the test.
---
clang/test/Driver/Inputs/MacOSX15.0.sdk/SDKSettings.json | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/test/Driver/Inputs/MacOSX15.0.sdk/SDKSettings.json
b/clang/test/Driver/Inputs/MacOSX15.0.sdk/SDKSettings.json
index 77b70e1a83c19c..ced45d5c219962 100644
--- a/clang/test/Driver/Inputs/MacOSX15.0.sdk/SDKSettings.json
+++ b/clang/test/Driver/Inputs/MacOSX15.0.sdk/SDKSettings.json
@@ -1 +1 @@
-{"Version":"990.0", "MaximumDeploymentTarget": "99.0.99"}
+{"Version":"15.0", "MaximumDeploymentTarget": "15.0.99"}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix a unit test input file (PR #102567)
https://github.com/ian-twilightcoder edited https://github.com/llvm/llvm-project/pull/102567 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix a unit test input file (PR #102567)
https://github.com/ian-twilightcoder closed https://github.com/llvm/llvm-project/pull/102567 ___ 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: > > 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)
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] [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 header
[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)
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)
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: > 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] 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: 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 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] 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] [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] [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] [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: > 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] [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] 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 011ff5a95a4a5de1dc6ae2d271ae42f28075b2b0 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 +++--
clang/unittests/Lex/HeaderSearchTest.cpp | 67
3 files changed, 81 insertions(+), 4 deletions(-)
diff --git a/clang/include/clang/Lex/HeaderSearch.h
b/clang/include/clang/Lex/HeaderSearch.h
index 5ac634d4e..d8ca1c528de36 100644
--- a/clang/include/clang/Lex/HeaderSearch.h
+++ b/clang/include/clang/Lex/HeaderSearch.h
@@ -90,7 +90,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 574723b33866a..18c0bab4a81e4 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 &HFI,
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;
}
diff --git a/clang/unittests/Lex/HeaderSearchTest.cpp
b/clang/unittests/Lex/HeaderSearchTest.cpp
index c578fa72c859e..f1735c2d46946 100644
--- a/clang/unittests/Lex/HeaderSearchTest.cpp
+++ b/clang/unittests/Lex/HeaderSearchTest.cpp
@@ -308,5 +308,72 @@ TEST_F(HeaderSearchTest, HeaderMapFrameworkLookup) {
EXPECT_EQ(Search.getIncludeNameForHeader(FE), "Foo/Foo.h");
}
+TEST_F(HeaderSearchTest, HeaderFileInfoMerge) {
+ auto AddHeader = [&](std::string HeaderPath) -> FileEntryRef {
+VFS->addFile(HeaderPath, 0,
+ llvm::MemoryBuffer::getMemBufferCopy("", HeaderPath),
+ /*User=*/std::nullopt, /*Group=*/std::nullopt,
+ llvm::sys::fs::file_type::regular_file);
+return *Search.LookupFile(
+HeaderPath, SourceLocation(), /*isAngled=*/false, /*FromDir=*/nullptr,
+/*CurDir=*/nullptr, /*Includers=*/{}, /*SearchPath=*/nullptr,
+/*RelativePath=*/nullptr, /*RequestingModule=*/nullptr,
+/*SuggestedModule=*/nullptr, /*IsMapped=*/nullptr,
+/*IsFrameworkFound=*/nullptr);
+ };
+
+ class MockExternalHeaderFileInfoSource : public ExternalHeaderFileInfoSource
{
+HeaderFileInfo GetHeaderFileInfo(FileEntryRef FE) {
+ HeaderFileInfo HFI;
+ auto FileName = FE.getName();
+ if (FileName == ModularPath)
+HFI.mergeModuleMembership(ModuleMap::NormalHeader);
+ else if (FileName == TextualPath)
+HFI.mergeModuleMembership(ModuleMap::TextualHeader);
+ HFI.External = true;
+ HFI.IsValid = true;
+ return HFI;
+}
+
+ public:
+std::string ModularPath = "/modular.h";
+std::string TextualPath = "/textual.h";
+ };
+
+ auto ExternalSource = new MockExternalHeaderFileInfoSource();
+ Search.SetExternalSource(ExternalSource);
+
+ // Everything should start out external.
+ auto ModularFE = AddHeader(ExternalSource->ModularPath);
+ auto TextualFE = AddHeader(ExternalSource->TextualPath);
+ EXPECT_TRUE(Search.getExistingFileInfo(ModularFE)->External);
+ EXPECT_TRUE(Search.getExistingFileInfo(TextualFE)->Ext
[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. We tried this, but it seems that by the time the modules compile, everything ends up being local instead of external anyway. I think a unit test is probably good enough for this one since we aren't even sure what the practical 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 updated
https://github.com/llvm/llvm-project/pull/89005
>From f6bcc20d07248069dee1ff19c1aa334152b311a8 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 +++--
clang/unittests/Lex/HeaderSearchTest.cpp | 67
3 files changed, 81 insertions(+), 4 deletions(-)
diff --git a/clang/include/clang/Lex/HeaderSearch.h
b/clang/include/clang/Lex/HeaderSearch.h
index 5ac634d4e..d8ca1c528de36 100644
--- a/clang/include/clang/Lex/HeaderSearch.h
+++ b/clang/include/clang/Lex/HeaderSearch.h
@@ -90,7 +90,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 574723b33866a..18c0bab4a81e4 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 &HFI,
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;
}
diff --git a/clang/unittests/Lex/HeaderSearchTest.cpp
b/clang/unittests/Lex/HeaderSearchTest.cpp
index c578fa72c859e..b55d52df14d5d 100644
--- a/clang/unittests/Lex/HeaderSearchTest.cpp
+++ b/clang/unittests/Lex/HeaderSearchTest.cpp
@@ -308,5 +308,72 @@ TEST_F(HeaderSearchTest, HeaderMapFrameworkLookup) {
EXPECT_EQ(Search.getIncludeNameForHeader(FE), "Foo/Foo.h");
}
+TEST_F(HeaderSearchTest, HeaderFileInfoMerge) {
+ auto AddHeader = [&](std::string HeaderPath) -> FileEntryRef {
+VFS->addFile(HeaderPath, 0,
+ llvm::MemoryBuffer::getMemBufferCopy("", HeaderPath),
+ /*User=*/std::nullopt, /*Group=*/std::nullopt,
+ llvm::sys::fs::file_type::regular_file);
+return *Search.LookupFile(
+HeaderPath, SourceLocation(), /*isAngled=*/false, /*FromDir=*/nullptr,
+/*CurDir=*/nullptr, /*Includers=*/{}, /*SearchPath=*/nullptr,
+/*RelativePath=*/nullptr, /*RequestingModule=*/nullptr,
+/*SuggestedModule=*/nullptr, /*IsMapped=*/nullptr,
+/*IsFrameworkFound=*/nullptr);
+ };
+
+ class MockExternalHeaderFileInfoSource : public ExternalHeaderFileInfoSource
{
+HeaderFileInfo GetHeaderFileInfo(FileEntryRef FE) {
+ HeaderFileInfo HFI;
+ auto FileName = FE.getName();
+ if (FileName == ModularPath)
+HFI.mergeModuleMembership(ModuleMap::NormalHeader);
+ else if (FileName == TextualPath)
+HFI.mergeModuleMembership(ModuleMap::TextualHeader);
+ HFI.External = true;
+ HFI.IsValid = true;
+ return HFI;
+}
+
+ public:
+std::string ModularPath = "/modular.h";
+std::string TextualPath = "/textual.h";
+ };
+
+ auto ExternalSource = new MockExternalHeaderFileInfoSource();
+ Search.SetExternalSource(ExternalSource);
+
+ // Everything should start out external.
+ auto ModularFE = AddHeader(ExternalSource->ModularPath);
+ auto TextualFE = AddHeader(ExternalSource->TextualPath);
+ EXPECT_TRUE(Search.getExistingFileInfo(ModularFE)->External);
+ EXPECT_TRUE(Search.getExistingFileInfo(TextualFE)->Ext
[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 a416e28efabdf812756f452b823c4b1e7388c865 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 -
clang/unittests/Lex/HeaderSearchTest.cpp | 73
3 files changed, 87 insertions(+), 4 deletions(-)
diff --git a/clang/include/clang/Lex/HeaderSearch.h
b/clang/include/clang/Lex/HeaderSearch.h
index 5ac634d4e..d8ca1c528de36 100644
--- a/clang/include/clang/Lex/HeaderSearch.h
+++ b/clang/include/clang/Lex/HeaderSearch.h
@@ -90,7 +90,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 574723b33866a..18c0bab4a81e4 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 &HFI,
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;
}
diff --git a/clang/unittests/Lex/HeaderSearchTest.cpp
b/clang/unittests/Lex/HeaderSearchTest.cpp
index c578fa72c859e..8bea41c642673 100644
--- a/clang/unittests/Lex/HeaderSearchTest.cpp
+++ b/clang/unittests/Lex/HeaderSearchTest.cpp
@@ -308,5 +308,78 @@ TEST_F(HeaderSearchTest, HeaderMapFrameworkLookup) {
EXPECT_EQ(Search.getIncludeNameForHeader(FE), "Foo/Foo.h");
}
+TEST_F(HeaderSearchTest, HeaderFileInfoMerge) {
+ auto AddHeader = [&](std::string HeaderPath) -> FileEntryRef {
+VFS->addFile(HeaderPath, 0,
+ llvm::MemoryBuffer::getMemBufferCopy("", HeaderPath),
+ /*User=*/std::nullopt, /*Group=*/std::nullopt,
+ llvm::sys::fs::file_type::regular_file);
+return *Search.LookupFile(
+HeaderPath, SourceLocation(), /*isAngled=*/false, /*FromDir=*/nullptr,
+/*CurDir=*/nullptr, /*Includers=*/{}, /*SearchPath=*/nullptr,
+/*RelativePath=*/nullptr, /*RequestingModule=*/nullptr,
+/*SuggestedModule=*/nullptr, /*IsMapped=*/nullptr,
+/*IsFrameworkFound=*/nullptr);
+ };
+
+ class MockExternalHeaderFileInfoSource : public ExternalHeaderFileInfoSource
{
+HeaderFileInfo GetHeaderFileInfo(FileEntryRef FE) {
+ HeaderFileInfo HFI;
+ auto FileName = FE.getName();
+ if (FileName == ModularPath)
+HFI.mergeModuleMembership(ModuleMap::NormalHeader);
+ else if (FileName == TextualPath)
+HFI.mergeModuleMembership(ModuleMap::TextualHeader);
+ HFI.External = true;
+ HFI.IsValid = true;
+ return HFI;
+}
+
+ public:
+std::string ModularPath = "/modular.h";
+std::string TextualPath = "/textual.h";
+ };
+
+ auto ExternalSource = new MockExternalHeaderFileInfoSource();
+ Search.SetExternalSource(ExternalSource);
+
+ // Everything should start out external.
+ auto ModularFE = AddHeader(ExternalSource->ModularPath);
+ auto TextualFE = AddHeader(ExternalSource->TextualPath);
+ EXPECT_TRUE(Search.getExistingFileInfo(ModularFE)->External);
+ EXPECT_TRUE(Search.getExistingFileInfo(TextualFE)->Ext
[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 bdb35190d9873e4413863be1bba842cb202842fc 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 -
clang/unittests/Lex/HeaderSearchTest.cpp | 77
3 files changed, 91 insertions(+), 4 deletions(-)
diff --git a/clang/include/clang/Lex/HeaderSearch.h
b/clang/include/clang/Lex/HeaderSearch.h
index 5ac634d4e..d8ca1c528de36 100644
--- a/clang/include/clang/Lex/HeaderSearch.h
+++ b/clang/include/clang/Lex/HeaderSearch.h
@@ -90,7 +90,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 574723b33866a..18c0bab4a81e4 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 &HFI,
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;
}
diff --git a/clang/unittests/Lex/HeaderSearchTest.cpp
b/clang/unittests/Lex/HeaderSearchTest.cpp
index c578fa72c859e..ac1359746668a 100644
--- a/clang/unittests/Lex/HeaderSearchTest.cpp
+++ b/clang/unittests/Lex/HeaderSearchTest.cpp
@@ -308,5 +308,82 @@ TEST_F(HeaderSearchTest, HeaderMapFrameworkLookup) {
EXPECT_EQ(Search.getIncludeNameForHeader(FE), "Foo/Foo.h");
}
+TEST_F(HeaderSearchTest, HeaderFileInfoMerge) {
+ auto AddHeader = [&](std::string HeaderPath) -> FileEntryRef {
+VFS->addFile(HeaderPath, 0,
+ llvm::MemoryBuffer::getMemBufferCopy("", HeaderPath),
+ /*User=*/std::nullopt, /*Group=*/std::nullopt,
+ llvm::sys::fs::file_type::regular_file);
+return *Search.LookupFile(
+HeaderPath, SourceLocation(), /*isAngled=*/false, /*FromDir=*/nullptr,
+/*CurDir=*/nullptr, /*Includers=*/{}, /*SearchPath=*/nullptr,
+/*RelativePath=*/nullptr, /*RequestingModule=*/nullptr,
+/*SuggestedModule=*/nullptr, /*IsMapped=*/nullptr,
+/*IsFrameworkFound=*/nullptr);
+ };
+
+ class MockExternalHeaderFileInfoSource : public ExternalHeaderFileInfoSource
{
+HeaderFileInfo GetHeaderFileInfo(FileEntryRef FE) {
+ HeaderFileInfo HFI;
+ auto FileName = FE.getName();
+ if (FileName == ModularPath)
+HFI.mergeModuleMembership(ModuleMap::NormalHeader);
+ else if (FileName == TextualPath)
+HFI.mergeModuleMembership(ModuleMap::TextualHeader);
+ HFI.External = true;
+ HFI.IsValid = true;
+ return HFI;
+}
+
+ public:
+std::string ModularPath = is_style_windows(llvm::sys::path::Style::native)
+ ? "C:/modular.h"
+ : "/modular.h";
+std::string TextualPath = is_style_windows(llvm::sys::path::Style::native)
+ ? "C:/textual.h"
+ : "/textual.h";
+ };
+
+ auto ExternalSource = new MockExternalHeaderFileInfoSource();
+ Search.SetExternalSource(ExternalSource);
+
+ // Ever
[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 eb296a4e28e54971f6e90e2fe4543ead4c7c2dbb 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 +++--
clang/unittests/Lex/HeaderSearchTest.cpp | 68
3 files changed, 82 insertions(+), 4 deletions(-)
diff --git a/clang/include/clang/Lex/HeaderSearch.h
b/clang/include/clang/Lex/HeaderSearch.h
index 5ac634d4e..d8ca1c528de36 100644
--- a/clang/include/clang/Lex/HeaderSearch.h
+++ b/clang/include/clang/Lex/HeaderSearch.h
@@ -90,7 +90,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 574723b33866a..18c0bab4a81e4 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 &HFI,
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;
}
diff --git a/clang/unittests/Lex/HeaderSearchTest.cpp
b/clang/unittests/Lex/HeaderSearchTest.cpp
index c578fa72c859e..23d0cf7789392 100644
--- a/clang/unittests/Lex/HeaderSearchTest.cpp
+++ b/clang/unittests/Lex/HeaderSearchTest.cpp
@@ -308,5 +308,73 @@ TEST_F(HeaderSearchTest, HeaderMapFrameworkLookup) {
EXPECT_EQ(Search.getIncludeNameForHeader(FE), "Foo/Foo.h");
}
+TEST_F(HeaderSearchTest, HeaderFileInfoMerge) {
+ auto AddHeader = [&](std::string HeaderPath) -> FileEntryRef {
+VFS->addFile(HeaderPath, 0,
+ llvm::MemoryBuffer::getMemBufferCopy("", HeaderPath),
+ /*User=*/std::nullopt, /*Group=*/std::nullopt,
+ llvm::sys::fs::file_type::regular_file);
+return *FileMgr.getOptionalFileRef(HeaderPath);
+ };
+
+ class MockExternalHeaderFileInfoSource : public ExternalHeaderFileInfoSource
{
+HeaderFileInfo GetHeaderFileInfo(FileEntryRef FE) {
+ HeaderFileInfo HFI;
+ auto FileName = FE.getName();
+ if (FileName == ModularPath)
+HFI.mergeModuleMembership(ModuleMap::NormalHeader);
+ else if (FileName == TextualPath)
+HFI.mergeModuleMembership(ModuleMap::TextualHeader);
+ HFI.External = true;
+ HFI.IsValid = true;
+ return HFI;
+}
+
+ public:
+std::string ModularPath = "/modular.h";
+std::string TextualPath = "/textual.h";
+ };
+
+ auto ExternalSource = new MockExternalHeaderFileInfoSource();
+ Search.SetExternalSource(ExternalSource);
+
+ // Everything should start out external.
+ auto ModularFE = AddHeader(ExternalSource->ModularPath);
+ auto TextualFE = AddHeader(ExternalSource->TextualPath);
+ EXPECT_TRUE(Search.getExistingFileInfo(ModularFE)->External);
+ EXPECT_TRUE(Search.getExistingFileInfo(TextualFE)->External);
+
+ // Marking the same role should keep it external
+ Search.MarkFileModuleHeader(ModularFE, ModuleMap::NormalHeader,
+ /*isCompilingModuleHeader=*/false);
+ Search.MarkFileModuleHeader(TextualFE, ModuleMap::TextualHeader,
+
[clang] [clang][modules] HeaderSearch::MarkFileModuleHeader sets textual headers' HeaderFileInfo non-external when it shouldn't (PR #89005)
@@ -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;
ian-twilightcoder wrote:
HFI doesn't have == declared, and I'm not sure it's worth adding one for 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 sets textual headers' HeaderFileInfo non-external when it shouldn't (PR #89005)
@@ -308,5 +308,73 @@ TEST_F(HeaderSearchTest, HeaderMapFrameworkLookup) {
EXPECT_EQ(Search.getIncludeNameForHeader(FE), "Foo/Foo.h");
}
+TEST_F(HeaderSearchTest, HeaderFileInfoMerge) {
+ auto AddHeader = [&](std::string HeaderPath) -> FileEntryRef {
+VFS->addFile(HeaderPath, 0,
+ llvm::MemoryBuffer::getMemBufferCopy("", HeaderPath),
+ /*User=*/std::nullopt, /*Group=*/std::nullopt,
+ llvm::sys::fs::file_type::regular_file);
+return *FileMgr.getOptionalFileRef(HeaderPath);
+ };
+
+ class MockExternalHeaderFileInfoSource : public ExternalHeaderFileInfoSource
{
+HeaderFileInfo GetHeaderFileInfo(FileEntryRef FE) {
+ HeaderFileInfo HFI;
+ auto FileName = FE.getName();
+ if (FileName == ModularPath)
+HFI.mergeModuleMembership(ModuleMap::NormalHeader);
+ else if (FileName == TextualPath)
+HFI.mergeModuleMembership(ModuleMap::TextualHeader);
+ HFI.External = true;
+ HFI.IsValid = true;
+ return HFI;
+}
+
+ public:
+std::string ModularPath = "/modular.h";
+std::string TextualPath = "/textual.h";
+ };
+
+ auto ExternalSource = new MockExternalHeaderFileInfoSource();
+ Search.SetExternalSource(ExternalSource);
+
+ // Everything should start out external.
+ auto ModularFE = AddHeader(ExternalSource->ModularPath);
+ auto TextualFE = AddHeader(ExternalSource->TextualPath);
+ EXPECT_TRUE(Search.getExistingFileInfo(ModularFE)->External);
+ EXPECT_TRUE(Search.getExistingFileInfo(TextualFE)->External);
+
+ // Marking the same role should keep it external
+ Search.MarkFileModuleHeader(ModularFE, ModuleMap::NormalHeader,
+ /*isCompilingModuleHeader=*/false);
+ Search.MarkFileModuleHeader(TextualFE, ModuleMap::TextualHeader,
+ /*isCompilingModuleHeader=*/false);
+ EXPECT_TRUE(Search.getExistingFileInfo(ModularFE)->External);
+ EXPECT_TRUE(Search.getExistingFileInfo(TextualFE)->External);
+
+ // textual -> modular should update the HFI, but modular -> textual should be
+ // a no-op.
+ Search.MarkFileModuleHeader(ModularFE, ModuleMap::TextualHeader,
+ /*isCompilingModuleHeader=*/false);
+ Search.MarkFileModuleHeader(TextualFE, ModuleMap::NormalHeader,
+ /*isCompilingModuleHeader=*/false);
+ auto ModularFI = Search.getExistingFileInfo(ModularFE);
+ auto TextualFI = Search.getExistingFileInfo(TextualFE);
+ EXPECT_TRUE(ModularFI->External);
+ EXPECT_TRUE(ModularFI->isModuleHeader);
+ EXPECT_FALSE(ModularFI->isTextualModuleHeader);
+ EXPECT_FALSE(TextualFI->External);
+ EXPECT_TRUE(ModularFI->isModuleHeader);
+ EXPECT_FALSE(ModularFI->isTextualModuleHeader);
ian-twilightcoder wrote:
Whoops yes
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 85df42bd55f76f8c77f44c78c8ccbe1bed8f8682 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 | 13 +++--
clang/unittests/Lex/HeaderSearchTest.cpp | 68
3 files changed, 81 insertions(+), 4 deletions(-)
diff --git a/clang/include/clang/Lex/HeaderSearch.h
b/clang/include/clang/Lex/HeaderSearch.h
index 5ac634d4e..d8ca1c528de36 100644
--- a/clang/include/clang/Lex/HeaderSearch.h
+++ b/clang/include/clang/Lex/HeaderSearch.h
@@ -90,7 +90,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 d6da6c2fe6c0e..9321a36b79a7f 100644
--- a/clang/lib/Lex/HeaderSearch.cpp
+++ b/clang/lib/Lex/HeaderSearch.cpp
@@ -1313,11 +1313,18 @@ OptionalFileEntryRef
HeaderSearch::LookupSubframeworkHeader(
// File Info Management.
//===--===//
+static bool moduleMembershipNeedsMerge(const HeaderFileInfo *HFI,
+ ModuleMap::ModuleHeaderRole Role) {
+ if (ModuleMap::isModular(Role))
+return !HFI->isModuleHeader || HFI->isTextualModuleHeader;
+ if (!HFI->isModuleHeader && (Role & ModuleMap::TextualHeader))
+return !HFI->isTextualModuleHeader;
+ return false;
+}
+
static void mergeHeaderFileInfoModuleBits(HeaderFileInfo &HFI,
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 +1439,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;
}
diff --git a/clang/unittests/Lex/HeaderSearchTest.cpp
b/clang/unittests/Lex/HeaderSearchTest.cpp
index a5f193ef37ce8..38ce3812c204f 100644
--- a/clang/unittests/Lex/HeaderSearchTest.cpp
+++ b/clang/unittests/Lex/HeaderSearchTest.cpp
@@ -323,5 +323,73 @@ TEST_F(HeaderSearchTest, HeaderMapFrameworkLookup) {
EXPECT_EQ(Search.getIncludeNameForHeader(FE), "Foo/Foo.h");
}
+TEST_F(HeaderSearchTest, HeaderFileInfoMerge) {
+ auto AddHeader = [&](std::string HeaderPath) -> FileEntryRef {
+VFS->addFile(HeaderPath, 0,
+ llvm::MemoryBuffer::getMemBufferCopy("", HeaderPath),
+ /*User=*/std::nullopt, /*Group=*/std::nullopt,
+ llvm::sys::fs::file_type::regular_file);
+return *FileMgr.getOptionalFileRef(HeaderPath);
+ };
+
+ class MockExternalHeaderFileInfoSource : public ExternalHeaderFileInfoSource
{
+HeaderFileInfo GetHeaderFileInfo(FileEntryRef FE) {
+ HeaderFileInfo HFI;
+ auto FileName = FE.getName();
+ if (FileName == ModularPath)
+HFI.mergeModuleMembership(ModuleMap::NormalHeader);
+ else if (FileName == TextualPath)
+HFI.mergeModuleMembership(ModuleMap::TextualHeader);
+ HFI.External = true;
+ HFI.IsValid = true;
+ return HFI;
+}
+
+ public:
+std::string ModularPath = "/modular.h";
+std::string TextualPath = "/textual.h";
+ };
+
+ auto ExternalSource = new MockExternalHeaderFileInfoSource();
+ Search.SetExternalSource(ExternalSource);
+
+ // Everything should start out external.
+ auto ModularFE = AddHeader(ExternalSource->ModularPath);
+ auto TextualFE = AddHeader(ExternalSource->TextualPath);
+ EXPECT_TRUE(Search.getExistingFileInfo(ModularFE)->External);
+ EXPECT_TRUE(Search.getExistingFileInfo(TextualFE)->External);
+
+ // Marking the same role should keep it external
+ Search.MarkFileModuleHeader(ModularFE, ModuleMap::NormalHeader,
+ /*isCompilingModuleHeader=*/false);
+ Search.MarkFileModuleHeader(TextualFE, ModuleMap::TextualHeader,
+ /*isCompilingMo
[clang] [clang][modules] HeaderSearch::MarkFileModuleHeader sets textual headers' HeaderFileInfo non-external when it shouldn't (PR #89005)
ian-twilightcoder wrote: The only error is in CodeGen/PowerPC/subreg-lanemasks.mir and doesn't look related to this change. 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 closed 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)
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] [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
