xiaobai updated this revision to Diff 188816.
xiaobai added a comment.

Address suggestions and add unit test


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

https://reviews.llvm.org/D58748

Files:
  packages/Python/lldbsuite/test/functionalities/paths/TestPaths.py
  source/Plugins/ExpressionParser/Clang/ClangHost.cpp
  source/Plugins/ExpressionParser/Clang/ClangHost.h
  unittests/Expression/ClangParserTest.cpp

Index: unittests/Expression/ClangParserTest.cpp
===================================================================
--- unittests/Expression/ClangParserTest.cpp
+++ unittests/Expression/ClangParserTest.cpp
@@ -6,6 +6,8 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "clang/Basic/Version.h"
+
 #include "Plugins/ExpressionParser/Clang/ClangHost.h"
 #include "TestingSupport/TestUtilities.h"
 #include "lldb/Host/FileSystem.h"
@@ -29,7 +31,7 @@
 };
 } // namespace
 
-#ifdef __APPLE__
+#if !defined(_WIN32)
 static std::string ComputeClangDir(std::string lldb_shlib_path,
                                    bool verify = false) {
   FileSpec clang_dir;
@@ -38,6 +40,18 @@
   return clang_dir.GetPath();
 }
 
+TEST_F(ClangHostTest, ComputeClangDirectory) {
+  std::string path_to_liblldb = "/foo/bar/lib/";
+  std::string path_to_clang_dir = "/foo/bar/lib/clang/" CLANG_VERSION_STRING;
+  EXPECT_EQ(ComputeClangDir(path_to_liblldb), path_to_clang_dir);
+
+  // The path doesn't really exist, so verify shouldn't return
+  // path_to_clang_dir.
+  EXPECT_NE(ComputeClangDir(path_to_liblldb, true),
+            ComputeClangDir(path_to_liblldb));
+}
+
+#if defined(__APPLE__)
 TEST_F(ClangHostTest, MacOSX) {
   // This returns whatever the POSIX fallback returns.
   std::string posix = "/usr/lib/liblldb.dylib";
@@ -76,4 +90,5 @@
   EXPECT_NE(ComputeClangDir(GetInputFilePath(xcode), true),
             ComputeClangDir(GetInputFilePath(xcode)));
 }
-#endif
+#endif // __APPLE__
+#endif // !_WIN32
Index: source/Plugins/ExpressionParser/Clang/ClangHost.h
===================================================================
--- source/Plugins/ExpressionParser/Clang/ClangHost.h
+++ source/Plugins/ExpressionParser/Clang/ClangHost.h
@@ -13,7 +13,7 @@
 
 class FileSpec;
 
-#if defined(__APPLE__)
+#if !defined(_WIN32)
 bool ComputeClangDirectory(FileSpec &lldb_shlib_spec, FileSpec &file_spec,
                            bool verify);
 #endif
Index: source/Plugins/ExpressionParser/Clang/ClangHost.cpp
===================================================================
--- source/Plugins/ExpressionParser/Clang/ClangHost.cpp
+++ source/Plugins/ExpressionParser/Clang/ClangHost.cpp
@@ -32,13 +32,18 @@
 static bool ComputeClangDirectory(FileSpec &file_spec) { return false; }
 #else
 static bool DefaultComputeClangDirectory(FileSpec &file_spec) {
-  return HostInfoPosix::ComputePathRelativeToLibrary(
-      file_spec, (llvm::Twine("/lib") + CLANG_LIBDIR_SUFFIX + "/clang/" +
-                  CLANG_VERSION_STRING)
-                     .str());
+  llvm::SmallString<128> relative_path;
+  llvm::sys::path::append(relative_path,
+                          llvm::Twine("lib") + CLANG_LIBDIR_SUFFIX, "clang",
+                          CLANG_VERSION_STRING);
+  return HostInfoPosix::ComputePathRelativeToLibrary(file_spec, relative_path);
 }
 
-#if defined(__APPLE__)
+static bool ComputeClangDirectory(FileSpec &file_spec) {
+  if (FileSpec lldb_file_spec = HostInfo::GetShlibDir())
+    return ComputeClangDirectory(lldb_file_spec, file_spec, true);
+  return false;
+}
 
 static bool VerifyClangPath(const llvm::Twine &clang_path) {
   if (FileSystem::Instance().IsDirectory(clang_path))
@@ -53,6 +58,20 @@
 
 bool lldb_private::ComputeClangDirectory(FileSpec &lldb_shlib_spec,
                                          FileSpec &file_spec, bool verify) {
+#if !defined(__APPLE__)
+  std::string raw_path = lldb_shlib_spec.GetPath();
+  llvm::StringRef parent_dir = llvm::sys::path::parent_path(raw_path);
+
+  llvm::SmallString<128> clang_dir(parent_dir);
+  llvm::sys::path::append(clang_dir, llvm::Twine("lib") + CLANG_LIBDIR_SUFFIX,
+                          "clang", CLANG_VERSION_STRING);
+  if (!verify || VerifyClangPath(clang_dir)) {
+    file_spec.GetDirectory().SetString(clang_dir);
+    FileSystem::Instance().Resolve(file_spec);
+    return true;
+  }
+  return DefaultComputeClangDirectory(file_spec);
+#else
   std::string raw_path = lldb_shlib_spec.GetPath();
 
   auto rev_it = llvm::sys::path::rbegin(raw_path);
@@ -114,20 +133,8 @@
   file_spec.SetFile(raw_path.c_str(), FileSpec::Style::native);
   FileSystem::Instance().Resolve(file_spec);
   return true;
-}
-
-static bool ComputeClangDirectory(FileSpec &file_spec) {
-  if (FileSpec lldb_file_spec = HostInfo::GetShlibDir())
-    return ComputeClangDirectory(lldb_file_spec, file_spec, true);
-  return false;
-}
-#else  // __APPLE__
-
-// All non-Apple posix systems.
-static bool ComputeClangDirectory(FileSpec &file_spec) {
-  return DefaultComputeClangDirectory(file_spec);
-}
 #endif // __APPLE__
+}
 #endif // _WIN32
 
 FileSpec lldb_private::GetClangResourceDir() {
Index: packages/Python/lldbsuite/test/functionalities/paths/TestPaths.py
===================================================================
--- packages/Python/lldbsuite/test/functionalities/paths/TestPaths.py
+++ packages/Python/lldbsuite/test/functionalities/paths/TestPaths.py
@@ -32,6 +32,15 @@
             # No directory path types should have the filename set
             self.assertTrue(f.GetFilename() is None)
 
+    # TODO: Merge into test_path when GetClangResourceDir is implemented on
+    # windows
+    @expectedFailureAll(oslist=["windows"])
+    @no_debug_info_test
+    def test_clang_dir_path(self):
+      '''Test to make sure clang dir is set correctly'''
+      clang_dir = lldb.SBHostOS.GetLLDBPath(lldb.ePathTypeClangDir)
+      self.assertFalse(clang_dir.GetDirectory() is None)
+
     @no_debug_info_test
     def test_directory_doesnt_end_with_slash(self):
         current_directory_spec = lldb.SBFileSpec(os.path.curdir)
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to