[clangd] Config: CompileFlags.Remove

[sammccall]

Summary

While here, add documentation to CompileFlags and CompileFlags.Add.

[hokein]

clang-tools-extra/clangd/ConfigCompile.cpp

126

the order of processing "Remove" and "Add" matters, if they are conflict, the Add will take precedence, I think this is intended? Maybe worth clarifying in the CompileFlagsBlock comments.

clang-tools-extra/clangd/ConfigFragment.h

136

nit: wil => will

141

nit: I'd prefer to give a few concrete examples (e.g. examples I mentioned in another patch), it would be helpful for readers to understand.

clang-tools-extra/clangd/CompileCommands.h

 //
 // Internally this builds a large (0.5MB) table of clang options on first use.
 // Both strip() and process() are fairly cheap after that.
 //
 // FIXME: this reimplements much of OptTable, it might be nice to expose more.
 // The table-building strategy may not make sense outside clangd.
 class ArgStripper {
 public:
+  ArgStripper() = default;
+  ArgStripper(ArgStripper &&) = default;
+  ArgStripper(const ArgStripper &) = delete;
+  ArgStripper &operator=(ArgStripper &&) = default;
+  ArgStripper &operator=(const ArgStripper &) = delete;
+
   // Adds the arg to the set which should be removed.
   //
   // Recognized clang flags are stripped semantically. When "-I" is stripped:
   //  - so is its value (either as -Ifoo or -I foo)
   //  - aliases like --include-directory=foo are also stripped
   //  - CL-style /Ifoo will be removed if the args indicate MS-compatible mode
   // Compile args not recognized as flags are removed literally, except:
   //  - strip("ABC*") will remove any arg with an ABC prefix.

clang-tools-extra/clangd/ConfigCompile.cpp

 // do as much work as possible in the first step. For example, regexes are
 // compiled in stage 1 and captured by the apply function. This is because:
 //
 //  - it's more efficient, as the work done in stage 1 must only be done once
 //  - problems can be reported in stage 1, in stage 2 we must silently recover
 //
 //===----------------------------------------------------------------------===//
 
+#include "CompileCommands.h"
 #include "Config.h"
 #include "ConfigFragment.h"
 #include "support/Logger.h"
 #include "support/Trace.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Regex.h"
 #include "llvm/Support/SMLoc.h"
 #include "llvm/Support/SourceMgr.h"
if (!PathExclude->empty()) {
             return llvm::none_of(*PathExclude, [&](const llvm::Regex &RE) {
               return RE.match(P.Path);
             });
           });
     }
   }
 
   void compile(Fragment::CompileFlagsBlock &&F) {
+    if (!F.Remove.empty()) {
+      auto Remove = std::make_shared<ArgStripper>();
+      for (auto &A : F.Remove)
+        Remove->strip(*A);
+      Out.Apply.push_back([Remove(std::shared_ptr<const ArgStripper>(
+                              std::move(Remove)))](Config &C) {
+        C.CompileFlags.Edits.push_back(
+            [Remove](std::vector<std::string> &Args) {
+              Remove->process(Args);
+            });
+      });
+    }
+
     if (!F.Add.empty()) {
       std::vector<std::string> Add;
       for (auto &A : F.Add)
         Add.push_back(std::move(*A));
       Out.Apply.push_back([Add(std::move(Add))](Config &C) {
         C.CompileFlags.Edits.push_back([Add](std::vector<std::string> &Args) {
           Args.insert(Args.end(), Add.begin(), Add.end());
         });

clang-tools-extra/clangd/ConfigFragment.h

struct IfBlock {
     std::vector<Located<std::string>> PathExclude;
 
     /// An unrecognized key was found while parsing the condition.
     /// The condition will evaluate to false.
     bool HasUnrecognizedCondition = false;
   };
   IfBlock If;
 
+  /// Conditions in the CompileFlags block affect how a file is parsed.
+  ///
+  /// clangd emulates how clang would interpret a file.
+  /// By default, it behaves roughly like `clang $FILENAME`, but real projects
+  /// usually require setting the include path (with the `-I` flag), defining
+  /// preprocessor symbols, configuring warnings etc.
+  /// Often, a compilation database specifies these compile commands. clangd
+  /// searches for compile_commands.json in parents of the source file.
+  ///
+  /// This section modifies how the compile command is constructed.
   struct CompileFlagsBlock {
+    /// List of flags to append to the compile command.
     std::vector<Located<std::string>> Add;
-  } CompileFlags;
+    /// List of flags to remove from the compile command.
+    ///
+    /// - If the value is a recognized clang flag (like "-I") then it will be
+    ///   removed along with any arguments. Synonyms like --include-dir= will
+    ///   also be removed.
+    /// - Otherwise, if the value ends in * (like "-DFOO=*") then any argument
+    ///   with the prefix will be removed.
+    /// - Otherwise any argument exactly matching the value is removed.
+    ///
+    /// In all cases, -Xclang is also removed where needed.
+    ///
+    /// Example:
+    ///   Command: clang++ --include-directory=/usr/include -DFOO=42 foo.cc
+    ///   Remove: [-I, -DFOO=*]
+    ///   Result: clang++ foo.cc
+    ///
+    /// Flags added by the same CompileFlags entry will not be removed.
+    std::vector<Located<std::string>> Remove;
+  };
+  CompileFlagsBlock CompileFlags;
 };
 
 } // namespace config
 } // namespace clangd
 } // namespace clang
 
 #endif

clang-tools-extra/clangd/ConfigYAML.cpp

private:
   }
 
   void parse(Fragment::CompileFlagsBlock &F, Node &N) {
     DictParser Dict("CompileFlags", this);
     Dict.handle("Add", [&](Node &N) {
       if (auto Values = scalarValues(N))
         F.Add = std::move(*Values);
     });
+    Dict.handle("Remove", [&](Node &N) {
+      if (auto Values = scalarValues(N))
+        F.Remove = std::move(*Values);
+    });
     Dict.parse(N);
   }
 
   // Helper for parsing mapping nodes (dictionaries).
   // We don't use YamlIO as we want to control over unknown keys.
   class DictParser {
     llvm::StringRef Description;
     std::vector<std::pair<llvm::StringRef, std::function<void(Node &)>>> Keys;

clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp

TEST_F(ConfigCompileTests, Condition) {
   Frag.If.HasUnrecognizedCondition = true;
   Frag.If.PathMatch.emplace_back("ba*r");
   EXPECT_FALSE(compileAndApply());
   EXPECT_THAT(Diags.Diagnostics, IsEmpty());
 }
 
 TEST_F(ConfigCompileTests, CompileCommands) {
   Frag.CompileFlags.Add.emplace_back("-foo");
-  std::vector<std::string> Argv = {"clang", "a.cc"};
+  Frag.CompileFlags.Remove.emplace_back("--include-directory=");
+  std::vector<std::string> Argv = {"clang", "-I", "bar/", "a.cc"};
   EXPECT_TRUE(compileAndApply());
-  EXPECT_THAT(Conf.CompileFlags.Edits, SizeIs(1));
-  Conf.CompileFlags.Edits.front()(Argv);
+  EXPECT_THAT(Conf.CompileFlags.Edits, SizeIs(2));
+  for (auto &Edit : Conf.CompileFlags.Edits)
+    Edit(Argv);
   EXPECT_THAT(Argv, ElementsAre("clang", "a.cc", "-foo"));
 }
 
 } // namespace
 } // namespace config
 } // namespace clangd
 } // namespace clang