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 
modules, and _Builtin_ modules are ignored for cstdlib headers")
 BENIGN_ENUM_LANGOPT(CompilingModule, CompilingModuleKind, 3, CMK_None,
----------------
vsapsai wrote:
> iana wrote:
> > vsapsai wrote:
> > > Why not to make the default value `true` to preserve the old behavior and 
> > > switch on the new behavior in the driver? It's not a veiled way to force 
> > > you to change it, I'm genuinely curious and want to consider pro/cons of 
> > > various alternatives.
> > I did it this way because the current behavior is nonintuitive and just 
> > kind of bizarre. I had no idea that `Darwin.C.stdint` also included the 
> > compiler's stdint.h, and that the OS's stdint.h was treated textually. I 
> > thought it would be better for the default behavior to be for modules to 
> > behave consistently and obviously, not have secret special cases, and be 
> > usable with C++.
> > 
> > The con of course is that if anyone besides Apple depends on the current 
> > behavior they're going to either have to fix their OS modules or add this 
> > flag. I'm not sure if other platforms have a convenient driver class where 
> > they can do that.
> I'm fine with the more aggressive approach as long as we test Swift on Linux. 
> That's the only project I know that is willing to keep using clang modules. 
> For other projects if things break and it is a motivation to move to C++ 
> modules, it is a positive direction, as for me.
> 
> Another reason I wanted to be more cautious is because of 
> https://discourse.llvm.org/t/configure-script-breakage-with-the-new-werror-implicit-function-declaration/65213
>  change. Clang started enforcing 20+ years old rule and that has broken some 
> stuff. So I don't see "you should've known better before relying on this 
> behavior" as a good argument.
> 
> Anyway, for Swift testing here is what you can do (if you have your own plan, 
> go ahead):
> 1. Make PR for the same change as this one at 
> https://github.com/apple/llvm-project/
> 2. Create inconsequential PR at https://github.com/apple/swift/
> 3. Use [[ 
> https://github.com/apple/swift/blob/main/docs/ContinuousIntegration.md#cross-repository-testing
>  | Cross Repository Testing ]] to test clang change from swift on Linux.
If things break I think we would just add the flag to a few more drivers and 
fix it like that. I'm definitely not trying to drive anyone away from clang 
modules, I'm just hoping they don't need the strange current behavior. If it 
turns out to be too much of a mess then it can be on by default and we'll do an 
fno flag instead that we can turn off in the Darwin driver when we're ready.


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

Reply via email to