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, LangOpts.Sanitize.has(SanitizerKind::SafeStack))
----------------
benlangmuir wrote:
> iana wrote:
> > benlangmuir wrote:
> > > This appears to be in a list of `// C++ TSes`. Near the bottom there is
> > > 'Miscellaneous language extensions' which might be a better fit.
> > I thought it went with `FEATURE(modules, LangOpts.Modules)`, but I can put
> > it down in 'Miscellaneous language extensions' if that's better.
> I think misc is probably better.
👍
================
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<f_Group>,
----------------
benlangmuir wrote:
> iana wrote:
> > I don't love this flag name, but I couldn't come up with anything more
> > succinct. It actually does two things.
> >
> > # Turns on `ModuleMap::isBuiltinHeader` behavior, that is the builtin
> > headers get added into the system modules.
> > # Causes the module map parser to ignore all of the new builtin modules
> > added in D159064. This is needed before the modules are added to assist
> > with Apple internal staging with the Swift compiler.
> >
> I think the name is good enough for a -cc1 option, unless someone else has a
> better suggestion. I suggest adding a HelpText - you could basically copy the
> description you added to LangOptions.def
Sure
================
Comment at: clang/lib/Lex/ModuleMap.cpp:1540
+ /// or added to Map's Modules/ModuleScopeIDs).
+ bool IgnoreModules = false;
+
----------------
benlangmuir wrote:
> iana wrote:
> > This an everything under it is gross, but the only other thing I could
> > think of was to duplicate the module map and decided to load a special one
> > for BuiltinHeadersInSystemModules. That seemed weirder.
> Would it be possible to get the right semantics using a `requires` decl in
> the modules plus a new module feature?
Maybe? I guess e.g. _Builtin_stdint would be seen, the new requires flag would
be seen, and if it's not satisfied then its headers wouldn't be added to the
module? That seems like a really weird use of requires though.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D159483/new/
https://reviews.llvm.org/D159483
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
