[Lldb-commits] [PATCH] D70721: [lldb/cpluspluslanguage] Add constructor substitutor

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Thanks for taking a look at this.



================
Comment at: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:327
 
+  bool trySubstitute(llvm::StringRef From, llvm::StringRef To) {
+    if (!llvm::StringRef(this->First, this->numLeft()).startswith(From))
----------------
shafik wrote:
> `trySubstitute` has a return value but it is not used in the code below.
Yeah, I couldn't make up my mind on what to do here. Having no indication of success from this function seems weird, but OTOH the callers don't actually need to actually vary their behavior based on the result. I've now just removed the return value...


================
Comment at: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:352
+  void appendUnchangedInput() {
+    Result += llvm::StringRef(Written, this->First - Written);
+    Written = this->First;
----------------
shafik wrote:
> `this->First - Written` feels awkward, I feel like given the names they should be reversed :-(
Yeah, it's a bit weird, but I'm not sure what to do about it.. do you have any specific suggestion? `std::distance(Written, First)` ? adding a member function like `currentParserPos()` to wrap the `First`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70721/new/

https://reviews.llvm.org/D70721