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

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno added a comment.

Hi Michael, thanks for improving this.



================
Comment at: clang/include/clang/Driver/Options.td:1448
   Flags<[CC1Option]>, Alias<fmodule_name_EQ>;
+def fsystem_module : Flag<["-"], "fsystem-module">, Flags<[CC1Option]>,
+  HelpText<"Build this module as a system module. Only used with -emit-module">;
----------------
I wonder if `-isystem-module` wouldn't be better since it's kinda similar with `-isystem` for headers, but for modules.


================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1912
+    Diags.Report(diag::err_drv_argument_only_allowed_with) << "-fsystem-module"
+                                                           << "-emit-module";
 
----------------
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.


================
Comment at: clang/test/Modules/fsystem-module.m:1
+// RUN: rm -rf %t
+// RUN: rm -rf %t-saved
----------------
Is this really needed given you are only using `%t-saved` below?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75395