vsapsai 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, ---------------- 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. ================ Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:2860-2861 + return SDKVersion >= VersionTuple(99U); + default: + return true; + } ---------------- iana wrote: > 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. Ok, that looks like a reasonable choice. 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
