[PATCH] D114213: Compilation Database: Point Bazel users to a solution

[Christopher Sauer via Phabricator via cfe-commits]

cpsauer created this revision.
cpsauer added a reviewer: sammccall.
cpsauer created this object with visibility "All Users".
Herald added subscribers: usaxena95, kadircet.
cpsauer requested review of this revision.
Herald added subscribers: cfe-commits, ilya-biryukov.
Herald added a project: clang.

This doc lists ways of getting compilation databases out of some popular build 
systems for use with clangd and other tooling.

We built such a way for Bazel and just released it after a bunch of requests on 
GitHub. Thought I should propose that we add a link to help people find it and 
use clang tooling.

(This is my first revision submitted via LLVM Phabricator, so if I've messed 
something up, I'd really appreciate your help and patience. Asking for a review 
from Sam McCall, because I've had a great time working with him elsewhere on 
GitHub before and because I saw him in the Git history for this file.)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114213

Files:
  clang/docs/JSONCompilationDatabase.rst


Index: clang/docs/JSONCompilationDatabase.rst
===
--- clang/docs/JSONCompilationDatabase.rst
+++ clang/docs/JSONCompilationDatabase.rst
@@ -36,6 +36,12 @@
 For projects on Linux, there is an alternative to intercept compiler
 calls with a tool called `Bear `_.
 
+`Bazel `_ can export a compilation database via 
+`this extractor extension 
+`_.
+Bazel is otherwise resistant to Bear and other compiler-intercept
+techniques.
+
 Clang's tooling interface supports reading compilation databases; see
 the :doc:`LibTooling documentation `. libclang and its
 python bindings also support this (since clang 3.2); see


Index: clang/docs/JSONCompilationDatabase.rst
===
--- clang/docs/JSONCompilationDatabase.rst
+++ clang/docs/JSONCompilationDatabase.rst
@@ -36,6 +36,12 @@
 For projects on Linux, there is an alternative to intercept compiler
 calls with a tool called `Bear `_.
 
+`Bazel `_ can export a compilation database via 
+`this extractor extension 
+`_.
+Bazel is otherwise resistant to Bear and other compiler-intercept
+techniques.
+
 Clang's tooling interface supports reading compilation databases; see
 the :doc:`LibTooling documentation `. libclang and its
 python bindings also support this (since clang 3.2); see
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114213: Compilation Database: Point Bazel users to a solution

[Christopher Sauer via Phabricator via cfe-commits]

cpsauer added a comment.

Good call, @nridge. Will do as soon as we land this one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114213

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

[PATCH] D114213: Compilation Database: Point Bazel users to a solution

[Christopher Sauer via Phabricator via cfe-commits]

cpsauer added a comment.

@sammccall (or others), could I ask for your help landing the change now that 
it's approved?

(I think I need to ask someone with commit access to do so, per 
https://llvm.org/docs/Phabricator.html#committing-a-change)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114213

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

[PATCH] D114213: Compilation Database: Point Bazel users to a solution

[Christopher Sauer via Phabricator via cfe-commits]

cpsauer added a comment.

Wahoo! Thank you


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114213

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

[PATCH] D138546: Clangd: Preserve target flags in system includes extractor

[Christopher Sauer via Phabricator via cfe-commits]

cpsauer added a subscriber: nridge.
cpsauer added a comment.

Ooh interesting. Thanks for your careful look, @ArcsinX.

To check my understanding: Sounds like, more generally, commands get massaged 
into clang argument form before they're passed into the query-driver machinery, 
which can break break calls through to other compilers.

So far that hadn't affected things, but probably we should be relayering things 
to operate on raw commands for query driver?
@nridge, I know you'd fixed a bunch of problems by relayering in the past. Any 
thoughts on this one?

More generally folks more experienced in the codebase: Any thoughts on how we 
should do this?


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

https://reviews.llvm.org/D138546

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

[PATCH] D138546: Clangd: Preserve target flags in system includes extractor

[Christopher Sauer via Phabricator via cfe-commits]

cpsauer added a comment.

@nridge, yep, confirming: For Android, --target is being added explicitly as 
part of the command and we'll need to pass through explicit (but not implicit) 
--target flags to the system includes extractor to get the correct paths.


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

https://reviews.llvm.org/D138546

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

[PATCH] D138546: Clangd: Preserve target flags in system includes extractor

[Christopher Sauer via Phabricator via cfe-commits]

cpsauer added a comment.

Thanks, Nathan. Makes sense; sounds good.

@kadircet, I think this one is ready for you, whenever you want it. Lmk how 
you'd like to proceed.


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

https://reviews.llvm.org/D138546

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

[PATCH] D138546: Clangd: Preserve target flags in system includes extractor

[Christopher Sauer via Phabricator via cfe-commits]

cpsauer updated this revision to Diff 489208.
cpsauer added a comment.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

@nridge, I took a shot at the change you suggested. Confirming that this what 
you were thinking of, removing `inferTargetAndDriverMode` from database load 
and replacing it with a call to the underlying `addTargetAndModeForProgramName` 
in the `CommandMangler` after the `SystemIncludeExtractor` is invoked?
Note that I also saw a parallel call to `inferTargetAndDriverMode` in 
`JSONCompilationDatabasePlugin`, which seemed like it might cause the same 
problem, so I eliminated the call in there, too. I then culled the dead code, 
since there weren't other call sites for the `addTargetAndModeForProgramName` 
wrappings. Could I ask you to sanity check that? I know that's a more general 
tooling library; I was having trouble determining whether other tooling needed 
the target inference or not.  
Also, @ArcsinX, could I get your take? Do you think this would fix the layering 
issue you raised?

We're definitely stretching my (currently quite limited) understanding of how 
clangd's implementation fits together, so I think I'll need to ask for 
help/guidance if that looks wrong.

Thanks again, guys!
-CS


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

https://reviews.llvm.org/D138546

Files:
  clang-tools-extra/clangd/CompileCommands.cpp
  clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
  clang-tools-extra/clangd/SystemIncludeExtractor.cpp
  clang-tools-extra/clangd/test/system-include-extractor.test
  clang/docs/tools/clang-formatted-files.txt
  clang/include/clang/Tooling/CompilationDatabase.h
  clang/lib/Tooling/CMakeLists.txt
  clang/lib/Tooling/GuessTargetAndModeCompilationDatabase.cpp
  clang/lib/Tooling/JSONCompilationDatabase.cpp
  clang/unittests/Tooling/CompilationDatabaseTest.cpp
  llvm/utils/gn/secondary/clang/lib/Tooling/BUILD.gn

Index: llvm/utils/gn/secondary/clang/lib/Tooling/BUILD.gn
===
--- llvm/utils/gn/secondary/clang/lib/Tooling/BUILD.gn
+++ llvm/utils/gn/secondary/clang/lib/Tooling/BUILD.gn
@@ -37,7 +37,6 @@
 "ExpandResponseFilesCompilationDatabase.cpp",
 "FileMatchTrie.cpp",
 "FixIt.cpp",
-"GuessTargetAndModeCompilationDatabase.cpp",
 "InterpolatingCompilationDatabase.cpp",
 "JSONCompilationDatabase.cpp",
 "NodeIntrospection.cpp",
Index: clang/unittests/Tooling/CompilationDatabaseTest.cpp
===
--- clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -929,31 +929,6 @@
   EXPECT_TRUE(CCRef != CCTest);
 }
 
