Parse .h files as objective-c++ if we don't have a compile command.

[sammccall]

Summary

This makes C++/objC not totally broken, without hurting C files too much.

[ilya-biryukov]

ObjC++ definitely seems like a nicer default. Unfortunately, we'll start getting ObjC++ completion results, which may be confusing. But that seems like a smaller evil.

Is there an easy way to add a regression test that checks we don't get any errors for headers without compile command with C++/ObjC++ code?

clangd/GlobalCompilationDatabase.cpp

24

Maybe use .equals_lower(".h") instead? Just in case.

[sammccall]

Will try to add a test.

clangd/GlobalCompilationDatabase.cpp

24

.H is (unambiguously) c++, and will be treated as such by clang.

[ilya-biryukov]

clangd/GlobalCompilationDatabase.cpp

24

Wasn't aware of that convention, thanks.
In that case, LG

[sammccall]

Add test, shave required yaks

[sammccall]

Added a unit test, and cleaned up a couple of things to make it pass:

• GlobalCompilationDatabase.h wasn't self-contained due to incomplete types
• testPath() isn't very usable with subdirectories on windows

clangd/GlobalCompilationDatabase.cpp

24

Yeah .C/.H is rare these days I think. (But present in Driver/Types.cpp)

[sammccall]

@ilya-biryukov Ping :-)

[ilya-biryukov]

LGTM

clangd/GlobalCompilationDatabase.h

 
 /// Gets compile args from tooling::CompilationDatabases built for parent
 /// directories.
 class DirectoryBasedGlobalCompilationDatabase
     : public GlobalCompilationDatabase {
 public:
   DirectoryBasedGlobalCompilationDatabase(
       llvm::Optional<Path> CompileCommandsDir);
+  ~DirectoryBasedGlobalCompilationDatabase() override;
 
   /// Scans File's parents looking for compilation databases.
   /// Any extra flags will be added.
   llvm::Optional<tooling::CompileCommand>
   getCompileCommand(PathRef File) const override;
 
   /// Uses the default fallback command, adding any extra flags.
   tooling::CompileCommand getFallbackCommand(PathRef File) const override;

clangd/GlobalCompilationDatabase.cpp

 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 
 namespace clang {
 namespace clangd {
 
 tooling::CompileCommand
 GlobalCompilationDatabase::getFallbackCommand(PathRef File) const {
+  std::vector<std::string> Argv = {"clang"};
+  // Clang treats .h files as C by default, resulting in unhelpful diagnostics.
+  // Parsing as Objective C++ is friendly to more cases.
+  if (llvm::sys::path::extension(File) == ".h")
+    Argv.push_back("-xobjective-c++-header");
+  Argv.push_back(File);
   return tooling::CompileCommand(llvm::sys::path::parent_path(File),
                                  llvm::sys::path::filename(File),
-                                 {"clang", File.str()},
+                                 std::move(Argv),
                                  /*Output=*/"");
 }
 
 DirectoryBasedGlobalCompilationDatabase::
     DirectoryBasedGlobalCompilationDatabase(
         llvm::Optional<Path> CompileCommandsDir)
     : CompileCommandsDir(std::move(CompileCommandsDir)) {}
 
+DirectoryBasedGlobalCompilationDatabase::
+    ~DirectoryBasedGlobalCompilationDatabase() = default;
+
 llvm::Optional<tooling::CompileCommand>
 DirectoryBasedGlobalCompilationDatabase::getCompileCommand(PathRef File) const {
   if (auto CDB = getCDBForFile(File)) {
     auto Candidates = CDB->getCompileCommands(File);
     if (!Candidates.empty()) {
       addExtraFlags(File, Candidates.front());
       return std::move(Candidates.front());
     }

unittests/clangd/CMakeLists.txt

add_extra_unittest(ClangdTests
   ClangdTests.cpp
   ClangdUnitTests.cpp
   CodeCompleteTests.cpp
   CodeCompletionStringsTests.cpp
   ContextTests.cpp
   DraftStoreTests.cpp
   FileIndexTests.cpp
   FuzzyMatchTests.cpp
+  GlobalCompilationDatabaseTests.cpp
   HeadersTests.cpp
   IndexTests.cpp
   JSONExprTests.cpp
   SourceCodeTests.cpp
   SymbolCollectorTests.cpp
   SyncAPI.cpp
   TestFS.cpp
   ThreadingTests.cpp

unittests/clangd/GlobalCompilationDatabaseTests.cpp

+//===-- GlobalCompilationDatabaseTests.cpp ----------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "GlobalCompilationDatabase.h"
+
+#include "TestFS.h"
+#include "llvm/ADT/StringExtras.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+using ::testing::ElementsAre;
+
+TEST(GlobalCompilationDatabaseTest, FallbackCommand) {
+  DirectoryBasedGlobalCompilationDatabase DB(llvm::None);
+  auto Cmd = DB.getFallbackCommand(testPath("foo/bar.cc"));
+  EXPECT_EQ(Cmd.Directory, testPath("foo"));
+  EXPECT_THAT(Cmd.CommandLine, ElementsAre("clang", testPath("foo/bar.cc")));
+  EXPECT_EQ(Cmd.Output, "");
+
+  // .h files have unknown language, so they are parsed liberally as obj-c++.
+  Cmd = DB.getFallbackCommand(testPath("foo/bar.h"));
+  EXPECT_THAT(Cmd.CommandLine, ElementsAre("clang", "-xobjective-c++-header",
+                                           testPath("foo/bar.h")));
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang

unittests/clangd/TestFS.cpp

 #else
   return "/clangd-test";
 #endif
 }
 
 std::string testPath(PathRef File) {
   assert(sys::path::is_relative(File) && "FileName should be relative");
 
+  SmallString<32> NativeFile = File;
+  sys::path::native(NativeFile);
   SmallString<32> Path;
-  sys::path::append(Path, testRoot(), File);
+  sys::path::append(Path, testRoot(), NativeFile);
   return Path.str();
 }
 
 } // namespace clangd
 } // namespace clang