xiaobai added a comment. In D59708#1439872 <https://reviews.llvm.org/D59708#1439872>, @jingham wrote:
> The behavior when verify is false seems a little weird to me. In that case > you will just always get the $install_dir/lib/clang/$clang_version path. > That's fairly different from the verify = true case. OTOH, I couldn't see > anybody passing verify = false anywhere. Do we need that? The only use of `verify = false` is in the ClangHostTest (in the poorly named `unittests/Expression/ClangParserTest.cpp`). We pass a dummy path with verify set to false in order to make sure that it can correctly append `lib/clang/$clang_version`. Then we pass verify = true and make sure that it doesn't actually exist. ================ 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); ---------------- compnerd wrote: > Does swift-lldb never honour `LIBDIR`? That is, can it never end up in > `lib64`? Oh, it absolutely does. I'll fix that! ================ Comment at: source/Plugins/ExpressionParser/Clang/ClangHost.cpp:73 + return true; + } + ---------------- compnerd wrote: > 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; > } > } > ``` Yeah sounds good to me. Seems more manageable. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59708/new/ https://reviews.llvm.org/D59708 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits