[Lldb-commits] [PATCH] D67760: [lldb] Decouple importing the std C++ module from the way the program is compiled

[Raphael Isemann via Phabricator via lldb-commits]

teemperor marked 6 inline comments as done.
teemperor added a comment.

Thanks for the quick review!



================
Comment at: lldb/include/lldb/Symbol/CompileUnit.h:228
 
+  /// Apply a lambda to each external module referenced by this compilation
+  /// unit. Recursively also descends into the referenced external modules
----------------
aprantl wrote:
> What does "external module" mean in this context? Can we be specific about lldb::Module vs clang::Module?
Rather lldb::Module. Well, it refers to the external type modules we get with -gmodules (i.e. what we store in SymbolFileDWRF::m_external_type_modules), so it is kind of both in this case.


================
Comment at: lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.cpp:46
+  // we should remove that '/bits' suffix to get the actual include directory.
+  if (dir.endswith("/usr/include/bits"))
+    dir.consume_back("/bits");
----------------
aprantl wrote:
> llvm::sys::path::append to avoid hardcoding path separators?
I would rather have this converted to POSIX style and work with that. Otherwise this whole function needs to be path-sep independent (including escaping the regex and what not). And that ends up being a lot of boilerplate to make this OS-specific code OS-agnostic.


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

https://reviews.llvm.org/D67760