[PATCH] D45442: Parse .h files as objective-c++ if we don't have a compile command.
This revision was automatically updated to reflect the committed changes. Closed by commit rL330418: Parse .h files as objective-c++ if we don't have a compile command. (authored by sammccall, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D45442 Files: clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.h clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt clang-tools-extra/trunk/unittests/clangd/GlobalCompilationDatabaseTests.cpp clang-tools-extra/trunk/unittests/clangd/TestFS.cpp Index: clang-tools-extra/trunk/unittests/clangd/GlobalCompilationDatabaseTests.cpp === --- clang-tools-extra/trunk/unittests/clangd/GlobalCompilationDatabaseTests.cpp +++ clang-tools-extra/trunk/unittests/clangd/GlobalCompilationDatabaseTests.cpp @@ -0,0 +1,37 @@ +//===-- 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 Index: clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt === --- clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt +++ clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt @@ -18,6 +18,7 @@ DraftStoreTests.cpp FileIndexTests.cpp FuzzyMatchTests.cpp + GlobalCompilationDatabaseTests.cpp HeadersTests.cpp IndexTests.cpp JSONExprTests.cpp Index: clang-tools-extra/trunk/unittests/clangd/TestFS.cpp === --- clang-tools-extra/trunk/unittests/clangd/TestFS.cpp +++ clang-tools-extra/trunk/unittests/clangd/TestFS.cpp @@ -55,8 +55,10 @@ 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(); } Index: clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.h === --- clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.h +++ clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.h @@ -52,6 +52,7 @@ public: DirectoryBasedGlobalCompilationDatabase( llvm::Optional CompileCommandsDir); + ~DirectoryBasedGlobalCompilationDatabase() override; /// Scans File's parents looking for compilation databases. /// Any extra flags will be added. Index: clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp === --- clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp +++ clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp @@ -18,17 +18,26 @@ tooling::CompileCommand GlobalCompilationDatabase::getFallbackCommand(PathRef File) const { + std::vector 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 CompileCommandsDir) : CompileCommandsDir(std::move(CompileCommandsDir)) {} +DirectoryBasedGlobalCompilationDatabase
[PATCH] D45442: Parse .h files as objective-c++ if we don't have a compile command.
ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. Herald added a subscriber: jkorous. LGTM Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45442 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45442: Parse .h files as objective-c++ if we don't have a compile command.
sammccall added a comment. @ilya-biryukov Ping :-) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45442 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45442: Parse .h files as objective-c++ if we don't have a compile command.
sammccall added a comment. 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 Comment at: clangd/GlobalCompilationDatabase.cpp:24 + // Parsing as Objective C++ is friendly to more cases. + if (llvm::sys::path::extension(File) == ".h") +Argv.push_back("-xobjective-c++-header"); ilya-biryukov wrote: > sammccall wrote: > > ilya-biryukov wrote: > > > Maybe use `.equals_lower(".h")` instead? Just in case. > > .H is (unambiguously) c++, and will be treated as such by clang. > Wasn't aware of that convention, thanks. > In that case, LG Yeah .C/.H is rare these days I think. (But present in Driver/Types.cpp) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45442 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45442: Parse .h files as objective-c++ if we don't have a compile command.
sammccall updated this revision to Diff 141957. sammccall added a comment. Herald added a subscriber: mgorny. Add test, shave required yaks Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45442 Files: clangd/GlobalCompilationDatabase.cpp clangd/GlobalCompilationDatabase.h unittests/clangd/CMakeLists.txt unittests/clangd/GlobalCompilationDatabaseTests.cpp unittests/clangd/TestFS.cpp Index: unittests/clangd/TestFS.cpp === --- unittests/clangd/TestFS.cpp +++ unittests/clangd/TestFS.cpp @@ -55,8 +55,10 @@ 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(); } Index: unittests/clangd/GlobalCompilationDatabaseTests.cpp === --- /dev/null +++ unittests/clangd/GlobalCompilationDatabaseTests.cpp @@ -0,0 +1,37 @@ +//===-- 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 Index: unittests/clangd/CMakeLists.txt === --- unittests/clangd/CMakeLists.txt +++ unittests/clangd/CMakeLists.txt @@ -18,6 +18,7 @@ DraftStoreTests.cpp FileIndexTests.cpp FuzzyMatchTests.cpp + GlobalCompilationDatabaseTests.cpp HeadersTests.cpp IndexTests.cpp JSONExprTests.cpp Index: clangd/GlobalCompilationDatabase.h === --- clangd/GlobalCompilationDatabase.h +++ clangd/GlobalCompilationDatabase.h @@ -52,6 +52,7 @@ public: DirectoryBasedGlobalCompilationDatabase( llvm::Optional CompileCommandsDir); + ~DirectoryBasedGlobalCompilationDatabase() override; /// Scans File's parents looking for compilation databases. /// Any extra flags will be added. Index: clangd/GlobalCompilationDatabase.cpp === --- clangd/GlobalCompilationDatabase.cpp +++ clangd/GlobalCompilationDatabase.cpp @@ -18,17 +18,26 @@ tooling::CompileCommand GlobalCompilationDatabase::getFallbackCommand(PathRef File) const { + std::vector 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 CompileCommandsDir) : CompileCommandsDir(std::move(CompileCommandsDir)) {} +DirectoryBasedGlobalCompilationDatabase:: +~DirectoryBasedGlobalCompilationDatabase() = default; + llvm::Optional DirectoryBasedGlobalCompilationDatabase::getCompileCommand(PathRef File) const { if (auto CDB = getCDBForFile(File)) { ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45442: Parse .h files as objective-c++ if we don't have a compile command.
ilya-biryukov added inline comments. Comment at: clangd/GlobalCompilationDatabase.cpp:24 + // Parsing as Objective C++ is friendly to more cases. + if (llvm::sys::path::extension(File) == ".h") +Argv.push_back("-xobjective-c++-header"); sammccall wrote: > ilya-biryukov wrote: > > Maybe use `.equals_lower(".h")` instead? Just in case. > .H is (unambiguously) c++, and will be treated as such by clang. Wasn't aware of that convention, thanks. In that case, LG Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45442 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45442: Parse .h files as objective-c++ if we don't have a compile command.
sammccall added a comment. Will try to add a test. Comment at: clangd/GlobalCompilationDatabase.cpp:24 + // Parsing as Objective C++ is friendly to more cases. + if (llvm::sys::path::extension(File) == ".h") +Argv.push_back("-xobjective-c++-header"); ilya-biryukov wrote: > Maybe use `.equals_lower(".h")` instead? Just in case. .H is (unambiguously) c++, and will be treated as such by clang. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45442 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45442: Parse .h files as objective-c++ if we don't have a compile command.
ilya-biryukov added a comment. 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? Comment at: clangd/GlobalCompilationDatabase.cpp:24 + // Parsing as Objective C++ is friendly to more cases. + if (llvm::sys::path::extension(File) == ".h") +Argv.push_back("-xobjective-c++-header"); Maybe use `.equals_lower(".h")` instead? Just in case. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45442 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45442: Parse .h files as objective-c++ if we don't have a compile command.
sammccall created this revision. sammccall added a reviewer: ilya-biryukov. Herald added subscribers: cfe-commits, ioeric, jkorous-apple, klimek. This makes C++/objC not totally broken, without hurting C files too much. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45442 Files: clangd/GlobalCompilationDatabase.cpp Index: clangd/GlobalCompilationDatabase.cpp === --- clangd/GlobalCompilationDatabase.cpp +++ clangd/GlobalCompilationDatabase.cpp @@ -18,9 +18,15 @@ tooling::CompileCommand GlobalCompilationDatabase::getFallbackCommand(PathRef File) const { + std::vector 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=*/""); } Index: clangd/GlobalCompilationDatabase.cpp === --- clangd/GlobalCompilationDatabase.cpp +++ clangd/GlobalCompilationDatabase.cpp @@ -18,9 +18,15 @@ tooling::CompileCommand GlobalCompilationDatabase::getFallbackCommand(PathRef File) const { + std::vector 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=*/""); } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits