labath added a comment.

It's not fully clear to me from the previous comments if you are proceeding 
with this or not, but in case you are, I have made comments inline. I see that 
you've added some lit tests, but I also think you it would be good add some 
unit tests for the name parser functionality per-se (similar to the existing 
name parsing tests), as it is much easier to see what is going on from those.

Right now, I don't think this solution can be specific to the "old" PDB plugin, 
as this functionality is used from other places as well (RichManglingContext 
being the most important one). Maybe once we start using the fancy MSVC 
demangler there, we can revisit that. (But given that the declared intent of 
that class is to chop up demangled names, I think it would make sense to keep 
this there even then. Unless it turns out we can delete the whole class at that 
point.)



================
Comment at: 
source/Plugins/Language/CPlusPlus/MSVCUndecoratedNameParser.cpp:62-64
+      m_specifiers.emplace_back(llvm::StringRef(name.data(), i - 1),
+                                llvm::StringRef(name.data() + last_base_start,
+                                                i - last_base_start - 1));
----------------
Rewrite this (and all other instances of StringRef -> char * -> StringRef 
roundtripping) in terms of StringRef functions.
Maybe something like: `emplace_back(name.take_front(i-1), 
name.slice(last_base_start, i-1));` ?


================
Comment at: source/Plugins/Language/CPlusPlus/MSVCUndecoratedNameParser.h:35-39
+  std::size_t GetSpecifiersCount() const { return m_specifiers.size(); }
+
+  MSVCUndecoratedNameSpecifier GetSpecifierAtIndex(std::size_t index) const {
+    return m_specifiers[index];
+  }
----------------
Could we replace these by something like 
`ArrayRef<MSVCUndecoratedNameSpecifier> GetSpecifiers()`


https://reviews.llvm.org/D52461



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to