adamcz added inline comments.
================
Comment at: clang-tools-extra/clangd/CompileCommands.cpp:256
+ case Option::RemainingArgsClass:
+ return {10000, 0};
+ case Option::RemainingArgsJoinedClass:
----------------
nit: could you replace 10000 with some constant value with descriptive name?
It's not immediately clear if this is some significant value (e.g. some flag
number or something) or just, as it's the case, just a very large number.
================
Comment at: clang-tools-extra/clangd/CompileCommands.cpp:301
+
+ // Collect sets of aliases, so we can treet -foo and -foo= as synonyms.
+ // Conceptually a double-linked list: PrevAlias[I] -> I -> NextAlias[I].
----------------
typo: treet
================
Comment at: clang-tools-extra/clangd/CompileCommands.cpp:401
+
+ // Examine args list to determine if we're in GCC, CL-compatible, or cc1
mode.
+ DriverMode MainMode = DM_GCC;
----------------
nit: this function is already quite long and this part seems like a re-usable
and self-contained piece. Could we extra this into some GetDriverMode() helper?
================
Comment at: clang-tools-extra/clangd/CompileCommands.cpp:431
+ continue; // current arg doesn't match the prefix string
+ bool PrefixMatch = Arg.size() > R.Text.size();
+ unsigned ArgCount = PrefixMatch ? R.PrefixArgs : R.ExactArgs;
----------------
Correct me if I'm wrong, but this is relying on the fact that Rules are sorted,
right? Or, to be more specific, on the fact that -foo comes before -foobar.
Consider two rules in config file, one to remove -include, another to remove
-include-pch, in that order. -include will do a prefix match on Arg ==
-include-pch and attempt to remove exactly one arg (-include is
JoinedOrSeparate, which has 1 for PrefixArgs), when in reality that was
"--include-pch foo.pch", an exact match on a different option.
So a config like:
Remove: [-include, -include-pch]
and command line:
[-include-pch, foo.pch]
should be [], but ends up being [foo.pch]
It looks like Options.inc is sorted in the correct way (include-pch will always
be before -include). I don't know if that's guaranteed, but it looks to be the
case. However, since you are adding these options to Rules on each strip()
call, you end up depending on the order of strip() calls. That seems like a bug.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D81958/new/
https://reviews.llvm.org/D81958
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
