[PATCH] D118311: [Clang][ModuleMap] Add conditional parsing via requires block declaration

iana added inline comments. Comment at: clang/include/clang/Basic/Module.h:249 + /// language options has the given feature. + static bool hasFeature(StringRef Feature, const LangOptions , + const TargetInfo ); Is `static` correct?

[PATCH] D118311: [Clang][ModuleMap] Add conditional parsing via requires block declaration

iana added inline comments. Comment at: clang/docs/Modules.rst:651 + +requires cplusplus { + header "vector" Is there any kind of `else` syntax here? Or do you just use `!whatever` for the else? Is something like this valid? ``` requires cplusplus11 {

[PATCH] D118311: [Clang][ModuleMap] Add conditional parsing via requires block declaration

iana added inline comments. Comment at: clang/docs/Modules.rst:651 + +requires cplusplus { + header "vector" Bigcheese wrote: > iana wrote: > > Is there any kind of `else` syntax here? Or do you just use `!whatever` for > > the else? Is something like

[PATCH] D159312: [Headers] Remove a space in NULL define

iana accepted this revision. iana added a comment. This revision is now accepted and ready to land. Can you add a comment that says the spacing is important please? Although why the heck is musl declaring NULL in stdio.h? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D158709: [Headers][Modules] Make separate headers for the stdarg.h and stddef.h pieces so that they can be modularized

iana added a comment. In D158709#4621199 , @aaron.ballman wrote: > At a high-level (not just for this patch, but for the entire series): is this > need specific to your downstream or is this a more general need? e.g., should > this complexity be kept

[PATCH] D159064: [Modules] Make clang modules for the C standard library headers

iana added a comment. Ah, the tests are failing because some of them are putting the builtin headers in other modules. This is going to need the "optionally ignore the builtin modules" change first then. I almost have that one finished. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D158021: [clang][modules] Mark builtin header 'inttypes.h' for modules

iana requested changes to this revision. iana added a comment. This revision now requires changes to proceed. I was talking to Ben about this one a little today since I ended up needing to make the same `-isystem`/`-I` -> `-internal-isystem` switch in some of the unit tests in D159064

[PATCH] D159064: [Modules] Make clang modules for the C standard library headers

iana updated this revision to Diff 556630. iana added a comment. Actually allow the stdarg module Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159064/new/ https://reviews.llvm.org/D159064 Files: clang/lib/Basic/Module.cpp

[PATCH] D159483: [Modules] Add a flag to control builtin headers being in system modules

iana updated this revision to Diff 556627. iana added a comment. __stddef_max_align_t.h always needs a header guard, because the type merger can't handle struct's. If it has a header guard then it always needs to have a module too, even if `-fbuiltin-headers-in-system-modules` is passed.

[PATCH] D159064: [Modules] Make clang modules for the C standard library headers

iana updated this revision to Diff 556629. iana added a comment. Adjust for the changes in D159483 . Mostly the header guards that were added in that review have moved to this one, the stdarg module needs to always be allowed, and a few tests needed to adjust

[PATCH] D159064: [Modules] Make clang modules for the C standard library headers

iana added inline comments. Comment at: clang/test/Modules/Inputs/System/usr/include/stdint.h:2 typedef int my_awesome_nonstandard_integer_type; + +/* C99 7.18.1.1 Exact-width integer types. benlangmuir wrote: > Why do we need all this code now (I assume this

[PATCH] D159064: [Modules] Make clang modules for the C standard library headers

iana updated this revision to Diff 556280. iana added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159064/new/ https://reviews.llvm.org/D159064 Files: clang/lib/Basic/Module.cpp clang/lib/Headers/__stddef_null.h

[PATCH] D159483: [Modules] Add a flag to control builtin headers being in system modules

iana updated this revision to Diff 556279. iana added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159483/new/ https://reviews.llvm.org/D159483 Files: clang/include/clang/Basic/Features.def

[PATCH] D159064: [Modules] Make clang modules for the C standard library headers

iana added inline comments. Comment at: clang/test/Modules/Inputs/System/usr/include/stdint.h:2 typedef int my_awesome_nonstandard_integer_type; + +/* C99 7.18.1.1 Exact-width integer types. iana wrote: > benlangmuir wrote: > > Why do we need all this code now

[PATCH] D159483: [Modules] Add a flag to control builtin headers being in system modules

iana marked an inline comment as done. iana added a comment. In D159483#4641289 , @benlangmuir wrote: >>> How does this work today? Wouldn't the include guard prevent this? >> >> Today they don't define their include guard when building with modules. >

[PATCH] D159483: [Modules] Add a flag to control builtin headers being in system modules

iana updated this revision to Diff 556206. iana added a comment. Fix formatting Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159483/new/ https://reviews.llvm.org/D159483 Files: clang/include/clang/Basic/Features.def

[PATCH] D159483: [Modules] Add a flag to control builtin headers being in system modules

iana updated this revision to Diff 556207. iana added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159483/new/ https://reviews.llvm.org/D159483 Files: clang/include/clang/Basic/Features.def

[PATCH] D159064: [Modules] Make clang modules for the C standard library headers

iana updated this revision to Diff 556208. iana added a comment. Fix formatting issue Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159064/new/ https://reviews.llvm.org/D159064 Files: clang/lib/Basic/Module.cpp

[PATCH] D159483: [Modules] Add a flag to control builtin headers being in system modules

iana marked an inline comment as done. iana added inline comments. Comment at: clang/include/clang/Basic/Features.def:233 FEATURE(modules, LangOpts.Modules) +FEATURE(builtin_headers_in_system_modules, LangOpts.BuiltinHeadersInSystemModules) FEATURE(safe_stack,

[PATCH] D159064: [Modules] Make clang modules for the C standard library headers

iana marked 2 inline comments as done. iana added inline comments. Comment at: clang/lib/Headers/module.modulemap:269 + explicit module wint_t { +header "__stddef_wint_t.h" +export * Arguably this should be textual since stddef.h shouldn't own wint_t

[PATCH] D159483: [Modules] Add a flag to control builtin headers being in system modules

iana added a comment. A big assumption this patch makes is that `ModuleMap::isBuiltinHeader` is primarily to support Apple's unfortunate module needs. Thus this patch turns that behavior off by default, which makes things work the way one would expect. That is, when

[PATCH] D159483: [Modules] Add a flag to control builtin headers being in system modules

iana added inline comments. Comment at: clang/include/clang/Driver/Options.td:2944 [NoXarchOption], [ClangOption, CLOption]>>; +def fbuiltin_headers_in_system_modules : Flag <["-"], "fbuiltin-headers-in-system-modules">, + Group, I don't love this

[PATCH] D159064: [Modules] Make clang modules for the C standard library headers

iana updated this revision to Diff 556192. iana added a comment. Rebase on top of D159483 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159064/new/ https://reviews.llvm.org/D159064 Files:

[PATCH] D159483: [Modules] Add a flag to control builtin headers being in system modules

iana created this revision. iana added reviewers: aaron.ballman, rsmith, efriedma, ldionne, ChuanqiXu, Bigcheese, vsapsai, benlangmuir. Herald added a subscriber: ributzka. Herald added a project: All. iana requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. Herald

[PATCH] D159483: [Modules] Add a flag to control builtin headers being in system modules

iana marked an inline comment as done. iana added inline comments. Comment at: clang/include/clang/Basic/Features.def:233 FEATURE(modules, LangOpts.Modules) +FEATURE(builtin_headers_in_system_modules, LangOpts.BuiltinHeadersInSystemModules) FEATURE(safe_stack,

[PATCH] D159483: [Modules] Add a flag to control builtin headers being in system modules

iana added a comment. In D159483#4641231 , @benlangmuir wrote: >> but need to be repeatedly included when they're used in system modules > > How does this work today? Wouldn't the include guard prevent this? Today they don't define their include guard

[PATCH] D159064: [Modules] Make clang modules for the C standard library headers

iana updated this revision to Diff 556311. iana added a comment. Try deleting the module cache to fix the failing tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159064/new/ https://reviews.llvm.org/D159064 Files: clang/lib/Basic/Module.cpp

[PATCH] D159064: [Modules] Make clang modules for the C standard library headers

iana added inline comments. Comment at: clang/test/Modules/Inputs/System/usr/include/stdint.h:2 typedef int my_awesome_nonstandard_integer_type; + +/* C99 7.18.1.1 Exact-width integer types. dexonsmith wrote: > benlangmuir wrote: > > iana wrote: > > > iana

[PATCH] D159064: [Modules] Make clang modules for the C standard library headers

iana updated this revision to Diff 556313. iana added a comment. Add the header guard comment to __stddef_null.h Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159064/new/ https://reviews.llvm.org/D159064 Files: clang/lib/Basic/Module.cpp

[PATCH] D140250: Define NULL in its own header

iana abandoned this revision. iana added a comment. This was succeeded by D158709 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140250/new/ https://reviews.llvm.org/D140250

[PATCH] D159064: [Modules] Make clang modules for the C standard library headers

iana updated this revision to Diff 556231. iana added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159064/new/ https://reviews.llvm.org/D159064 Files: clang/lib/Basic/Module.cpp clang/lib/Headers/__stddef_null.h

[PATCH] D159483: [Modules] Add a flag to control builtin headers being in system modules

iana updated this revision to Diff 556230. iana marked 3 inline comments as done. iana added a comment. Add comments to stdarg.h and stddef.h explaining their interesting header guard setup. Ignore -fbuiltin-headers-in-system-modules if -fmodules isn't passed. DriverKit doesn't need to pass

[PATCH] D159064: [Modules] Make clang modules for the C standard library headers

iana updated this revision to Diff 556355. iana added a comment. - Drop the -verify off the new tests so that I can get the full errors Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159064/new/ https://reviews.llvm.org/D159064 Files:

[PATCH] D159064: [Modules] Make clang modules for the C standard library headers

iana updated this revision to Diff 556356. iana added a comment. - Drop the -verify off the stddef tests so that I can get the full errors Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159064/new/ https://reviews.llvm.org/D159064 Files:

[PATCH] D159018: [clang][modules] Add a c23 module feature

This revision was automatically updated to reflect the committed changes. Closed by commit rGadb68c979d84: [clang][modules] Add a c23 module feature (authored by iana). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159018/new/

[PATCH] D159064: [Modules] Make clang modules for the C standard library headers

iana updated this revision to Diff 554832. iana marked an inline comment as done. iana added a comment. The nullptr_t submodule can't require c23 because it needs to be used in C++ in some cases too. Remove the `requires` from the module map and add a C23 guard to the header. Repository: rG

[PATCH] D159064: [Modules] Make clang modules for the C standard library headers

iana marked an inline comment as done. iana added inline comments. Comment at: clang/lib/Headers/__stddef_null.h:14 + * __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

[PATCH] D158709: [Headers][Modules] Make separate headers for the stdarg.h and stddef.h pieces so that they can be modularized

This revision was automatically updated to reflect the committed changes. Closed by commit rG680da4b5d7ec: [Headers][Modules] Make separate headers for the stdarg.h and stddef.h pieces… (authored by iana). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D159064: [Modules] Make clang modules for the C standard library headers

iana updated this revision to Diff 554882. iana added a comment. Update Module::directlyUses Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159064/new/ https://reviews.llvm.org/D159064 Files: clang/lib/Basic/Module.cpp

[PATCH] D159064: [Modules] Make clang modules for the C standard library headers

iana added inline comments. Comment at: clang/lib/Headers/module.modulemap:269 + explicit module wint_t { +header "__stddef_wint_t.h" +export * iana wrote: > Arguably this should be textual since stddef.h shouldn't own wint_t and it's > just here for

[PATCH] D159483: [Modules] Add a flag to control builtin headers being in system modules

iana marked 2 inline comments as done. iana added inline comments. Comment at: clang/include/clang/Basic/LangOptions.def:176 COMPATIBLE_LANGOPT(CPlusPlusModules, 1, 0, "C++ modules syntax") +LANGOPT(BuiltinHeadersInSystemModules, 1, 0, "builtin headers belong to system

[PATCH] D159483: [Modules] Add a flag to control builtin headers being in system modules

This revision was automatically updated to reflect the committed changes. Closed by commit rG0ea3d88bdb16: [Modules] Add a flag to control builtin headers being in system modules (authored by iana). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D159064: [Modules] Make clang modules for the C standard library headers

iana marked 5 inline comments as done. iana added inline comments. Comment at: clang/test/Modules/Inputs/System/usr/include/stdint.h:2 typedef int my_awesome_nonstandard_integer_type; + +/* C99 7.18.1.1 Exact-width integer types. dexonsmith wrote: > Bigcheese

[PATCH] D159064: [Modules] Make clang modules for the C standard library headers

iana updated this revision to Diff 557549. iana added a comment. Minimize the testing stdint.h Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159064/new/ https://reviews.llvm.org/D159064 Files: clang/lib/Basic/Module.cpp

[PATCH] D159064: [Modules] Make clang modules for the C standard library headers

This revision was automatically updated to reflect the committed changes. Closed by commit rG9a7a6dd3c358: [Modules] Make clang modules for the C standard library headers (authored by iana). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159064/new/

[PATCH] D159064: [Modules] Make clang modules for the C standard library headers

iana added a comment. In D159064#4647709 , @vsapsai wrote: > Still going through the patch and through the discussion. But wanted to ask > if there are any complications in reverting this change? For libc++ we've > discussed that we don't really know

[PATCH] D159483: [Modules] Add a flag to control builtin headers being in system modules

iana marked an inline comment as done. iana added inline comments. Comment at: clang/include/clang/Basic/LangOptions.def:176 COMPATIBLE_LANGOPT(CPlusPlusModules, 1, 0, "C++ modules syntax") +LANGOPT(BuiltinHeadersInSystemModules, 1, 0, "builtin headers belong to system

[PATCH] D157757: [Headers] Replace __need_STDDEF_H_misc with specific __need_ macros

iana added inline comments. Comment at: clang/lib/Headers/stddef.h:118-122 +#ifdef __cplusplus +namespace std { +typedef decltype(nullptr) nullptr_t; +} +using ::std::nullptr_t; aaron.ballman wrote: > iana wrote: > > ldionne wrote: > > > iana wrote: > > > >

[PATCH] D157757: [Headers] Replace __need_STDDEF_H_misc with specific __need_ macros

iana added inline comments. Comment at: clang/lib/Headers/stddef.h:118-122 +#ifdef __cplusplus +namespace std { +typedef decltype(nullptr) nullptr_t; +} +using ::std::nullptr_t; aaron.ballman wrote: > iana wrote: > > aaron.ballman wrote: > > > iana wrote: > > >

[PATCH] D157757: [Headers] Replace __need_STDDEF_H_misc with specific __need_ macros

iana updated this revision to Diff 552591. iana added a comment. Only define nullptr_t in C++ if _MSC_EXTENSIONS is defined, even if __need_nullptr_t is explicitly set. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157757/new/

[PATCH] D157757: [Headers] Replace __need_STDDEF_H_misc with specific __need_ macros

iana added inline comments. Comment at: clang/lib/Headers/stddef.h:118-122 +#ifdef __cplusplus +namespace std { +typedef decltype(nullptr) nullptr_t; +} +using ::std::nullptr_t; aaron.ballman wrote: > ldionne wrote: > > iana wrote: > > > aaron.ballman wrote: > >

[PATCH] D157757: [Headers] Replace __need_STDDEF_H_misc with specific __need_ macros

iana marked 13 inline comments as done. iana added inline comments. Comment at: clang/lib/Headers/stddef.h:119-121 +namespace std { +typedef decltype(nullptr) nullptr_t; +} ldionne wrote: > You could reduce the diff by not reformatting this. That's what I did

[PATCH] D157757: [Headers] Replace __need_STDDEF_H_misc with specific __need_ macros

iana added a comment. In D157757#4611492 , @iana wrote: > In D157757#4611425 , @ldionne wrote: > >> Also we should figure out why the Clang modules build is failing on top of >> this, it doesn't look like a

[PATCH] D157757: [Headers] Replace __need_STDDEF_H_misc with specific __need_ macros

iana marked an inline comment as done. iana added a comment. In D157757#4611425 , @ldionne wrote: > Also we should figure out why the Clang modules build is failing on top of > this, it doesn't look like a false positive. Where do you see that failure?

[PATCH] D157757: [Headers] Replace __need_STDDEF_H_misc with specific __need_ macros

iana updated this revision to Diff 553003. iana added a comment. Try rebasing one more time Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157757/new/ https://reviews.llvm.org/D157757 Files: clang/lib/Headers/stddef.h

[PATCH] D157757: [Headers] Replace __need_STDDEF_H_misc with specific __need_ macros

iana updated this revision to Diff 552971. iana added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157757/new/ https://reviews.llvm.org/D157757 Files: clang/lib/Headers/stddef.h clang/test/Headers/stddef.c

[PATCH] D158709: [Headers][Modules] Make separate headers for the stdarg.h and stddef.h pieces so that they can be modularized

iana created this revision. iana added reviewers: aaron.ballman, rsmith, efriedma, ldionne. Herald added subscribers: Enna1, ributzka, mstorsjo, kadircet, arphaman. Herald added a project: All. iana requested review of this revision. Herald added projects: clang, Sanitizers, LLVM,

[PATCH] D157757: [Headers] Replace __need_STDDEF_H_misc with specific __need_ macros

iana added a comment. Windows succeeding earlier, I'm pretty confident that the current crash isn't due to this review, so I'm going to go ahead and land. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157757/new/ https://reviews.llvm.org/D157757

[PATCH] D157793: [Headers] Add missing __need_ macros to stdarg.h

This revision was automatically updated to reflect the committed changes. Closed by commit rG62f769aa82f6: [Headers] Add missing __need_ macros to stdarg.h (authored by iana). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157793/new/

[PATCH] D157757: [Headers] Replace __need_STDDEF_H_misc with specific __need_ macros

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG72da678d8c84: [Headers] Replace __need_STDDEF_H_misc with specific __need_ macros (authored by iana). Repository: rG LLVM Github Monorepo

[PATCH] D157757: [Headers] Replace __need_STDDEF_H_misc with specific __need_ macros

iana marked an inline comment as done. iana added a comment. In D157757#4592766 , @philnik wrote: > I think these changes would allow us to drop the `stddef.h` compat header > from libc++, which would be really nice. I'd really rather not change how

[PATCH] D157757: [Headers] Replace __need_STDDEF_H_misc with specific __need_ macros

iana added a comment. In D157757#4592794 , @iana wrote: > In D157757#4592766 , @philnik wrote: > >> I think these changes would allow us to drop the `stddef.h` compat header >> from libc++, which would be really

[PATCH] D157757: [Headers] Replace __need_STDDEF_H_misc with specific __need_ macros

iana updated this revision to Diff 550826. iana added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157757/new/ https://reviews.llvm.org/D157757 Files: clang/lib/Headers/stddef.h clang/test/Headers/stddef.c

[PATCH] D157793: [Headers] Add missing __need_ macros to stdarg.h

iana updated this revision to Diff 550823. iana added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157793/new/ https://reviews.llvm.org/D157793 Files: clang/lib/Headers/stdarg.h clang/test/Headers/stdarg.c

[PATCH] D157757: [Headers] Replace __need_STDDEF_H_misc with specific __need_ macros

iana added a comment. In D157757#4600357 , @ldionne wrote: > This is going to be really naive, but can someone explain why we need these > `__need_X` macros? Why doesn't `` simply always declare what it > should declare? Also, does anybody

[PATCH] D158709: [Headers][Modules] Make separate headers for the stdarg.h and stddef.h pieces so that they can be modularized

iana updated this revision to Diff 553360. iana added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158709/new/ https://reviews.llvm.org/D158709 Files:

[PATCH] D158709: [Headers][Modules] Make separate headers for the stdarg.h and stddef.h pieces so that they can be modularized

iana updated this revision to Diff 553242. iana added a comment. Fixed the failing test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158709/new/ https://reviews.llvm.org/D158709 Files:

[PATCH] D158709: [Headers][Modules] Make separate headers for the stdarg.h and stddef.h pieces so that they can be modularized

iana added a comment. In D158709#4617295 , @aaron.ballman wrote: > I'm a bit concerned about the impact on (non-modules) build time performance. > This splits stddef.h and stdarg.h into needing to open 14 different files > instead of 2. Hopefully

[PATCH] D158709: [Headers][Modules] Make separate headers for the stdarg.h and stddef.h pieces so that they can be modularized

iana updated this revision to Diff 553677. iana added a comment. Modules don't support the #include #undef #include pattern to redefine macros, the second include has no effect. Move the #undef of NULL into __stddef_null.h so that the textual behavior is preserved, but the module behavior

[PATCH] D157793: [Headers] Add missing __need_ macros to stdarg.h

iana added a comment. In D157793#4588909 , @jyknight wrote: > I'm a bit confused by this change vs its description. > > It looks like stdarg already supported `__need___va_list`, which is what you > said Apple needs. Does Apple //also// require the

[PATCH] D157793: [Headers] Add missing __need_ macros to stdarg.h

iana added a comment. In D157793#4583663 , @MaskRay wrote: >> Apple needs a __need_ macro for va_list. Add one, and also ones for >> va_start/va_arg/va_end, __va_copy, va_copy. > > Do you have a link to the specification or the source that this is

[PATCH] D158021: [clang][modules] Mark builtin header 'inttypes.h' for modules

iana added a subscriber: vsapsai. iana added a comment. Herald added a subscriber: ormris. Can you add @vsapsai to the reviewers please? He was looking at this one a year or two ago. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158021/new/

[PATCH] D159018: [clang][modules] Add a c23 module feature

iana added a comment. Would we want to back port this to llvm 17? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159018/new/ https://reviews.llvm.org/D159018 ___ cfe-commits mailing list

[PATCH] D159064: [Modules] Make clang modules for the C standard library headers

iana updated this revision to Diff 554193. iana added a comment. Add `@import` tests for all of the new modules Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159064/new/ https://reviews.llvm.org/D159064 Files:

[PATCH] D159064: [Modules] Make clang modules for the C standard library headers

iana added inline comments. Comment at: clang/lib/Headers/__stddef_null.h:14 + * __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.

[PATCH] D159018: [clang][modules] Add a c23 module feature

iana created this revision. iana added reviewers: ChuanqiXu, Bigcheese, v.g.vassilev, aaron.ballman. Herald added a subscriber: ributzka. Herald added a project: All. iana requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Add a c23 module

[PATCH] D159064: [Modules] Make clang modules for the C standard library headers

iana created this revision. iana added reviewers: aaron.ballman, rsmith, efriedma, ldionne, ChuanqiXu, Bigcheese, vsapsai, benlangmuir, dexonsmith. Herald added a subscriber: ributzka. Herald added a project: All. iana requested review of this revision. Herald added a project: clang. Herald added

[PATCH] D159064: [Modules] Make clang modules for the C standard library headers

iana updated this revision to Diff 554183. iana added a comment. Remove the module cache before running the stdarg/stddef unit tests in module mode Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159064/new/ https://reviews.llvm.org/D159064 Files:

[PATCH] D159018: [clang][modules] Add a c23 module feature

iana updated this revision to Diff 554158. iana added a comment. remove the extra #endif (which oddly didn't produce an error locally so I must've ran the tests wrong the first time...) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159018/new/

[PATCH] D157757: [Headers] Replace __need_STDDEF_H_misc with specific __need_ macros

iana added inline comments. Comment at: clang/lib/Headers/stddef.h:1 /*=== stddef.h - Basic type definitions === * aaron.ballman wrote: > ldionne wrote: > > Making a thread out of this: > > > > > The relationship between

[PATCH] D132971: [clang][modules] Don't hard code [no_undeclared_includes] for the Darwin module

iana added a comment. In D132971#3759538 , @Bigcheese wrote: > I'm fine with this change, but do we actually have a backwards compatibility > policy anywhere in Clang? Would be good to know what range of SDKs a compiler > release is expected to

[PATCH] D132971: [clang][modules] Don't hard code [no_undeclared_includes] for the Darwin module

This revision was automatically updated to reflect the committed changes. Closed by commit rG260fb2bc3f79: [clang][modules] Dont hard code [no_undeclared_includes] for the Darwin module (authored by iana). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D132971: [clang][modules] Don't hard code [no_undeclared_includes] for the Darwin module

iana created this revision. iana added reviewers: bruno, ributzka, vsapsai, Bigcheese. Herald added a project: All. iana requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. The Darwin module has specified [no_undeclared_includes] for at least

[PATCH] D131213: [clang][Headers] Fix unintentional error in D130800

iana accepted this revision. iana added a comment. This revision is now accepted and ready to land. That was the only one I saw from D130800 , can someone double check me though? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D131213: [clang][Headers] Fix unintentional error in D130800

iana added a comment. In D131213#3702699 , @ddcc wrote: > I missed line 19, yeah that makes sense. @iana is that ok with you? Ah, I didn't see that either. Can we just lose the `defined` on line 26 then? It's redundant and little confusing.

[PATCH] D130800: [clang][Headers] Avoid compiler warnings in builtin headers

iana added inline comments. Comment at: clang/lib/Headers/stdint.h:99-100 typedef __UINT64_TYPE__ uint64_t; +# undef __int_least64_t # define __int_least64_t int64_t +# undef __uint_least64_t What are you seeing that's defining `__int_least64_t` and all these

[PATCH] D130800: [clang][Headers] Avoid compiler warnings in builtin headers

iana added inline comments. Comment at: clang/lib/Headers/stdint.h:99-100 typedef __UINT64_TYPE__ uint64_t; +# undef __int_least64_t # define __int_least64_t int64_t +# undef __uint_least64_t iana wrote: > What are you seeing that's defining `__int_least64_t`

[PATCH] D140250: Define NULL in its own header

iana updated this revision to Diff 486100. iana added a comment. Herald added subscribers: llvm-commits, Sanitizers, Enna1, kadircet, arphaman. Herald added projects: Sanitizers, LLVM, clang-tools-extra. Add a module for __stddef_null.h and give it the same special treatment as

[PATCH] D140250: Define NULL in its own header

iana planned changes to this revision. iana added a comment. This needs a bit more work, it looks like it's not enough to just add a header. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140250/new/ https://reviews.llvm.org/D140250

[PATCH] D140250: Define NULL in its own header

iana planned changes to this revision. iana added a comment. I think this should be all we need, but I'm still running tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140250/new/ https://reviews.llvm.org/D140250

[PATCH] D140250: Define NULL in its own header

iana created this revision. iana added reviewers: ributzka, vsapsai, Bigcheese. Herald added a subscriber: mstorsjo. Herald added a project: All. iana requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Make a new header to define NULL so that

[PATCH] D140250: Define NULL in its own header

iana added inline comments. Comment at: clang/lib/Headers/__stddef_null.h:15 +#ifdef __cplusplus +#if !defined(__MINGW32__) && !defined(_MSC_VER) +#define NULL __null `arc lint` wanted to remove all of the indentation that was in stddef.h. Repository: rG

[PATCH] D140250: Define NULL in its own header

iana updated this revision to Diff 491604. iana added a comment. Rebase, update diagnostic to include the full module name instead of just the top level. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140250/new/ https://reviews.llvm.org/D140250

[PATCH] D140250: Define NULL in its own header

iana updated this revision to Diff 491607. iana added a comment. Fix the debuginfo-generic-assignment-tracking-sroa test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140250/new/ https://reviews.llvm.org/D140250 Files:

[PATCH] D140250: Define NULL in its own header

iana added inline comments. Comment at: llvm/test/DebugInfo/Generic/assignment-tracking/sroa/unspecified-var-size.ll:37 !7 = !DIFile(filename: "clang/12.0.0/include/__stddef_max_align_t.h", directory: "/") -!8 = !DICompositeType(tag: DW_TAG_structure_type, file: !7, line: 19,

[PATCH] D140250: Define NULL in its own header

iana updated this revision to Diff 491644. iana added a comment. Fix the broken tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140250/new/ https://reviews.llvm.org/D140250 Files:

[PATCH] D140250: Define NULL in its own header

iana added a comment. In D140250#4076851 , @aaron.ballman wrote: > I'd expect there to be a test under `clang/test/Headers/` showing that this > new header works as expected. It's covered pretty well by stddefneeds.cpp already. I can add a new one if

[PATCH] D140250: Define NULL in its own header

iana added a comment. In D140250#4077956 , @aaron.ballman wrote: > In D140250#4077826 , @iana wrote: > >> In D140250#4076851 , >> @aaron.ballman wrote: >> >>> I'd

[PATCH] D140250: Define NULL in its own header

iana updated this revision to Diff 491869. iana added a comment. Add an explicit header test for __stddef_null.h Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140250/new/ https://reviews.llvm.org/D140250 Files:

[PATCH] D140250: Define NULL in its own header

iana added a comment. In D140250#4077974 , @iana wrote: > In D140250#4077956 , @aaron.ballman > wrote: > >> In D140250#4077826 , @iana wrote: >> >>> In D140250#4076851

  1   2   >