Bigcheese added inline comments.
================
Comment at: clang/include/clang/Frontend/CompilerInvocation.h:248
+ /// \returns - True if parsing was successful, false otherwise
+ bool parseSimpleArgs(const llvm::opt::ArgList &Args,
+ DiagnosticsEngine &Diags);
----------------
Is there a reason for this to be a member of `CompilerInvocation`? The rest of
the argument parsing functions are static file local functions.
================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3658
MissingArgCount, IncludedFlagsBitmask);
+
LangOptions &LangOpts = *Res.getLangOpts();
----------------
Put formatting fixups in a separate commit.
================
Comment at: clang/unittests/Frontend/CompilerInvocationTest.cpp:41
+TEST_F(CC1CommandLineGenerationTest, CanGenerateCC1CommandLineFlag) {
+ const char *Args[] = {"clang", "-xc++", "-fmodules-strict-context-hash",
"-"};
+
----------------
This reminded me that some of the -cc1 arguments are positional such as
`-xc++`. I think we'll need custom handling for `INPUT` arguments, although we
don't need them (or want them) for the modules use case. Do you have any
thoughts on how to handle them? I don't think answering that should block the
initial patch though.
================
Comment at: llvm/include/llvm/Option/OptParser.td:153
+}
+class MarshallingFlagRequired<code keypath, bit ispositive, code defaultvalue>
+ : MarshallingFlag<keypath, ispositive, defaultvalue> {
----------------
I'm not sure Required is a great name here. I initially read that as the option
was required, not that it should always be emitted.
================
Comment at: llvm/include/llvm/Option/OptParser.td:167-171
+class MarshallingEnum<code keypath, code defaultvalue, list<code> enumvalues>
+ : OptionMarshallingInfo<"enum", keypath> {
+ code DefaultValue = defaultvalue;
+ list<code>EnumValues = enumvalues;
+}
----------------
I noticed that this isn't being used for the enum case you added
(`-mrelocation-model`). Do you intend to use it? You should either use it in
this patch or remove it until it actually is used.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79796/new/
https://reviews.llvm.org/D79796
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
