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

2019-03-26 Thread Alex Langford via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB357030: [ExpressionParser] Add swift-lldb case for finding clang resource dir (authored by xiaobai, committed by ). Herald added a subscriber: teemperor. Herald added a project: LLDB. Changed prior to

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

2019-03-26 Thread Alex Langford via Phabricator via lldb-commits
xiaobai added inline comments. Comment at: source/Plugins/ExpressionParser/Clang/ClangHost.cpp:42 /// This will compute the clang resource directory assuming that clang was -/// installed with the same prefix as lldb. +/// installed with the same prefix as lldb. Note: the verify

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

2019-03-25 Thread Jim Ingham via Phabricator via lldb-commits
jingham accepted this revision. jingham added a comment. Except for a typo and a little quibble about a comment, this looks okay to me. Comment at: source/Plugins/ExpressionParser/Clang/ClangHost.cpp:42 /// This will compute the clang resource directory assuming that clang was

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

2019-03-25 Thread Alex Langford via Phabricator via lldb-commits
xiaobai updated this revision to Diff 192204. xiaobai added a comment. Added comment about verify CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59708/new/ https://reviews.llvm.org/D59708 Files: source/Plugins/ExpressionParser/Clang/ClangHost.cpp unittests/Expression/ClangParserTest

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

2019-03-25 Thread Saleem Abdulrasool via Phabricator via lldb-commits
compnerd accepted this revision. compnerd added a comment. This revision is now accepted and ready to land. Please add a comment about `verify` being only for tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59708/new/ https://reviews.llvm.org/D59708 _

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

2019-03-25 Thread Alex Langford via Phabricator via lldb-commits
xiaobai added a comment. I changed the default behavior here setting file_spec to some known default unverified value to not setting it at all. It previously relied on there being a relative path to the clang resource dir, of which there are now two to pick from. Additionally, we pass in the di

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

2019-03-25 Thread Alex Langford via Phabricator via lldb-commits
xiaobai updated this revision to Diff 192185. xiaobai added a comment. Herald added a subscriber: jdoerfert. - Respect LIBDIR for swift-lldb case - Loop over a list of known directory suffixes - Change default behavior to not set file_spec and return false upon failure. CHANGES SINCE LAST ACTION

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

2019-03-25 Thread Alex Langford via Phabricator via lldb-commits
xiaobai added a comment. In 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 veri

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

2019-03-22 Thread 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 n

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

2019-03-22 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. 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

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

2019-03-22 Thread Alex Langford via Phabricator via lldb-commits
xiaobai created this revision. xiaobai added reviewers: aprantl, davide, compnerd, JDevlieghere, jingham. I'm adding this to reduce the difference between swift-lldb and llvm.org's lldb. https://reviews.llvm.org/D59708 Files: source/Plugins/ExpressionParser/Clang/ClangHost.cpp Index: source