aprantl added a comment.
Very nice!
================
Comment at: include/lldb/Expression/ExpressionParser.h:52
+ virtual bool Complete(StringList &matches, unsigned line, unsigned pos,
+ unsigned typed_pos) = 0;
----------------
Could you add a Doxygen comment?
================
Comment at: source/Commands/CommandObjectExpression.cpp:333
+ // Skip any leading whitespace.
+ while (!cmd.empty() && cmd.front() == ' ') {
+ cmd = cmd.drop_front();
----------------
Even if it is less efficient, a version of this using cmd.split(' ') or a loop
over cmd.ltrim(' ') would probably be much shorter and potentially easier to
read. There is also llvm::StringExtras::getToken().
================
Comment at: source/Commands/CommandObjectExpression.cpp:392
+
+ if (target) {
+ std::string arg;
----------------
Could you convert this to an early exit?
```
if (!target)
return 0;
```
================
Comment at: source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp:590
+ // to completing the current token.
+ StringRef to_remove = cmd;
+ while (!to_remove.empty() && !IsTokenSeparator(to_remove.back())) {
----------------
Should this perhaps use some form of StringRef::dropWhile or similar?
(https://llvm.org/doxygen/classllvm_1_1StringRef.html#aed7897c6ee084440516a7bb5e79a2094)
================
Comment at: source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp:696
+ // If we have a function decl that has no arguments we want to
+ // complete the empty brackets for the user. If the function has
+ // arguments, we at least complete the opening bracket.
----------------
s/brackets/parentheses/
================
Comment at: source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp:699
+ if (const FunctionDecl *F = dyn_cast<FunctionDecl>(D)) {
+ if (F->getNumParams() == 0) {
+ ToInsert += "()";
----------------
no need for curly braces here
================
Comment at: source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h:172
+ /// The number of parsing errors.
+ //-------------------------------------------------------------------
+ unsigned ParseInternal(DiagnosticManager &diagnostic_manager,
----------------
Perhaps return an llvm::Error instead of an unsigned?
================
Comment at: source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp:629
+ column = 0;
+
+ assert(abs_pos <= code.size() && "Absolute position outside code string?");
----------------
You could also slice the StringRef at abs_pos, ::count('\n'), set column to
slice.size-rfind('\n'). Your implementation is slightly faster, tough.
================
Comment at: source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp:659
+ lldb::LanguageType lang_type = lldb::LanguageType::eLanguageTypeUnknown;
+ if (auto new_lang = GetLanguageForExpr(diagnostic_manager, exe_ctx)) {
+ lang_type = new_lang.getValue();
----------------
We don't typically put single statements in curly braces in LLVM coding style.
https://reviews.llvm.org/D48465
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
