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 
modules, and _Builtin_ modules are ignored for cstdlib headers")
 BENIGN_ENUM_LANGOPT(CompilingModule, CompilingModuleKind, 3, CMK_None,
----------------
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.


================
Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:2860-2861
+    return SDKVersion >= VersionTuple(99U);
+  default:
+    return true;
+  }
----------------
vsapsai wrote:
> Another question regarding defaults. Doesn't look consistent with your 
> position
> > [...] I thought it safer to default to the current behavior which is false.
> 
> Personally I gravitate towards `false` for extra safety but it's not a strong 
> opinion.
default only covers DriverKit, which can be true. Any future platforms should 
also default to true.


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