labath added a comment. I don't know much about modules so these are just two comments I noticed from a quick glance:
- using both `virtual` and `override` on a method (like `ExternalASTSourceWrapper` does) is overkill. Drop `virtual`. - Returning `/usr/include` from `GetSystemIncludeDirectoriesForLanguage` is going to be wrong for remote sessions. The platform instance should have a "sysroot" path stored somewhere (from `platform select foo --sysroot=/bar`), so it would be good to at least honor that. It might even be good to pass the sysroot as the `--sysroot` parameter to clang. ================ Comment at: lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp:267 +void PlatformLinux::GetSystemIncludeDirectoriesForLanguage( + lldb::LanguageType lang, std::vector<std::string> &directories) { + switch (lang) { ---------------- return directories by value? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58125/new/ https://reviews.llvm.org/D58125 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits