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
