labath added inline comments.
================ Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:1875-1878 + if (entry.startswith(line)) { + llvm::StringRef res = entry.substr(line.size()); + result = res.str(); + return result; ---------------- teemperor wrote: > gedatsu217 wrote: > > labath wrote: > > > ``` > > > if (entry.consume_front(line)) { > > > result = entry.str(); > > > return result; // Why is this returning the result both as a proper > > > return value and a by-ref argument? > > > } > > > ``` > > That is because using the llvm::Optional as a return value instead of void > > would make it more obvious what the function returns in case there is no > > suggestion. > > > > We've discussed this issue before, so please see the comments above. > Pavel's point is that the function is returning >both<. You can just remove > the `result` parameter and do `result = ... > GetAutoSuggestionForCommand(line);` when you call it is his point. Yes that was one of my points. Thank you, Raphael. My other point was that using `consume_front` is better than a `startswith`+`substr` combo. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81001/new/ https://reviews.llvm.org/D81001 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits