[Lldb-commits] [PATCH] D58125: Add ability to import std module into expression parser to improve C++ debugging

[Pavel Labath via Phabricator via lldb-commits]

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