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:
> 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.


================
Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:2851
+  if (!SDKInfo)
+    return false;
+  
----------------
benlangmuir wrote:
> I would have expected the default to be true; do old SDKs not have SDKInfo or 
> something?
I'm not sure if we ever expect there to not be SDKInfo, but I thought it safer 
to default to the current behavior which is false.


================
Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:2856
+  case Darwin::MacOS:
+    return SDKVersion >= VersionTuple(100U);
+  case Darwin::IPhoneOS:
----------------
benlangmuir wrote:
> Are these really supposed to be 100 or is this a placeholder?
Placeholder until we actually update the SDKs.


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