[PATCH] D78836: [clangd] Strip /showIncludes in clangd compile commands

[Nico Weber via Phabricator via cfe-commits]

thakis added inline comments.



Comment at: clang/lib/Tooling/ArgumentsAdjusters.cpp:101
   // -MF, -MG, -MP, -MT, -MQ, -MD, and -MMD.
-  if (!Arg.startswith("-M")) {
+  if (!Arg.startswith("-M") && Arg != "/showIncludes") {
 AdjustedArgs.push_back(Args[i]);

kadircet wrote:
> aeubanks wrote:
> > kadircet wrote:
> > > kadircet wrote:
> > > > this can also be --show-includes
> > > and looks like there's also `/showIncludes:user` :/
> > Handled `/showIncludes:user`, thanks for catching that. I don't see 
> > `--show-includes` though, clang-cl and clang both don't like the option?
> ah sorry, that one is a frontend option rather than a driver one. so nvm that 
> one.
clang-cl suports both /foo and -foo style flags. See the top of 
clang/include/clang/Driver/CLCompatOptions.td

You probably want to add the -showIncludes prefix here too. Ideally this 
probably wouldn't do its own commandline parsing here (?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78836



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D78836: [clangd] Strip /showIncludes in clangd compile commands

[Arthur Eubanks via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8000d506afcd: [clangd] Strip /showIncludes in clangd compile 
commands (authored by aeubanks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78836

Files:
  clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
  clang/lib/Tooling/ArgumentsAdjusters.cpp
  clang/unittests/Tooling/ToolingTest.cpp

Index: clang/unittests/Tooling/ToolingTest.cpp
===
--- clang/unittests/Tooling/ToolingTest.cpp
+++ clang/unittests/Tooling/ToolingTest.cpp
@@ -530,6 +530,62 @@
   EXPECT_TRUE(HasFlag("-w"));
 }
 
+// Check getClangStripDependencyFileAdjuster strips /showIncludes
+TEST(ClangToolTest, StripDependencyFileAdjusterShowIncludes) {
+  FixedCompilationDatabase Compilations("/", {"/showIncludes", "-c"});
+
+  ClangTool Tool(Compilations, std::vector(1, "/a.cc"));
+  Tool.mapVirtualFile("/a.cc", "void a() {}");
+
+  std::unique_ptr Action(
+  newFrontendActionFactory());
+
+  CommandLineArguments FinalArgs;
+  ArgumentsAdjuster CheckFlagsAdjuster =
+  [](const CommandLineArguments , StringRef /*unused*/) {
+FinalArgs = Args;
+return Args;
+  };
+  Tool.clearArgumentsAdjusters();
+  Tool.appendArgumentsAdjuster(getClangStripDependencyFileAdjuster());
+  Tool.appendArgumentsAdjuster(CheckFlagsAdjuster);
+  Tool.run(Action.get());
+
+  auto HasFlag = [](const std::string ) {
+return llvm::find(FinalArgs, Flag) != FinalArgs.end();
+  };
+  EXPECT_FALSE(HasFlag("/showIncludes"));
+  EXPECT_TRUE(HasFlag("-c"));
+}
+
+// Check getClangStripDependencyFileAdjuster strips /showIncludes:user
+TEST(ClangToolTest, StripDependencyFileAdjusterShowIncludesUser) {
+  FixedCompilationDatabase Compilations("/", {"/showIncludes:user", "-c"});
+
+  ClangTool Tool(Compilations, std::vector(1, "/a.cc"));
+  Tool.mapVirtualFile("/a.cc", "void a() {}");
+
+  std::unique_ptr Action(
+  newFrontendActionFactory());
+
+  CommandLineArguments FinalArgs;
+  ArgumentsAdjuster CheckFlagsAdjuster =
+  [](const CommandLineArguments , StringRef /*unused*/) {
+FinalArgs = Args;
+return Args;
+  };
+  Tool.clearArgumentsAdjusters();
+  Tool.appendArgumentsAdjuster(getClangStripDependencyFileAdjuster());
+  Tool.appendArgumentsAdjuster(CheckFlagsAdjuster);
+  Tool.run(Action.get());
+
+  auto HasFlag = [](const std::string ) {
+return llvm::find(FinalArgs, Flag) != FinalArgs.end();
+  };
+  EXPECT_FALSE(HasFlag("/showIncludes:user"));
+  EXPECT_TRUE(HasFlag("-c"));
+}
+
 // Check getClangStripPluginsAdjuster strips plugin related args.
 TEST(ClangToolTest, StripPluginsAdjuster) {
   FixedCompilationDatabase Compilations(
Index: clang/lib/Tooling/ArgumentsAdjusters.cpp
===
--- clang/lib/Tooling/ArgumentsAdjusters.cpp
+++ clang/lib/Tooling/ArgumentsAdjusters.cpp
@@ -98,7 +98,7 @@
   StringRef Arg = Args[i];
   // All dependency-file options begin with -M. These include -MM,
   // -MF, -MG, -MP, -MT, -MQ, -MD, and -MMD.
-  if (!Arg.startswith("-M")) {
+  if (!Arg.startswith("-M") && !Arg.startswith("/showIncludes")) {
 AdjustedArgs.push_back(Args[i]);
 continue;
   }
Index: clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
===
--- clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
+++ clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
@@ -79,6 +79,20 @@
 EXPECT_THAT(Cmd, Not(Contains(Stripped)));
 }
 
+TEST(CommandMangler, StripShowIncludes) {
+  auto Mangler = CommandMangler::forTests();
+  std::vector Cmd = {"clang-cl", "/showIncludes", "foo.cc"};
+  Mangler.adjust(Cmd);
+  EXPECT_THAT(Cmd, Not(Contains("/showIncludes")));
+}
+
+TEST(CommandMangler, StripShowIncludesUser) {
+  auto Mangler = CommandMangler::forTests();
+  std::vector Cmd = {"clang-cl", "/showIncludes:user", "foo.cc"};
+  Mangler.adjust(Cmd);
+  EXPECT_THAT(Cmd, Not(Contains("/showIncludes:user")));
+}
+
 TEST(CommandMangler, ClangPath) {
   auto Mangler = CommandMangler::forTests();
   Mangler.ClangPath = testPath("fake/clang");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D78836: [clangd] Strip /showIncludes in clangd compile commands

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks added a comment.

In D78836#2003696 , @kadircet wrote:

> thanks, lgtm. do you have commit access?


Yep :)

Thanks for the reviews!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78836



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D78836: [clangd] Strip /showIncludes in clangd compile commands

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet accepted this revision.
kadircet added a comment.

thanks, lgtm. do you have commit access?




Comment at: clang/lib/Tooling/ArgumentsAdjusters.cpp:101
   // -MF, -MG, -MP, -MT, -MQ, -MD, and -MMD.
-  if (!Arg.startswith("-M")) {
+  if (!Arg.startswith("-M") && Arg != "/showIncludes") {
 AdjustedArgs.push_back(Args[i]);

aeubanks wrote:
> kadircet wrote:
> > kadircet wrote:
> > > this can also be --show-includes
> > and looks like there's also `/showIncludes:user` :/
> Handled `/showIncludes:user`, thanks for catching that. I don't see 
> `--show-includes` though, clang-cl and clang both don't like the option?
ah sorry, that one is a frontend option rather than a driver one. so nvm that 
one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78836



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D78836: [clangd] Strip /showIncludes in clangd compile commands

[Arthur Eubanks via Phabricator via cfe-commits]

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



Comment at: clang/lib/Tooling/ArgumentsAdjusters.cpp:101
   // -MF, -MG, -MP, -MT, -MQ, -MD, and -MMD.
-  if (!Arg.startswith("-M")) {
+  if (!Arg.startswith("-M") && Arg != "/showIncludes") {
 AdjustedArgs.push_back(Args[i]);

kadircet wrote:
> kadircet wrote:
> > this can also be --show-includes
> and looks like there's also `/showIncludes:user` :/
Handled `/showIncludes:user`, thanks for catching that. I don't see 
`--show-includes` though, clang-cl and clang both don't like the option?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78836



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D78836: [clangd] Strip /showIncludes in clangd compile commands

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks updated this revision to Diff 260103.
aeubanks added a comment.

Handle /showIncludes:user


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78836

Files:
  clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
  clang/lib/Tooling/ArgumentsAdjusters.cpp
  clang/unittests/Tooling/ToolingTest.cpp

Index: clang/unittests/Tooling/ToolingTest.cpp
===
--- clang/unittests/Tooling/ToolingTest.cpp
+++ clang/unittests/Tooling/ToolingTest.cpp
@@ -530,6 +530,62 @@
   EXPECT_TRUE(HasFlag("-w"));
 }
 
+// Check getClangStripDependencyFileAdjuster strips /showIncludes
+TEST(ClangToolTest, StripDependencyFileAdjusterShowIncludes) {
+  FixedCompilationDatabase Compilations("/", {"/showIncludes", "-c"});
+
+  ClangTool Tool(Compilations, std::vector(1, "/a.cc"));
+  Tool.mapVirtualFile("/a.cc", "void a() {}");
+
+  std::unique_ptr Action(
+  newFrontendActionFactory());
+
+  CommandLineArguments FinalArgs;
+  ArgumentsAdjuster CheckFlagsAdjuster =
+  [](const CommandLineArguments , StringRef /*unused*/) {
+FinalArgs = Args;
+return Args;
+  };
+  Tool.clearArgumentsAdjusters();
+  Tool.appendArgumentsAdjuster(getClangStripDependencyFileAdjuster());
+  Tool.appendArgumentsAdjuster(CheckFlagsAdjuster);
+  Tool.run(Action.get());
+
+  auto HasFlag = [](const std::string ) {
+return llvm::find(FinalArgs, Flag) != FinalArgs.end();
+  };
+  EXPECT_FALSE(HasFlag("/showIncludes"));
+  EXPECT_TRUE(HasFlag("-c"));
+}
+
+// Check getClangStripDependencyFileAdjuster strips /showIncludes:user
+TEST(ClangToolTest, StripDependencyFileAdjusterShowIncludesUser) {
+  FixedCompilationDatabase Compilations("/", {"/showIncludes:user", "-c"});
+
+  ClangTool Tool(Compilations, std::vector(1, "/a.cc"));
+  Tool.mapVirtualFile("/a.cc", "void a() {}");
+
+  std::unique_ptr Action(
+  newFrontendActionFactory());
+
+  CommandLineArguments FinalArgs;
+  ArgumentsAdjuster CheckFlagsAdjuster =
+  [](const CommandLineArguments , StringRef /*unused*/) {
+FinalArgs = Args;
+return Args;
+  };
+  Tool.clearArgumentsAdjusters();
+  Tool.appendArgumentsAdjuster(getClangStripDependencyFileAdjuster());
+  Tool.appendArgumentsAdjuster(CheckFlagsAdjuster);
+  Tool.run(Action.get());
+
+  auto HasFlag = [](const std::string ) {
+return llvm::find(FinalArgs, Flag) != FinalArgs.end();
+  };
+  EXPECT_FALSE(HasFlag("/showIncludes:user"));
+  EXPECT_TRUE(HasFlag("-c"));
+}
+
 // Check getClangStripPluginsAdjuster strips plugin related args.
 TEST(ClangToolTest, StripPluginsAdjuster) {
   FixedCompilationDatabase Compilations(
Index: clang/lib/Tooling/ArgumentsAdjusters.cpp
===
--- clang/lib/Tooling/ArgumentsAdjusters.cpp
+++ clang/lib/Tooling/ArgumentsAdjusters.cpp
@@ -98,7 +98,7 @@
   StringRef Arg = Args[i];
   // All dependency-file options begin with -M. These include -MM,
   // -MF, -MG, -MP, -MT, -MQ, -MD, and -MMD.
-  if (!Arg.startswith("-M")) {
+  if (!Arg.startswith("-M") && !Arg.startswith("/showIncludes")) {
 AdjustedArgs.push_back(Args[i]);
 continue;
   }
Index: clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
===
--- clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
+++ clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
@@ -79,6 +79,20 @@
 EXPECT_THAT(Cmd, Not(Contains(Stripped)));
 }
 
+TEST(CommandMangler, StripShowIncludes) {
+  auto Mangler = CommandMangler::forTests();
+  std::vector Cmd = {"clang-cl", "/showIncludes", "foo.cc"};
+  Mangler.adjust(Cmd);
+  EXPECT_THAT(Cmd, Not(Contains("/showIncludes")));
+}
+
+TEST(CommandMangler, StripShowIncludesUser) {
+  auto Mangler = CommandMangler::forTests();
+  std::vector Cmd = {"clang-cl", "/showIncludes:user", "foo.cc"};
+  Mangler.adjust(Cmd);
+  EXPECT_THAT(Cmd, Not(Contains("/showIncludes:user")));
+}
+
 TEST(CommandMangler, ClangPath) {
   auto Mangler = CommandMangler::forTests();
   Mangler.ClangPath = testPath("fake/clang");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D78836: [clangd] Strip /showIncludes in clangd compile commands

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang/lib/Tooling/ArgumentsAdjusters.cpp:101
   // -MF, -MG, -MP, -MT, -MQ, -MD, and -MMD.
-  if (!Arg.startswith("-M")) {
+  if (!Arg.startswith("-M") && Arg != "/showIncludes") {
 AdjustedArgs.push_back(Args[i]);

kadircet wrote:
> this can also be --show-includes
and looks like there's also `/showIncludes:user` :/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78836



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D78836: [clangd] Strip /showIncludes in clangd compile commands

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang/lib/Tooling/ArgumentsAdjusters.cpp:101
   // -MF, -MG, -MP, -MT, -MQ, -MD, and -MMD.
-  if (!Arg.startswith("-M")) {
+  if (!Arg.startswith("-M") && Arg != "/showIncludes") {
 AdjustedArgs.push_back(Args[i]);

this can also be --show-includes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78836



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D78836: [clangd] Strip /showIncludes in clangd compile commands

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks updated this revision to Diff 260040.
aeubanks added a comment.

Add test in ToolingTest


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78836

Files:
  clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
  clang/lib/Tooling/ArgumentsAdjusters.cpp
  clang/unittests/Tooling/ToolingTest.cpp


Index: clang/unittests/Tooling/ToolingTest.cpp
===
--- clang/unittests/Tooling/ToolingTest.cpp
+++ clang/unittests/Tooling/ToolingTest.cpp
@@ -530,6 +530,34 @@
   EXPECT_TRUE(HasFlag("-w"));
 }
 
+// Check getClangStripDependencyFileAdjuster strips /showIncludes
+TEST(ClangToolTest, StripDependencyFileAdjusterShowIncludes) {
+  FixedCompilationDatabase Compilations("/", {"/showIncludes", "-c"});
+
+  ClangTool Tool(Compilations, std::vector(1, "/a.cc"));
+  Tool.mapVirtualFile("/a.cc", "void a() {}");
+
+  std::unique_ptr Action(
+  newFrontendActionFactory());
+
+  CommandLineArguments FinalArgs;
+  ArgumentsAdjuster CheckFlagsAdjuster =
+[](const CommandLineArguments , StringRef /*unused*/) {
+  FinalArgs = Args;
+  return Args;
+};
+  Tool.clearArgumentsAdjusters();
+  Tool.appendArgumentsAdjuster(getClangStripDependencyFileAdjuster());
+  Tool.appendArgumentsAdjuster(CheckFlagsAdjuster);
+  Tool.run(Action.get());
+
+  auto HasFlag = [](const std::string ) {
+return llvm::find(FinalArgs, Flag) != FinalArgs.end();
+  };
+  EXPECT_FALSE(HasFlag("/showIncludes"));
+  EXPECT_TRUE(HasFlag("-c"));
+}
+
 // Check getClangStripPluginsAdjuster strips plugin related args.
 TEST(ClangToolTest, StripPluginsAdjuster) {
   FixedCompilationDatabase Compilations(
Index: clang/lib/Tooling/ArgumentsAdjusters.cpp
===
--- clang/lib/Tooling/ArgumentsAdjusters.cpp
+++ clang/lib/Tooling/ArgumentsAdjusters.cpp
@@ -98,7 +98,7 @@
   StringRef Arg = Args[i];
   // All dependency-file options begin with -M. These include -MM,
   // -MF, -MG, -MP, -MT, -MQ, -MD, and -MMD.
-  if (!Arg.startswith("-M")) {
+  if (!Arg.startswith("-M") && Arg != "/showIncludes") {
 AdjustedArgs.push_back(Args[i]);
 continue;
   }
Index: clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
===
--- clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
+++ clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
@@ -79,6 +79,13 @@
 EXPECT_THAT(Cmd, Not(Contains(Stripped)));
 }
 
+TEST(CommandMangler, StripShowIncludes) {
+  auto Mangler = CommandMangler::forTests();
+  std::vector Cmd = {"clang-cl", "/showIncludes", "foo.cc"};
+  Mangler.adjust(Cmd);
+  EXPECT_THAT(Cmd, Not(Contains("/showIncludes")));
+}
+
 TEST(CommandMangler, ClangPath) {
   auto Mangler = CommandMangler::forTests();
   Mangler.ClangPath = testPath("fake/clang");


Index: clang/unittests/Tooling/ToolingTest.cpp
===
--- clang/unittests/Tooling/ToolingTest.cpp
+++ clang/unittests/Tooling/ToolingTest.cpp
@@ -530,6 +530,34 @@
   EXPECT_TRUE(HasFlag("-w"));
 }
 
+// Check getClangStripDependencyFileAdjuster strips /showIncludes
+TEST(ClangToolTest, StripDependencyFileAdjusterShowIncludes) {
+  FixedCompilationDatabase Compilations("/", {"/showIncludes", "-c"});
+
+  ClangTool Tool(Compilations, std::vector(1, "/a.cc"));
+  Tool.mapVirtualFile("/a.cc", "void a() {}");
+
+  std::unique_ptr Action(
+  newFrontendActionFactory());
+
+  CommandLineArguments FinalArgs;
+  ArgumentsAdjuster CheckFlagsAdjuster =
+[](const CommandLineArguments , StringRef /*unused*/) {
+  FinalArgs = Args;
+  return Args;
+};
+  Tool.clearArgumentsAdjusters();
+  Tool.appendArgumentsAdjuster(getClangStripDependencyFileAdjuster());
+  Tool.appendArgumentsAdjuster(CheckFlagsAdjuster);
+  Tool.run(Action.get());
+
+  auto HasFlag = [](const std::string ) {
+return llvm::find(FinalArgs, Flag) != FinalArgs.end();
+  };
+  EXPECT_FALSE(HasFlag("/showIncludes"));
+  EXPECT_TRUE(HasFlag("-c"));
+}
+
 // Check getClangStripPluginsAdjuster strips plugin related args.
 TEST(ClangToolTest, StripPluginsAdjuster) {
   FixedCompilationDatabase Compilations(
Index: clang/lib/Tooling/ArgumentsAdjusters.cpp
===
--- clang/lib/Tooling/ArgumentsAdjusters.cpp
+++ clang/lib/Tooling/ArgumentsAdjusters.cpp
@@ -98,7 +98,7 @@
   StringRef Arg = Args[i];
   // All dependency-file options begin with -M. These include -MM,
   // -MF, -MG, -MP, -MT, -MQ, -MD, and -MMD.
-  if (!Arg.startswith("-M")) {
+  if (!Arg.startswith("-M") && Arg != "/showIncludes") {
 AdjustedArgs.push_back(Args[i]);
 continue;
   }
Index: clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp

[PATCH] D78836: [clangd] Strip /showIncludes in clangd compile commands

[Arthur Eubanks via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp:82
 
+TEST(CommandMangler, StripShowIncludes) {
+  auto Mangler = CommandMangler::forTests();

sammccall wrote:
> It's useful to have this test to verify the behavior at the clangd level, but 
> the arg adjuster is a separate library and also used in other contexts.
> 
> The unit-test for this arg-adjuster lives in 
> clang/unittests/Tooling/ToolingTest.cpp around line 500. Could you add a test 
> for the new behavior there, too?
Done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78836



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D78836: [clangd] Strip /showIncludes in clangd compile commands

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

Thanks!




Comment at: clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp:82
 
+TEST(CommandMangler, StripShowIncludes) {
+  auto Mangler = CommandMangler::forTests();

It's useful to have this test to verify the behavior at the clangd level, but 
the arg adjuster is a separate library and also used in other contexts.

The unit-test for this arg-adjuster lives in 
clang/unittests/Tooling/ToolingTest.cpp around line 500. Could you add a test 
for the new behavior there, too?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78836



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D78836: [clangd] Strip /showIncludes in clangd compile commands

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks added a comment.
Herald added a subscriber: ormris.

I did verify that clangd on vscode now works on Windows on the LLVM codebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78836



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits