[Lldb-commits] [PATCH] D138834: [lldb] Fix simple template names interaction with debug info declarations

[Arthur Eubanks via Phabricator via lldb-commits]

aeubanks added inline comments.


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:765
+        m_ast.CreateClassTemplateSpecializationType(class_specialization_decl);
+    return clang_type.GetTypeName(/*BaseOnly*/ true);
+  }
----------------
Michael137 wrote:
> aeubanks wrote:
> > Michael137 wrote:
> > > Michael137 wrote:
> > > > Ok so what we're doing is:
> > > > 1. Create a `ClassTemplateSpecializationDecl` with an empty basename
> > > > 2. Return the type-name and since the basename is empty we end up with just the template arguments
> > > > 
> > > > Why can't we just call `GetTemplateParametersString(die)` instead of creating this function?
> > > Can confirm that this works locally. Though required moving that function out of the DWARFASTParserClang, which seems OK
> > `GetTemplateParametersString` is specifically only used for `GetCPlusPlusQualifiedName`, which is used below
> > ```
> >       // For C++, we rely solely upon the one definition rule that says
> >       // only one thing can exist at a given decl context. We ignore the
> >       // file and line that things are declared on.
> >       std::string qualified_name = GetCPlusPlusQualifiedName(die);
> > ```
> > so it doesn't have to match clang's printing. but for the simple template name stuff, we are comparing clang-generated names, so everything needs to go through clang.
> > 
> > I've replaced `GetTemplateParametersString` with this code that goes through clang as well
> But didn't `GetTemplateParametersString` go through Clang's type printer too? Probably missing something.
> What would be an example of the difference in output that could arise between the new vs. old method?
one thing that I was aware of even initially implementing `GetTemplateParametersString` was spacing between two `>`, clang will add a space (unless you tell it not to), whereas the naive implementation didn't


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138834