[Lldb-commits] [PATCH] D52461: [PDB] Introduce `MSVCUndecoratedNameParser`

[Pavel Labath via Phabricator via lldb-commits]

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