[Lldb-commits] [RFC] Type lookup for template types is broken...

[via lldb-commits]

> -----Original Message-----
> From: lldb-commits [mailto:lldb-commits-bounces at lists.llvm.org] On Behalf
> Of Pavel Labath via lldb-commits
> Sent: Tuesday, May 08, 2018 10:48 AM
> To: friss at apple.com
> Cc: lldb-commits at lists.llvm.org
> Subject: Re: [Lldb-commits] [RFC] Type lookup for template types is
> broken...
> 
> Well.. it encodes some assumptions about how a class name looks like,
> which
> are probably valid for C++, but they don't have to hold for any language
> frontend LLVM supports. That said, I am not saying this is worth the
> trouble of adding a special "these are the additional names you are to
> insert into the index" channel that clang should use to communicate this
> (I
> wouldn't be surprised if we make even stronger assumptions elsewhere). I
> was just curious about what your thoughts here were.

If you add an accelerator entry for "C" what does it point to?  All the
instantiations of "C"?  The DWARF does not describe the template, only
the concrete instances.
--paulr


> On Tue, 8 May 2018 at 15:29, Frédéric Riss <friss at apple.com> wrote:
> 
> 
> 
> > On May 8, 2018, at 2:23 AM, Pavel Labath <labath at google.com> wrote:
> 
> > I am still building a picture for myself of how the accelerator tables
> and
> > our name lookup works, but from what I managed to learn so far, adding
> an
> > accelerator for "C" seems like a useful thing to do. However, this does
> go
> > beyond what the DWARF 5 spec says we should do (we are only required to
> add
> > the DW_AT_name string). We are still free to add any extra entries we
> like,
> > but if we're going to be relying on this, we should try to get some of
> this
> > into the next version of the spec.
> 
> 
> > On Mon, 7 May 2018 at 22:19, Frédéric Riss via lldb-commits <
> > lldb-commits at lists.llvm.org> wrote:
> 
> > (...At least when using accelerator tables)
> 
> 
> > If you apply the following patch, TestClassTemplateParameterPack.py will
> 
> > start failing:
> 
> > diff --git
> 
> 
> a/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-
> pack/main.cpp
> 
> b/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-
> pack/main.cpp
> 
> > index 90e63b40f..304872a15 100644
> > ---
> 
> 
> a/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-
> pack/main.cpp
> 
> > +++
> 
> 
> b/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-
> pack/main.cpp
> 
> > @@ -37,7 +37,7 @@ template <> struct D<int, int, bool> : D<int, int> {
> 
> 
> 
> 
> >   int main (int argc, char const *argv[])
> >   {
> > -    C<int,16,32> myC;
> > +    C<int,16,32> myC; //% self.runCmd("settings set
> 
> > target.experimental.inject-local-vars false")
> 
> >       C<int,16> myLesserC;
> >       myC.member = 64;
> >       (void)C<int,16,32>().isSixteenThirtyTwo();
> 
> 
> > The test does things like invoke methods on temporary template objects:
> > //% self.expect("expression -- C<int, 16>().isSixteenThirtyTwo()",
> 
> > DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["false"])
> 
> > The above expression currently works because there’s a local of type
> 
> > C<int, 16>. With injected locals, the type is made readily available to
> > Clang. No type lookup is required for this to work in this setup.
> 
> > If you stop injecting locals, the test fails. We don’t provide the
> 
> > information to Clang to understand what C is. The reason is that when
> Clang
> > parses “C<int , 16>”, it is going to ask about “C”, not the fully
> templated
> > name. Our accelerator tables contain references to the full names, but
> not
> > to C alone and we never find it. If I change Clang and dsymutil to add
> an
> > accelerator for “C” each time an instance of C is seen then it nearly
> > works. I just need this additional lldb patch:
> 
> > diff --git a/source/Symbol/TypeMap.cpp b/source/Symbol/TypeMap.cpp
> > index 2838039ad..d2f2026bf 100644
> > --- a/source/Symbol/TypeMap.cpp
> > +++ b/source/Symbol/TypeMap.cpp
> > @@ -227,8 +227,11 @@ void TypeMap::RemoveMismatchedTypes(const
> 
> > std::string &type_scope,
> 
> >         } else {
> >           // The type we are currently looking at doesn't exists in a
> 
> > namespace
> 
> >           // or class, so it only matches if there is no type scope...
> > -        keep_match =
> > -            type_scope.empty() &&
> type_basename.compare(match_type_name)
> 
> > == 0;
> 
> > +        if (type_scope.empty()) {
> > +          keep_match = type_basename.compare(match_type_name) == 0 ||
> > +            (strlen(match_type_name) > type_basename.size() &&
> > +             match_type_name[type_basename.size()] == '<');
> > +        }
> >         }
> >       }
> 
> 
> > I didn’t post this as a Phabricator review as it requires changes in
> llvm
> 
> > before doing anything in LLDB and I wanted to make sure we agree this is
> > the right thing to do. I’m also not sure if this works out of the box on
> > platforms without accelerator tables.
> 
> > It won't work "out of the box", but it should be fairly simple to change
> > our indexing code to add the extra entries, so that a lookup for "C"
> works
> > the same way in both cases. BTW, how were you planning to compute the
> > untemplated string ("C"). Will you just strip everything after the first
> > '<' character, or were you thinking of something more fancy?
> 
> 
> > AFAIK, there are no fully qualified names in the debug info we generate
> so taking what’s before the first ‘<‘ should always return the class name.
> Does this logic seem flawed?
> 
> > Fred
> _______________________________________________
> lldb-commits mailing list
> lldb-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits