[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 dont
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)) {}
[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
