[PATCH] D75395: [clang][Modules] Add -fsystem-module flag

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese marked 2 inline comments as done.
Bigcheese added inline comments.


================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1912
+    Diags.Report(diag::err_drv_argument_only_allowed_with) << "-fsystem-module"
+                                                           << "-emit-module";
 
----------------
bruno wrote:
> What happens when we use this with implicit modules? It will just be ignored? If so, users might think that using `-fsystem-module` might help them get rid of some warnings/errors, when in fact it won't.
It's an error unless you also pass -emit-module. If you also have implicit modules enabled while explicitly building a module then those modules will be treated as normal, but I don't think there's a huge cause for confusion there. The only way to use it is when explicitly building a module, so it's clear which module it applies to.


================
Comment at: clang/test/Modules/fsystem-module.m:1
+// RUN: rm -rf %t
+// RUN: rm -rf %t-saved
----------------
bruno wrote:
> Is this really needed given you are only using `%t-saved` below?
It's not, that's a leftover from the test I based this one on. I'll clean it up.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75395/new/

https://reviews.llvm.org/D75395