sammccall marked an inline comment as done.
sammccall added inline comments.


================
Comment at: clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp:197
+}
+
+TEST(ArgStripperTest, Spellings) {
----------------
hokein wrote:
> sammccall wrote:
> > hokein wrote:
> > > add tests for stripping the diagnostic flags, `-Wfoo` etc.
> > Those aren't actually separate flags: "-W" is a Joined flag and foo is an 
> > arg.
> > Do you want a test specifically for such a string anyway?
> > Or do you want special behavior for them? (Like interaction between -Wfoo 
> > and -Wno-foo)
> I was a bit unclear about how the stripper strips the "-W/-D"-like flag, 
> would be nice to have them in tests as I think these are important cases.
> 
> > Those aren't actually separate flags: "-W" is a Joined flag and foo is an 
> > arg.
> 
> oh, if I understand correctly:
> 
> - `strip("unused", "clang -Wunused foo.cc")` => `clang foo.cc` ?
> - `strip("-W", "clang -Wunused -Wextra foo.cc")` => `clang foo.cc` // remove 
> all -W flags ?
> - `strip("error=unused", "clang -Wunused -Werror=unused -Werror=date-time 
> foo.cc")`  =>  `clang -Wunused -Werror=date-time foo.cc`?
> 
> I was a bit unclear about how the stripper strips the "-W/-D"-like flag, 
> would be nice to have them in tests as I think these are important cases.

Added.

> strip("unused", "clang -Wunused foo.cc") => clang foo.cc ?

No, we only strip clang args or flag names, not flag args (confusing 
terminology).
-W will strip -Wunused (flag name). -Wunused will strip -Wunused (clang arg). 
-W* will strip -Wunused (clang arg). but "unused" doesn't match - it's not the 
name of a flag, and it's not the arg either.

> strip("-W", "clang -Wunused -Wextra foo.cc") => clang foo.cc // remove all -W 
> flags ?

Yes

> strip("error=unused", "clang -Wunused -Werror=unused -Werror=date-time 
> foo.cc") => clang -Wunused -Werror=date-time foo.cc?

No, again we don't strip flag args.
(It's not clear how useful/easy to use this would be, maybe we can re-examine 
later?)


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

Reply via email to