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

Pavel Labath via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Mon Mar 11 01:44:52 PDT 2019

labath added a comment.

Thanks Raphael. Apart from some inline nits, I have no further comments from my side, but hopefully someone with more familiarity with modules and clang ASTs can give this the final pass.

Comment at: lldb/include/lldb/Expression/ExpressionSourceCode.h:13-16
+#include "llvm/ADT/ArrayRef.h"
 #include <string>
+#include <vector>
It looks like these includes are no longer needed.

Comment at: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp:279
+    std::string module_imports;
+    for (std::string module : modules) {
+      module_imports.append("@import ");
`const std::string &` avoids a copy

Comment at: lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp:480-481
+    for (const SourceModule &m : sc.comp_unit->GetImportedModules()) {
+      // FIXME: Replace make_range with join(begin(), end(), ".") once join
+      // supports ConstString.
+      LLDB_LOG(log, "Found module in compile unit: {0} - include dir: {1}",
I would actually say that make_range is better than join here, because it avoids constructing a temporary string. So, I'd just remove the FIXME here. (BTW, if you want to control the separator string, there's already a syntax for that: `{0:$[.]}`)



More information about the lldb-commits mailing list