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:$[.]}`)
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
