[PATCH] D81958: [clangd] Add library to semantically strip flags by name.

[Sam McCall via Phabricator via cfe-commits]

sammccall marked 2 inline comments as done.
sammccall added a comment.

OK, I think this is good to go now.



================
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;
----------------
sammccall wrote:
> adamcz wrote:
> > 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.
> This is a really good point, I hadn't realized the option table was order-dependent (e.g. I figured -include wasn't a Joined option).
> 
> The real option parser processes the options in the order they appear in the file, so that should definitely be correct.
> I think processing them longest-to-shortest is probably also correct, since a spelling that's always shadowed by a prefix isn't that useful, I'd hope they don't actually exist.
Opted for explicitly recording the index and traversing all the rules to find the best one (instead of stopping when we find a single match).
This should cost a single factor of 2 and it's really simple.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81958