[Lldb-commits] [PATCH] D50071: Use rich mangling information in Symtab::InitNameIndexes()

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

You and Pavel have done great work getting this all straight.  I don't have much to add except some trivial quibbles about variable naming, a grammar error and some other inessentials.



================
Comment at: include/lldb/Core/Mangled.h:323-325
+  /// @return
+  ///     The rich mangling info on success, null otherwise. Expect the pointer
+  ///     to be valid only until the next call to this funtion.
----------------
The return description isn't right anymore.


================
Comment at: include/lldb/Core/RichManglingContext.h:98
+
+  /// Cast the given parser to the given type. Ideally we had a type trait
+  /// to deduce \a ParserT from a given InfoProvider, but unfortunately we
----------------
had -> would have


================
Comment at: source/Core/Mangled.cpp:305-306
+  // Check whether or not we are interested in this name at all.
+  llvm::StringRef M = m_mangled.GetStringRef();
+  ManglingScheme S = cstring_mangling_scheme(M.data());
+  if (skip_mangled_name && skip_mangled_name(M, S))
----------------
locals in lldb are lower case always, and we really try not to use one letter variables (M & S below...)


================
Comment at: source/Core/Mangled.cpp:239-240
+//----------------------------------------------------------------------
+namespace {
+
+char *GetMSVCDemangledStr(const char *M) {
----------------
The LLVM conventions suggest only putting declarations in the anon namespace, and then having the definitions out of line:

http://www.llvm.org/docs/CodingStandards.html#anonymous-namespaces


================
Comment at: source/Core/RichManglingContext.cpp:147
+  case ItaniumPartialDemangler: {
+    auto N = m_ipd_buf_size;
+    auto buf = m_ipd.getFunctionDeclContextName(m_ipd_buf, &N);
----------------
Again, don't use capitol letter local variables


================
Comment at: source/Symbol/Symtab.cpp:376-377
+  // Only register functions that have a base name.
+  MC.ParseFunctionBaseName();
+  llvm::StringRef base_name = MC.GetBufferRef();
+  if (base_name.empty())
----------------
Not MC either...


https://reviews.llvm.org/D50071