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

Jim Ingham via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Wed Aug 8 09:59:34 PDT 2018

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:


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...


More information about the lldb-commits mailing list