[Lldb-commits] [PATCH] D59708: [ExpressionParser] Add swift-lldb case for finding clang resource dir

[Saleem Abdulrasool via Phabricator via lldb-commits]

compnerd added inline comments.


================
Comment at: source/Plugins/ExpressionParser/Clang/ClangHost.cpp:67
+  relative_path.clear();
+  llvm::sys::path::append(relative_path, "lib", "lldb", "clang");
+  llvm::sys::path::append(clang_dir, relative_path);
----------------
Does swift-lldb never honour `LIBDIR`?  That is, can it never end up in `lib64`?


================
Comment at: source/Plugins/ExpressionParser/Clang/ClangHost.cpp:73
+    return true;
+  }
+
----------------
I think it would be nicer if you could create a static list of the paths and loop over it:

```
static const StringRef kResourceDirSuffixes[] = {
  "lib" TO_STRING(CLANG_LIBDIR_SUFFIX) "/clang" CLANG_VERSION_STRING,
  "lib" TO_STRING(CLANG_LIBDIR_SUFFIX) "/lldb/clang",
};

for (const auto &Suffix : kResourceDirSuffixes) {
    llvm::SmallString<256> clang_dir(parent_dir);
    llvm::SmallString<32> relative_path(Suffix);
    llvm::sys::path::native(relative_path);
    llvm::sys::path::append(clang_dir, relative_path);
    if (!verify || VerifyClangPath(clang_dir)) {
      file_spec.GetDirectory().SetString(clang_dir);
      FileSystem::Instance().Resolve(file_spec);
      return true;
    }
}
```


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

https://reviews.llvm.org/D59708