-class TargetAndModeTest : public MemDBTest {
-public:
-  TargetAndModeTest() { llvm::InitializeAllTargetInfos(); }
-
-protected:
-  // Look up the command from a relative path, and return it in string form.
-  std::string getCommand(llvm::StringRef F) {
-auto Results = inferTargetAndDriverMode(std::make_unique(Entries))
-   ->getCompileCommands(path(F));
-if (Results.empty())
-  return "none";
-return llvm::join(Results[0].CommandLine, " ");
-  }
-};
-
-TEST_F(TargetAndModeTest, TargetAndMode) {
-  add("foo.cpp", "clang-cl", "");
-  add("bar.cpp", "clang++", "");
-
-  EXPECT_EQ(getCommand("foo.cpp"),
-"clang-cl --driver-mode=cl foo.cpp -D foo.cpp");
-  EXPECT_EQ(getCommand("bar.cpp"),
-"clang++ --driver-mode=g++ bar.cpp -D bar.cpp");
-}
-
 class ExpandResponseFilesTest : public MemDBTest {
 public:
   ExpandResponseFilesTest() : FS(new llvm::vfs::InMemoryFileSystem) {}
Index: clang/lib/Tooling/JSONCompilationDatabase.cpp
===
--- clang/lib/Tooling/JSONCompilationDatabase.cpp
+++ clang/lib/Tooling/JSONCompilationDatabase.cpp
@@ -165,9 +165,8 @@
 llvm::sys::path::append(JSONDatabasePath, "compile_commands.json");
 auto Base = JSONCompilationDatabase::loadFromFile(
 JSONDatabasePath, ErrorMessage, JSONCommandLineSyntax::AutoDetect);
-return Base ? inferTargetAndDriverMode(
-  inferMissingCompileCommands(expandResponseFiles(
-  std::move(Base), llvm::vfs::getRealFileSystem(
+return Base ? inferMissingCompileCommands(expandResponseFiles(
+  std::move(Base), llvm::vfs::getRealFileSystem()))
 : nullptr;
   }
 };
Index: clang/lib/Tooling/GuessTargetAndModeCompilationDatabase.cpp
===
--- clang/lib/Tooling/GuessTargetAndModeCompilationDatabase.cpp
+++ /dev/null
@@ -1,57 +0,0 @@
-//===- GuessTargetAndModeCompilationDatabase.cpp --===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: A

[PATCH] D138546: Clangd: Preserve target flags in system includes extractor

[Christopher Sauer via Phabricator via cfe-commits]

cpsauer updated this revision to Diff 489214.

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

https://reviews.llvm.org/D138546

Files:
  clang-tools-extra/clangd/CompileCommands.cpp
  clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
  clang-tools-extra/clangd/SystemIncludeExtractor.cpp
  clang-tools-extra/clangd/test/system-include-extractor.test
  clang/docs/tools/clang-formatted-files.txt
  clang/include/clang/Tooling/CompilationDatabase.h
  clang/lib/Tooling/CMakeLists.txt
  clang/lib/Tooling/GuessTargetAndModeCompilationDatabase.cpp
  clang/lib/Tooling/JSONCompilationDatabase.cpp
  clang/unittests/Tooling/CompilationDatabaseTest.cpp
  llvm/utils/gn/secondary/clang/lib/Tooling/BUILD.gn

Index: llvm/utils/gn/secondary/clang/lib/Tooling/BUILD.gn
===
--- llvm/utils/gn/secondary/clang/lib/Tooling/BUILD.gn
+++ llvm/utils/gn/secondary/clang/lib/Tooling/BUILD.gn
@@ -37,7 +37,6 @@
 "ExpandResponseFilesCompilationDatabase.cpp",
 "FileMatchTrie.cpp",
 "FixIt.cpp",
-"GuessTargetAndModeCompilationDatabase.cpp",
 "InterpolatingCompilationDatabase.cpp",
 "JSONCompilationDatabase.cpp",
 "NodeIntrospection.cpp",
Index: clang/unittests/Tooling/CompilationDatabaseTest.cpp
===
--- clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -929,31 +929,6 @@
   EXPECT_TRUE(CCRef != CCTest);
 }
 
-class TargetAndModeTest : public MemDBTest {
-public:
-  TargetAndModeTest() { llvm::InitializeAllTargetInfos(); }
-
-protected:
-  // Look up the command from a relative path, and return it in string form.
-  std::string getCommand(llvm::StringRef F) {
-auto Results = inferTargetAndDriverMode(std::make_unique(Entries))
-   ->getCompileCommands(path(F));
-if (Results.empty())
-  return "none";
-return llvm::join(Results[0].CommandLine, " ");
-  }
-};
-
-TEST_F(TargetAndModeTest, TargetAndMode) {
-  add("foo.cpp", "clang-cl", "");
-  add("bar.cpp", "clang++", "");
-
-  EXPECT_EQ(getCommand("foo.cpp"),
-"clang-cl --driver-mode=cl foo.cpp -D foo.cpp");
-  EXPECT_EQ(getCommand("bar.cpp"),
-"clang++ --driver-mode=g++ bar.cpp -D bar.cpp");
-}
-
 class ExpandResponseFilesTest : public MemDBTest {
 public:
   ExpandResponseFilesTest() : FS(new llvm::vfs::InMemoryFileSystem) {}
Index: clang/lib/Tooling/JSONCompilationDatabase.cpp
===
--- clang/lib/Tooling/JSONCompilationDatabase.cpp
+++ clang/lib/Tooling/JSONCompilationDatabase.cpp
@@ -165,9 +165,8 @@
 llvm::sys::path::append(JSONDatabasePath, "compile_commands.json");
 auto Base = JSONCompilationDatabase::loadFromFile(
 JSONDatabasePath, ErrorMessage, JSONCommandLineSyntax::AutoDetect);
-return Base ? inferTargetAndDriverMode(
-  inferMissingCompileCommands(expandResponseFiles(
-  std::move(Base), llvm::vfs::getRealFileSystem(
+return Base ? inferMissingCompileCommands(expandResponseFiles(
+  std::move(Base), llvm::vfs::getRealFileSystem()))
 : nullptr;
   }
 };
Index: clang/lib/Tooling/GuessTargetAndModeCompilationDatabase.cpp
===
--- clang/lib/Tooling/GuessTargetAndModeCompilationDatabase.cpp
+++ /dev/null
@@ -1,57 +0,0 @@
-//===- GuessTargetAndModeCompilationDatabase.cpp --===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#include "clang/Tooling/CompilationDatabase.h"
-#include "clang/Tooling/Tooling.h"
-#include 
-
-namespace clang {
-namespace tooling {
-
-namespace {
-class TargetAndModeAdderDatabase : public CompilationDatabase {
-public:
-  TargetAndModeAdderDatabase(std::unique_ptr Base)
-  : Base(std::move(Base)) {
-assert(this->Base != nullptr);
-  }
-
-  std::vector getAllFiles() const override {
-return Base->getAllFiles();
-  }
-
-  std::vector getAllCompileCommands() const override {
-return addTargetAndMode(Base->getAllCompileCommands());
-  }
-
-  std::vector
-  getCompileCommands(StringRef FilePath) const override {
-return addTargetAndMode(Base->getCompileCommands(FilePath));
-  }
-
-private:
-  std::vector
-  addTargetAndMode(std::vector Cmds) const {
-for (auto &Cmd : Cmds) {
-  if (Cmd.CommandLine.empty())
-continue;
-  addTargetAndModeForProgramName(Cmd.CommandLine, Cmd.CommandLine.front());
-}
-return Cmds;
-  }
-  std::unique_ptr Base;
-};
-} // namespace
-
-std::unique_ptr
-inferTargetAndDriverMode(std::unique_ptr Base) {
-  

[PATCH] D138546: Clangd: Preserve target flags in system includes extractor

[Christopher Sauer via Phabricator via cfe-commits]

cpsauer updated this revision to Diff 489216.

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

https://reviews.llvm.org/D138546

Files:
  clang-tools-extra/clangd/CompileCommands.cpp
  clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
  clang-tools-extra/clangd/SystemIncludeExtractor.cpp
  clang-tools-extra/clangd/test/system-include-extractor.test
  clang/bindings/python/tests/cindex/test_cdb.py
  clang/docs/tools/clang-formatted-files.txt
  clang/include/clang/Tooling/CompilationDatabase.h
  clang/lib/Tooling/CMakeLists.txt
  clang/lib/Tooling/GuessTargetAndModeCompilationDatabase.cpp
  clang/lib/Tooling/JSONCompilationDatabase.cpp
  clang/unittests/Tooling/CompilationDatabaseTest.cpp
  llvm/utils/gn/secondary/clang/lib/Tooling/BUILD.gn

Index: llvm/utils/gn/secondary/clang/lib/Tooling/BUILD.gn
===
--- llvm/utils/gn/secondary/clang/lib/Tooling/BUILD.gn
+++ llvm/utils/gn/secondary/clang/lib/Tooling/BUILD.gn
@@ -37,7 +37,6 @@
 "ExpandResponseFilesCompilationDatabase.cpp",
 "FileMatchTrie.cpp",
 "FixIt.cpp",
-"GuessTargetAndModeCompilationDatabase.cpp",
 "InterpolatingCompilationDatabase.cpp",
 "JSONCompilationDatabase.cpp",
 "NodeIntrospection.cpp",
Index: clang/unittests/Tooling/CompilationDatabaseTest.cpp
===
--- clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -929,31 +929,6 @@
   EXPECT_TRUE(CCRef != CCTest);
 }
 
-class TargetAndModeTest : public MemDBTest {
-public:
-  TargetAndModeTest() { llvm::InitializeAllTargetInfos(); }
-
-protected:
-  // Look up the command from a relative path, and return it in string form.
-  std::string getCommand(llvm::StringRef F) {
-auto Results = inferTargetAndDriverMode(std::make_unique(Entries))
-   ->getCompileCommands(path(F));
-if (Results.empty())
-  return "none";
-return llvm::join(Results[0].CommandLine, " ");
-  }
-};
-
-TEST_F(TargetAndModeTest, TargetAndMode) {
-  add("foo.cpp", "clang-cl", "");
-  add("bar.cpp", "clang++", "");
-
-  EXPECT_EQ(getCommand("foo.cpp"),
-"clang-cl --driver-mode=cl foo.cpp -D foo.cpp");
-  EXPECT_EQ(getCommand("bar.cpp"),
-"clang++ --driver-mode=g++ bar.cpp -D bar.cpp");
-}
-
 class ExpandResponseFilesTest : public MemDBTest {
 public:
   ExpandResponseFilesTest() : FS(new llvm::vfs::InMemoryFileSystem) {}
Index: clang/lib/Tooling/JSONCompilationDatabase.cpp
===
--- clang/lib/Tooling/JSONCompilationDatabase.cpp
+++ clang/lib/Tooling/JSONCompilationDatabase.cpp
@@ -165,9 +165,8 @@
 llvm::sys::path::append(JSONDatabasePath, "compile_commands.json");
 auto Base = JSONCompilationDatabase::loadFromFile(
 JSONDatabasePath, ErrorMessage, JSONCommandLineSyntax::AutoDetect);
-return Base ? inferTargetAndDriverMode(
-  inferMissingCompileCommands(expandResponseFiles(
-  std::move(Base), llvm::vfs::getRealFileSystem(
+return Base ? inferMissingCompileCommands(expandResponseFiles(
+  std::move(Base), llvm::vfs::getRealFileSystem()))
 : nullptr;
   }
 };
Index: clang/lib/Tooling/GuessTargetAndModeCompilationDatabase.cpp
===
--- clang/lib/Tooling/GuessTargetAndModeCompilationDatabase.cpp
+++ /dev/null
@@ -1,57 +0,0 @@
-//===- GuessTargetAndModeCompilationDatabase.cpp --===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#include "clang/Tooling/CompilationDatabase.h"
-#include "clang/Tooling/Tooling.h"
-#include 
-
-namespace clang {
-namespace tooling {
-
-namespace {
-class TargetAndModeAdderDatabase : public CompilationDatabase {
-public:
-  TargetAndModeAdderDatabase(std::unique_ptr Base)
-  : Base(std::move(Base)) {
-assert(this->Base != nullptr);
-  }
-
-  std::vector getAllFiles() const override {
-return Base->getAllFiles();
-  }
-
-  std::vector getAllCompileCommands() const override {
-return addTargetAndMode(Base->getAllCompileCommands());
-  }
-
-  std::vector
-  getCompileCommands(StringRef FilePath) const override {
-return addTargetAndMode(Base->getCompileCommands(FilePath));
-  }
-
-private:
-  std::vector
-  addTargetAndMode(std::vector Cmds) const {
-for (auto &Cmd : Cmds) {
-  if (Cmd.CommandLine.empty())
-continue;
-  addTargetAndModeForProgramName(Cmd.CommandLine, Cmd.CommandLine.front());
-}
-return Cmds;
-  }
-  std::unique_ptr Base;
-};
-} // namespace
-
-std::unique_ptr
-inf

[PATCH] D138546: Clangd: Preserve target flags in system includes extractor

[Christopher Sauer via Phabricator via cfe-commits]

cpsauer updated this revision to Diff 489217.

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

https://reviews.llvm.org/D138546

Files:
  clang-tools-extra/clangd/CompileCommands.cpp
  clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
  clang-tools-extra/clangd/SystemIncludeExtractor.cpp
  clang-tools-extra/clangd/test/system-include-extractor.test
  clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
  clang/bindings/python/tests/cindex/test_cdb.py
  clang/docs/tools/clang-formatted-files.txt
  clang/include/clang/Tooling/CompilationDatabase.h
  clang/lib/Tooling/CMakeLists.txt
  clang/lib/Tooling/GuessTargetAndModeCompilationDatabase.cpp
  clang/lib/Tooling/JSONCompilationDatabase.cpp
  clang/unittests/Tooling/CompilationDatabaseTest.cpp
  llvm/utils/gn/secondary/clang/lib/Tooling/BUILD.gn

Index: llvm/utils/gn/secondary/clang/lib/Tooling/BUILD.gn
===
--- llvm/utils/gn/secondary/clang/lib/Tooling/BUILD.gn
+++ llvm/utils/gn/secondary/clang/lib/Tooling/BUILD.gn
@@ -37,7 +37,6 @@
 "ExpandResponseFilesCompilationDatabase.cpp",
 "FileMatchTrie.cpp",
 "FixIt.cpp",
-"GuessTargetAndModeCompilationDatabase.cpp",
 "InterpolatingCompilationDatabase.cpp",
 "JSONCompilationDatabase.cpp",
 "NodeIntrospection.cpp",
Index: clang/unittests/Tooling/CompilationDatabaseTest.cpp
===
--- clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -929,31 +929,6 @@
   EXPECT_TRUE(CCRef != CCTest);
 }
 
-class TargetAndModeTest : public MemDBTest {
-public:
-  TargetAndModeTest() { llvm::InitializeAllTargetInfos(); }
-
-protected:
-  // Look up the command from a relative path, and return it in string form.
-  std::string getCommand(llvm::StringRef F) {
-auto Results = inferTargetAndDriverMode(std::make_unique(Entries))
-   ->getCompileCommands(path(F));
-if (Results.empty())
-  return "none";
-return llvm::join(Results[0].CommandLine, " ");
-  }
-};
-
-TEST_F(TargetAndModeTest, TargetAndMode) {
-  add("foo.cpp", "clang-cl", "");
-  add("bar.cpp", "clang++", "");
-
-  EXPECT_EQ(getCommand("foo.cpp"),
-"clang-cl --driver-mode=cl foo.cpp -D foo.cpp");
-  EXPECT_EQ(getCommand("bar.cpp"),
-"clang++ --driver-mode=g++ bar.cpp -D bar.cpp");
-}
-
 class ExpandResponseFilesTest : public MemDBTest {
 public:
   ExpandResponseFilesTest() : FS(new llvm::vfs::InMemoryFileSystem) {}
Index: clang/lib/Tooling/JSONCompilationDatabase.cpp
===
--- clang/lib/Tooling/JSONCompilationDatabase.cpp
+++ clang/lib/Tooling/JSONCompilationDatabase.cpp
@@ -165,9 +165,8 @@
 llvm::sys::path::append(JSONDatabasePath, "compile_commands.json");
 auto Base = JSONCompilationDatabase::loadFromFile(
 JSONDatabasePath, ErrorMessage, JSONCommandLineSyntax::AutoDetect);
-return Base ? inferTargetAndDriverMode(
-  inferMissingCompileCommands(expandResponseFiles(
-  std::move(Base), llvm::vfs::getRealFileSystem(
+return Base ? inferMissingCompileCommands(expandResponseFiles(
+  std::move(Base), llvm::vfs::getRealFileSystem()))
 : nullptr;
   }
 };
Index: clang/lib/Tooling/GuessTargetAndModeCompilationDatabase.cpp
===
--- clang/lib/Tooling/GuessTargetAndModeCompilationDatabase.cpp
+++ /dev/null
@@ -1,57 +0,0 @@
-//===- GuessTargetAndModeCompilationDatabase.cpp --===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#include "clang/Tooling/CompilationDatabase.h"
-#include "clang/Tooling/Tooling.h"
-#include 
-
-namespace clang {
-namespace tooling {
-
-namespace {
-class TargetAndModeAdderDatabase : public CompilationDatabase {
-public:
-  TargetAndModeAdderDatabase(std::unique_ptr Base)
-  : Base(std::move(Base)) {
-assert(this->Base != nullptr);
-  }
-
-  std::vector getAllFiles() const override {
-return Base->getAllFiles();
-  }
-
-  std::vector getAllCompileCommands() const override {
-return addTargetAndMode(Base->getAllCompileCommands());
-  }
-
-  std::vector
-  getCompileCommands(StringRef FilePath) const override {
-return addTargetAndMode(Base->getCompileCommands(FilePath));
-  }
-
-private:
-  std::vector
-  addTargetAndMode(std::vector Cmds) const {
-for (auto &Cmd : Cmds) {
-  if (Cmd.CommandLine.empty())
-continue;
-  addTargetAndModeForProgramName(Cmd.CommandLine, Cmd.CommandLine.front());
-}
-return Cmds;
-  }
-  std

[PATCH] D138546: Clangd: Preserve target flags in system includes extractor

[Christopher Sauer via Phabricator via cfe-commits]

cpsauer updated this revision to Diff 489317.

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

https://reviews.llvm.org/D138546

Files:
  clang-tools-extra/clangd/CompileCommands.cpp
  clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
  clang-tools-extra/clangd/SystemIncludeExtractor.cpp
  clang-tools-extra/clangd/test/system-include-extractor.test
  clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
  clang/bindings/python/tests/cindex/test_cdb.py
  clang/docs/tools/clang-formatted-files.txt
  clang/include/clang/Tooling/CompilationDatabase.h
  clang/lib/Tooling/CMakeLists.txt
  clang/lib/Tooling/GuessTargetAndModeCompilationDatabase.cpp
  clang/lib/Tooling/JSONCompilationDatabase.cpp
  clang/unittests/Tooling/CompilationDatabaseTest.cpp
  llvm/utils/gn/secondary/clang/lib/Tooling/BUILD.gn

Index: llvm/utils/gn/secondary/clang/lib/Tooling/BUILD.gn
===
--- llvm/utils/gn/secondary/clang/lib/Tooling/BUILD.gn
+++ llvm/utils/gn/secondary/clang/lib/Tooling/BUILD.gn
@@ -37,7 +37,6 @@
 "ExpandResponseFilesCompilationDatabase.cpp",
 "FileMatchTrie.cpp",
 "FixIt.cpp",
-"GuessTargetAndModeCompilationDatabase.cpp",
 "InterpolatingCompilationDatabase.cpp",
 "JSONCompilationDatabase.cpp",
 "NodeIntrospection.cpp",
Index: clang/unittests/Tooling/CompilationDatabaseTest.cpp
===
--- clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -929,31 +929,6 @@
   EXPECT_TRUE(CCRef != CCTest);
 }
 
-class TargetAndModeTest : public MemDBTest {
-public:
-  TargetAndModeTest() { llvm::InitializeAllTargetInfos(); }
-
-protected:
-  // Look up the command from a relative path, and return it in string form.
-  std::string getCommand(llvm::StringRef F) {
-auto Results = inferTargetAndDriverMode(std::make_unique(Entries))
-   ->getCompileCommands(path(F));
-if (Results.empty())
-  return "none";
-return llvm::join(Results[0].CommandLine, " ");
-  }
-};
-
-TEST_F(TargetAndModeTest, TargetAndMode) {
-  add("foo.cpp", "clang-cl", "");
-  add("bar.cpp", "clang++", "");
-
-  EXPECT_EQ(getCommand("foo.cpp"),
-"clang-cl --driver-mode=cl foo.cpp -D foo.cpp");
-  EXPECT_EQ(getCommand("bar.cpp"),
-"clang++ --driver-mode=g++ bar.cpp -D bar.cpp");
-}
-
 class ExpandResponseFilesTest : public MemDBTest {
 public:
   ExpandResponseFilesTest() : FS(new llvm::vfs::InMemoryFileSystem) {}
Index: clang/lib/Tooling/JSONCompilationDatabase.cpp
===
--- clang/lib/Tooling/JSONCompilationDatabase.cpp
+++ clang/lib/Tooling/JSONCompilationDatabase.cpp
@@ -165,9 +165,8 @@
 llvm::sys::path::append(JSONDatabasePath, "compile_commands.json");
 auto Base = JSONCompilationDatabase::loadFromFile(
 JSONDatabasePath, ErrorMessage, JSONCommandLineSyntax::AutoDetect);
-return Base ? inferTargetAndDriverMode(
-  inferMissingCompileCommands(expandResponseFiles(
-  std::move(Base), llvm::vfs::getRealFileSystem(
+return Base ? inferMissingCompileCommands(expandResponseFiles(
+  std::move(Base), llvm::vfs::getRealFileSystem()))
 : nullptr;
   }
 };
Index: clang/lib/Tooling/GuessTargetAndModeCompilationDatabase.cpp
===
--- clang/lib/Tooling/GuessTargetAndModeCompilationDatabase.cpp
+++ /dev/null
@@ -1,57 +0,0 @@
-//===- GuessTargetAndModeCompilationDatabase.cpp --===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#include "clang/Tooling/CompilationDatabase.h"
-#include "clang/Tooling/Tooling.h"
-#include 
-
-namespace clang {
-namespace tooling {
-
-namespace {
-class TargetAndModeAdderDatabase : public CompilationDatabase {
-public:
-  TargetAndModeAdderDatabase(std::unique_ptr Base)
-  : Base(std::move(Base)) {
-assert(this->Base != nullptr);
-  }
-
-  std::vector getAllFiles() const override {
-return Base->getAllFiles();
-  }
-
-  std::vector getAllCompileCommands() const override {
-return addTargetAndMode(Base->getAllCompileCommands());
-  }
-
-  std::vector
-  getCompileCommands(StringRef FilePath) const override {
-return addTargetAndMode(Base->getCompileCommands(FilePath));
-  }
-
-private:
-  std::vector
-  addTargetAndMode(std::vector Cmds) const {
-for (auto &Cmd : Cmds) {
-  if (Cmd.CommandLine.empty())
-continue;
-  addTargetAndModeForProgramName(Cmd.CommandLine, Cmd.CommandLine.front());
-}
-return Cmds;
-  }
-  std

[PATCH] D138546: Clangd: Preserve target flags in system includes extractor

[Christopher Sauer via Phabricator via cfe-commits]

cpsauer added a comment.

(If anyone knows what's going on with the (unrelated seeming?) testing timeouts 
please do say.)


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

https://reviews.llvm.org/D138546

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

[PATCH] D138546: Clangd: Preserve target flags in system includes extractor

[Christopher Sauer via Phabricator via cfe-commits]

cpsauer added a comment.

Yay. Thanks, Nathan, for guiding, replying, and just generally being so kind, 
great, and thorough.

I'll back out the plugin part of the changes changes momentarily, then.
Just to confirm: Sounds like you're not concerned, then, that the 
JSONCompilationDatabasePlugin will get invoked and the results then passed to a 
SystemIncludeExtractor-enabled mangler?
(I'd seen some fallback calls into the plugin registry in 
GlobalCompilationDatabase, hence the original change, but, lacking broader 
context, I was having trouble tracing the expected control flow.)


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

https://reviews.llvm.org/D138546

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

[PATCH] D138546: Clangd: Preserve target flags in system includes extractor

[Christopher Sauer via Phabricator via cfe-commits]

cpsauer updated this revision to Diff 491592.

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

https://reviews.llvm.org/D138546

Files:
  clang-tools-extra/clangd/CompileCommands.cpp
  clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
  clang-tools-extra/clangd/SystemIncludeExtractor.cpp
  clang-tools-extra/clangd/test/system-include-extractor.test
  clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp

Index: clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
===
--- clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
+++ clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
@@ -49,7 +49,7 @@
   Cmd.CommandLine = {"clang++", "--", "foo.cc", "bar.cc"};
   Mangler(Cmd, "foo.cc");
   EXPECT_THAT(Cmd.CommandLine,
-  ElementsAre(testPath("fake/clang++"),
+  ElementsAre(testPath("fake/clang++"), "--driver-mode=g++",
   "-resource-dir=" + testPath("fake/resources"),
   "-isysroot", testPath("fake/sysroot"), "--",
   "foo.cc"));
@@ -198,7 +198,8 @@
   }
   // Edits are applied in given order and before other mangling and they always
   // go before filename.
-  EXPECT_THAT(Cmd.CommandLine, ElementsAre(_, "--hello", "--", "FOO.CC"));
+  EXPECT_THAT(Cmd.CommandLine,
+  ElementsAre(_, "--driver-mode=g++", "--hello", "--", "FOO.CC"));
 }
 
 static std::string strip(llvm::StringRef Arg, llvm::StringRef Argv) {
Index: clang-tools-extra/clangd/test/system-include-extractor.test
===
--- clang-tools-extra/clangd/test/system-include-extractor.test
+++ clang-tools-extra/clangd/test/system-include-extractor.test
@@ -16,6 +16,8 @@
 # RUN: echo '[ -z "${args##*"-nostdinc"*}" ] || exit' >> %t.dir/bin/my_driver.sh
 # RUN: echo '[ -z "${args##*"-isysroot=/isysroot"*}" ] || exit' >> %t.dir/bin/my_driver.sh
 # RUN: echo '[ -z "${args##*"--sysroot /my/sysroot/path"*}" ] || exit' >> %t.dir/bin/my_driver.sh
+# RUN: echo '[ -z "${args##*"-target arm-linux-gnueabi"*}" ] || exit' >> %t.dir/bin/my_driver.sh
+# RUN: echo '[ -z "${args##*"--target=arm-linux-gnueabi"*}" ] || exit' >> %t.dir/bin/my_driver.sh
 # RUN: echo 'echo line to ignore >&2' >> %t.dir/bin/my_driver.sh
 # RUN: echo 'printf "Target: arm-linux-gnueabihf\r\n" >&2' >> %t.dir/bin/my_driver.sh
 # RUN: echo 'printf "#include <...> search starts here:\r\n" >&2' >> %t.dir/bin/my_driver.sh
@@ -32,7 +34,7 @@
 
 # Generate a compile_commands.json that will query the mock driver we've
 # created. Which should add a.h and b.h into include search path.
-# RUN: echo '[{"directory": "%/t.dir", "command": "my_driver.sh the-file.cpp -nostdinc --sysroot /my/sysroot/path -isysroot=/isysroot", "file": "the-file.cpp"}]' > %t.dir/compile_commands.json
+# RUN: echo '[{"directory": "%/t.dir", "command": "my_driver.sh the-file.cpp -nostdinc --sysroot /my/sysroot/path -isysroot=/isysroot -target arm-linux-gnueabi --target=arm-linux-gnueabi", "file": "the-file.cpp"}]' > %t.dir/compile_commands.json
 
 # RUN: sed -e "s|INPUT_DIR|%/t.dir|g" %s > %t.test.1
 # On Windows, we need the URI in didOpen to look like "uri":"file:///C:/..."
@@ -70,7 +72,7 @@
 {"jsonrpc":"2.0","method":"exit"}
 
 # Generate a different compile_commands.json which does not point to the mock driver
-# RUN: echo '[{"directory": "%/t.dir", "command": "gcc the-file.cpp -nostdinc --sysroot /my/sysroot/path -isysroot=/isysroot", "file": "the-file.cpp"}]' > %t.dir/compile_commands.json
+# RUN: echo '[{"directory": "%/t.dir", "command": "gcc the-file.cpp -nostdinc --sysroot /my/sysroot/path -isysroot=/isysroot -target arm-linux-gnueabi --target=arm-linux-gnueabi", "file": "the-file.cpp"}]' > %t.dir/compile_commands.json
 
 # Generate a clangd config file which points to the mock driver instead
 # RUN: echo 'CompileFlags:' > %t.dir/.clangd
Index: clang-tools-extra/clangd/SystemIncludeExtractor.cpp
===
--- clang-tools-extra/clangd/SystemIncludeExtractor.cpp
+++ clang-tools-extra/clangd/SystemIncludeExtractor.cpp
@@ -184,13 +184,18 @@
   const llvm::StringRef FlagsToPreserve[] = {
   "-nostdinc", "--no-standard-includes", "-nostdinc++", "-nobuiltininc"};
   // Preserves these flags and their values, either as separate args or with an
-  // equalsbetween them
+  // equals between them
   const llvm::StringRef ArgsToPreserve[] = {"--sysroot", "-isysroot"};
 
   for (size_t I = 0, E = CommandLine.size(); I < E; ++I) {
 llvm::StringRef Arg = CommandLine[I];
 if (llvm::is_contained(FlagsToPreserve, Arg)) {
   Args.push_back(Arg);
+} else if (Arg.startswith("--target=")) {
+  Args.push_back(Arg);
+} else if (I + 1 < E && Arg.equals("-target")) {
+  Args.push_back(CommandLine[I]);
+  Args.push_back(CommandLine[++I]);
 } else {
   const auto *Found =
   

[PATCH] D138546: Clangd: Preserve target flags in system includes extractor

[Christopher Sauer via Phabricator via cfe-commits]

cpsauer updated this revision to Diff 491593.
cpsauer added a comment.

Rebased


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

https://reviews.llvm.org/D138546

Files:
  clang-tools-extra/clangd/CompileCommands.cpp
  clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
  clang-tools-extra/clangd/SystemIncludeExtractor.cpp
  clang-tools-extra/clangd/test/system-include-extractor.test
  clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp

Index: clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
===
--- clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
+++ clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
@@ -49,7 +49,7 @@
   Cmd.CommandLine = {"clang++", "--", "foo.cc", "bar.cc"};
   Mangler(Cmd, "foo.cc");
   EXPECT_THAT(Cmd.CommandLine,
-  ElementsAre(testPath("fake/clang++"),
+  ElementsAre(testPath("fake/clang++"), "--driver-mode=g++",
   "-resource-dir=" + testPath("fake/resources"),
   "-isysroot", testPath("fake/sysroot"), "--",
   "foo.cc"));
@@ -198,7 +198,8 @@
   }
   // Edits are applied in given order and before other mangling and they always
   // go before filename.
-  EXPECT_THAT(Cmd.CommandLine, ElementsAre(_, "--hello", "--", "FOO.CC"));
+  EXPECT_THAT(Cmd.CommandLine,
+  ElementsAre(_, "--driver-mode=g++", "--hello", "--", "FOO.CC"));
 }
 
 static std::string strip(llvm::StringRef Arg, llvm::StringRef Argv) {
Index: clang-tools-extra/clangd/test/system-include-extractor.test
===
--- clang-tools-extra/clangd/test/system-include-extractor.test
+++ clang-tools-extra/clangd/test/system-include-extractor.test
@@ -16,6 +16,8 @@
 # RUN: echo '[ -z "${args##*"-nostdinc"*}" ] || exit' >> %t.dir/bin/my_driver.sh
 # RUN: echo '[ -z "${args##*"-isysroot=/isysroot"*}" ] || exit' >> %t.dir/bin/my_driver.sh
 # RUN: echo '[ -z "${args##*"--sysroot /my/sysroot/path"*}" ] || exit' >> %t.dir/bin/my_driver.sh
+# RUN: echo '[ -z "${args##*"-target arm-linux-gnueabi"*}" ] || exit' >> %t.dir/bin/my_driver.sh
+# RUN: echo '[ -z "${args##*"--target=arm-linux-gnueabi"*}" ] || exit' >> %t.dir/bin/my_driver.sh
 # RUN: echo 'echo line to ignore >&2' >> %t.dir/bin/my_driver.sh
 # RUN: echo 'printf "Target: arm-linux-gnueabihf\r\n" >&2' >> %t.dir/bin/my_driver.sh
 # RUN: echo 'printf "#include <...> search starts here:\r\n" >&2' >> %t.dir/bin/my_driver.sh
@@ -32,7 +34,7 @@
 
 # Generate a compile_commands.json that will query the mock driver we've
 # created. Which should add a.h and b.h into include search path.
-# RUN: echo '[{"directory": "%/t.dir", "command": "my_driver.sh the-file.cpp -nostdinc --sysroot /my/sysroot/path -isysroot=/isysroot", "file": "the-file.cpp"}]' > %t.dir/compile_commands.json
+# RUN: echo '[{"directory": "%/t.dir", "command": "my_driver.sh the-file.cpp -nostdinc --sysroot /my/sysroot/path -isysroot=/isysroot -target arm-linux-gnueabi --target=arm-linux-gnueabi", "file": "the-file.cpp"}]' > %t.dir/compile_commands.json
 
 # RUN: sed -e "s|INPUT_DIR|%/t.dir|g" %s > %t.test.1
 # On Windows, we need the URI in didOpen to look like "uri":"file:///C:/..."
@@ -70,7 +72,7 @@
 {"jsonrpc":"2.0","method":"exit"}
 
 # Generate a different compile_commands.json which does not point to the mock driver
-# RUN: echo '[{"directory": "%/t.dir", "command": "gcc the-file.cpp -nostdinc --sysroot /my/sysroot/path -isysroot=/isysroot", "file": "the-file.cpp"}]' > %t.dir/compile_commands.json
+# RUN: echo '[{"directory": "%/t.dir", "command": "gcc the-file.cpp -nostdinc --sysroot /my/sysroot/path -isysroot=/isysroot -target arm-linux-gnueabi --target=arm-linux-gnueabi", "file": "the-file.cpp"}]' > %t.dir/compile_commands.json
 
 # Generate a clangd config file which points to the mock driver instead
 # RUN: echo 'CompileFlags:' > %t.dir/.clangd
Index: clang-tools-extra/clangd/SystemIncludeExtractor.cpp
===
--- clang-tools-extra/clangd/SystemIncludeExtractor.cpp
+++ clang-tools-extra/clangd/SystemIncludeExtractor.cpp
@@ -185,13 +185,18 @@
   const llvm::StringRef FlagsToPreserve[] = {
   "-nostdinc", "--no-standard-includes", "-nostdinc++", "-nobuiltininc"};
   // Preserves these flags and their values, either as separate args or with an
-  // equalsbetween them
+  // equals between them
   const llvm::StringRef ArgsToPreserve[] = {"--sysroot", "-isysroot"};
 
   for (size_t I = 0, E = CommandLine.size(); I < E; ++I) {
 llvm::StringRef Arg = CommandLine[I];
 if (llvm::is_contained(FlagsToPreserve, Arg)) {
   Args.push_back(Arg);
+} else if (Arg.startswith("--target=")) {
+  Args.push_back(Arg);
+} else if (I + 1 < E && Arg.equals("-target")) {
+  Args.push_back(CommandLine[I]);
+  Args.push_back(CommandLine[++I]);
 } else {

[PATCH] D138546: Clangd: Preserve target flags in system includes extractor

[Christopher Sauer via Phabricator via cfe-commits]

cpsauer created this revision.
cpsauer added reviewers: nridge, sammccall, kadircet.
Herald added a subscriber: arphaman.
Herald added a project: All.
cpsauer requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

This preserves --target and -target flags in the system includes extractor, 
which is needed for cross-compiling to, e.g. Android, since the target flags 
can influence the default include paths.

Note that, like isysroot (which is handled incorrectly above and elsewhere), 
the target flag doesn't fit cleanly into the ArgsToPreserve abstraction and 
does indeed have a different number of - in its = and non = forms. (ref 
)
 There are plenty of bugs in this file, but this is an incremental improvement.

For more context, please see https://github.com/clangd/clangd/issues/1378

Thanks for all you do, wonderful LLVM folks :)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138546

Files:
  clang-tools-extra/clangd/SystemIncludeExtractor.cpp


Index: clang-tools-extra/clangd/SystemIncludeExtractor.cpp
===
--- clang-tools-extra/clangd/SystemIncludeExtractor.cpp
+++ clang-tools-extra/clangd/SystemIncludeExtractor.cpp
@@ -184,13 +184,18 @@
   const llvm::StringRef FlagsToPreserve[] = {
   "-nostdinc", "--no-standard-includes", "-nostdinc++", "-nobuiltininc"};
   // Preserves these flags and their values, either as separate args or with an
-  // equalsbetween them
+  // equals between them
   const llvm::StringRef ArgsToPreserve[] = {"--sysroot", "-isysroot"};
 
   for (size_t I = 0, E = CommandLine.size(); I < E; ++I) {
 llvm::StringRef Arg = CommandLine[I];
 if (llvm::is_contained(FlagsToPreserve, Arg)) {
   Args.push_back(Arg);
+} else if (Arg.startswith("--target=")) {
+  Args.push_back(Arg);
+} else if (I + 1 < E && Arg.equals("-target")) {
+  Args.push_back(CommandLine[I]);
+  Args.push_back(CommandLine[++I]);
 } else {
   const auto *Found =
   llvm::find_if(ArgsToPreserve, [&Arg](llvm::StringRef S) {


Index: clang-tools-extra/clangd/SystemIncludeExtractor.cpp
===
--- clang-tools-extra/clangd/SystemIncludeExtractor.cpp
+++ clang-tools-extra/clangd/SystemIncludeExtractor.cpp
@@ -184,13 +184,18 @@
   const llvm::StringRef FlagsToPreserve[] = {
   "-nostdinc", "--no-standard-includes", "-nostdinc++", "-nobuiltininc"};
   // Preserves these flags and their values, either as separate args or with an
-  // equalsbetween them
+  // equals between them
   const llvm::StringRef ArgsToPreserve[] = {"--sysroot", "-isysroot"};
 
   for (size_t I = 0, E = CommandLine.size(); I < E; ++I) {
 llvm::StringRef Arg = CommandLine[I];
 if (llvm::is_contained(FlagsToPreserve, Arg)) {
   Args.push_back(Arg);
+} else if (Arg.startswith("--target=")) {
+  Args.push_back(Arg);
+} else if (I + 1 < E && Arg.equals("-target")) {
+  Args.push_back(CommandLine[I]);
+  Args.push_back(CommandLine[++I]);
 } else {
   const auto *Found =
   llvm::find_if(ArgsToPreserve, [&Arg](llvm::StringRef S) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D138546: Clangd: Preserve target flags in system includes extractor

[Christopher Sauer via Phabricator via cfe-commits]

cpsauer added a comment.

Sam, my read, too, is that the memoizing design isn't safe--also that the key 
bug is preexisting.

I had the same reaction reading through this file after spotting problems in 
the log. That's what spawned https://github.com/clangd/clangd/issues/1378.

Any chance I could get you to quickly read through that issue if you haven't 
already? (The relevant section to this part: "If we think sysroots, targets, 
and the other flags enumerated effect the system includes, we'd better include 
them as part of the memoization key.")

There are sadly *lots* of problems in this file that leap out on a quick read. 
Nathan and I were thinking, though, that we'd should post this incremental fix 
for review rather than getting bogged down in trying to fix multiple things 
atomically.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138546

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

[PATCH] D138546: Clangd: Preserve target flags in system includes extractor

[Christopher Sauer via Phabricator via cfe-commits]

cpsauer added a comment.

Makes sense! Thanks for your time, Sam, and for being great, as always.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138546

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

[PATCH] D138546: Clangd: Preserve target flags in system includes extractor

[Christopher Sauer via Phabricator via cfe-commits]

cpsauer added a comment.

I suppose, if it ever might help make the case with said employer (Google, 
right?) the broken, non-stock-clang use case that's motivating this is Android 
(and also usage from Bazel/Blaze).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138546

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

[PATCH] D138546: Clangd: Preserve target flags in system includes extractor

[Christopher Sauer via Phabricator via cfe-commits]

cpsauer added a comment.

@kadircet, could I get your review on this one?


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

https://reviews.llvm.org/D138546

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

[PATCH] D138546: Clangd: Preserve target flags in system includes extractor

[Christopher Sauer via Phabricator via cfe-commits]

cpsauer added a comment.

@kadircet, friendly check in, since I got reminded of this: How would you like 
to proceed from here?


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

https://reviews.llvm.org/D138546

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

[PATCH] D138546: Clangd: Preserve target flags in system includes extractor

[Christopher Sauer via Phabricator via cfe-commits]

cpsauer added a comment.

Sorry for being so slow this time myself, guys. Took me a while to dig myself 
out of a deep inbox hole after Thanksgiving (US holiday). Really appreciate you 
all and your friendly responsiveness, especially as I get up to speed here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138546

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

[PATCH] D138546: Clangd: Preserve target flags in system includes extractor

[Christopher Sauer via Phabricator via cfe-commits]

cpsauer updated this revision to Diff 481141.

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

https://reviews.llvm.org/D138546

Files:
  clang-tools-extra/clangd/SystemIncludeExtractor.cpp
  clang-tools-extra/clangd/test/system-include-extractor.test


Index: clang-tools-extra/clangd/test/system-include-extractor.test
===
--- clang-tools-extra/clangd/test/system-include-extractor.test
+++ clang-tools-extra/clangd/test/system-include-extractor.test
@@ -16,6 +16,8 @@
 # RUN: echo '[ -z "${args##*"-nostdinc"*}" ] || exit' >> 
%t.dir/bin/my_driver.sh
 # RUN: echo '[ -z "${args##*"-isysroot=/isysroot"*}" ] || exit' >> 
%t.dir/bin/my_driver.sh
 # RUN: echo '[ -z "${args##*"--sysroot /my/sysroot/path"*}" ] || exit' >> 
%t.dir/bin/my_driver.sh
+# RUN: echo '[ -z "${args##*"-target arm-linux-gnueabi"*}" ] || exit' >> 
%t.dir/bin/my_driver.sh
+# RUN: echo '[ -z "${args##*"--target=arm-linux-gnueabi"*}" ] || exit' >> 
%t.dir/bin/my_driver.sh
 # RUN: echo 'echo line to ignore >&2' >> %t.dir/bin/my_driver.sh
 # RUN: echo 'printf "Target: arm-linux-gnueabihf\r\n" >&2' >> 
%t.dir/bin/my_driver.sh
 # RUN: echo 'printf "#include <...> search starts here:\r\n" >&2' >> 
%t.dir/bin/my_driver.sh
@@ -32,7 +34,7 @@
 
 # Generate a compile_commands.json that will query the mock driver we've
 # created. Which should add a.h and b.h into include search path.
-# RUN: echo '[{"directory": "%/t.dir", "command": "my_driver.sh the-file.cpp 
-nostdinc --sysroot /my/sysroot/path -isysroot=/isysroot", "file": 
"the-file.cpp"}]' > %t.dir/compile_commands.json
+# RUN: echo '[{"directory": "%/t.dir", "command": "my_driver.sh the-file.cpp 
-nostdinc --sysroot /my/sysroot/path -isysroot=/isysroot -target 
arm-linux-gnueabi --target=arm-linux-gnueabi", "file": "the-file.cpp"}]' > 
%t.dir/compile_commands.json
 
 # RUN: sed -e "s|INPUT_DIR|%/t.dir|g" %s > %t.test.1
 # On Windows, we need the URI in didOpen to look like "uri":"file:///C:/..."
@@ -70,7 +72,7 @@
 {"jsonrpc":"2.0","method":"exit"}
 
 # Generate a different compile_commands.json which does not point to the mock 
driver
-# RUN: echo '[{"directory": "%/t.dir", "command": "gcc the-file.cpp -nostdinc 
--sysroot /my/sysroot/path -isysroot=/isysroot", "file": "the-file.cpp"}]' > 
%t.dir/compile_commands.json
+# RUN: echo '[{"directory": "%/t.dir", "command": "gcc the-file.cpp -nostdinc 
--sysroot /my/sysroot/path -isysroot=/isysroot -target arm-linux-gnueabi 
--target=arm-linux-gnueabi", "file": "the-file.cpp"}]' > 
%t.dir/compile_commands.json
 
 # Generate a clangd config file which points to the mock driver instead
 # RUN: echo 'CompileFlags:' > %t.dir/.clangd
Index: clang-tools-extra/clangd/SystemIncludeExtractor.cpp
===
--- clang-tools-extra/clangd/SystemIncludeExtractor.cpp
+++ clang-tools-extra/clangd/SystemIncludeExtractor.cpp
@@ -184,13 +184,18 @@
   const llvm::StringRef FlagsToPreserve[] = {
   "-nostdinc", "--no-standard-includes", "-nostdinc++", "-nobuiltininc"};
   // Preserves these flags and their values, either as separate args or with an
-  // equalsbetween them
+  // equals between them
   const llvm::StringRef ArgsToPreserve[] = {"--sysroot", "-isysroot"};
 
   for (size_t I = 0, E = CommandLine.size(); I < E; ++I) {
 llvm::StringRef Arg = CommandLine[I];
 if (llvm::is_contained(FlagsToPreserve, Arg)) {
   Args.push_back(Arg);
+} else if (Arg.startswith("--target=")) {
+  Args.push_back(Arg);
+} else if (I + 1 < E && Arg.equals("-target")) {
+  Args.push_back(CommandLine[I]);
+  Args.push_back(CommandLine[++I]);
 } else {
   const auto *Found =
   llvm::find_if(ArgsToPreserve, [&Arg](llvm::StringRef S) {


Index: clang-tools-extra/clangd/test/system-include-extractor.test
===
--- clang-tools-extra/clangd/test/system-include-extractor.test
+++ clang-tools-extra/clangd/test/system-include-extractor.test
@@ -16,6 +16,8 @@
 # RUN: echo '[ -z "${args##*"-nostdinc"*}" ] || exit' >> %t.dir/bin/my_driver.sh
 # RUN: echo '[ -z "${args##*"-isysroot=/isysroot"*}" ] || exit' >> %t.dir/bin/my_driver.sh
 # RUN: echo '[ -z "${args##*"--sysroot /my/sysroot/path"*}" ] || exit' >> %t.dir/bin/my_driver.sh
+# RUN: echo '[ -z "${args##*"-target arm-linux-gnueabi"*}" ] || exit' >> %t.dir/bin/my_driver.sh
+# RUN: echo '[ -z "${args##*"--target=arm-linux-gnueabi"*}" ] || exit' >> %t.dir/bin/my_driver.sh
 # RUN: echo 'echo line to ignore >&2' >> %t.dir/bin/my_driver.sh
 # RUN: echo 'printf "Target: arm-linux-gnueabihf\r\n" >&2' >> %t.dir/bin/my_driver.sh
 # RUN: echo 'printf "#include <...> search starts here:\r\n" >&2' >> %t.dir/bin/my_driver.sh
@@ -32,7 +34,7 @@
 
 # Generate a compile_commands.json that will query the mock driver we've
 # created. Which should add a.h and b.h into include search path.
-# RUN: echo '[{"directory": "%/

[PATCH] D138546: Clangd: Preserve target flags in system includes extractor

[Christopher Sauer via Phabricator via cfe-commits]

cpsauer added a comment.

@nridge, I took a shot at amending the test. Thanks for the pointer! Please me 
know if that looks good to you!
The host-target cross-compile situation you hypothesize is exactly what caused 
me to notice this. Super common with Bazel.


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

https://reviews.llvm.org/D138546

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

[PATCH] D138546: Clangd: Preserve target flags in system includes extractor

[Christopher Sauer via Phabricator via cfe-commits]

cpsauer added a comment.

@kadircet, you were asking about what's requiring the use of query-driver--I'll 
reply over in the issue (as you suggest), keeping everything in one place.


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

https://reviews.llvm.org/D138546

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

[PATCH] D138546: Clangd: Preserve target flags in system includes extractor

[Christopher Sauer via Phabricator via cfe-commits]

cpsauer added a comment.

Sweet. Thanks again, Nathan.

Guys, lmk how you'd like to go from here. If you approve; feel free to copy 
in/land as part of the other change if that would save time. 
(I don't have commit access anyway.)


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

https://reviews.llvm.org/D138546

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