JDevlieghere added inline comments.
================ Comment at: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp:267 : ExpressionParser(exe_scope, expr, generate_debug_info), m_compiler(), - m_pp_callbacks(nullptr), + m_pp_callbacks(nullptr), m_filename(filename), m_include_directories(std::move(include_directories)) { ---------------- `std::move(filename)` ================ Comment at: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.h:29 + static ClangExpressionSourceCode * + CreateWrapped(std::string filename, const char *prefix, const char *body) { + return new ClangExpressionSourceCode(filename, "$__lldb_expr", prefix, body, ---------------- Can we use StringRefs for prefix and body? ================ Comment at: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.h:31 + return new ClangExpressionSourceCode(filename, "$__lldb_expr", prefix, body, + true); } ---------------- Can you use an enum to name the boolean parameter? ``` enum Wrapping : bool { Wrap = true, NoWrap = false, }; ``` ================ Comment at: lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.h:88 private: + // The counter used by GetNextExprFileName. + uint32_t m_next_user_file_id = 0; ---------------- `///` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65646/new/ https://reviews.llvm.org/D65646 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits