[PATCH] Fix source range of destructor name

[Richard Smith]

On Wed Jan 15 2014 at 2:29:52 PM, Olivier Goffart <ogoffart at kde.org> wrote:

> On Wednesday 15 January 2014 13:50:49 Richard Smith wrote:
> > On Wed, Jan 15, 2014 at 1:25 PM, Olivier Goffart <ogoffart at kde.org>
> wrote:
> > > Hi,
> > >
> > > This fixes http://llvm.org/bugs/show_bug.cgi?id=15125
> > > The problem is that destructor->getNameInfo().getSourceRange() only
> > > contains
> > > the '~' and not the class name after it.
> > >
> > > This is why for example the destructor name is not highlighted in my
> code
> > > browser, only the '~' has a tooltip.
> > >
> > > http://code.woboq.org/userspace/llvm/tools/clang/
> include/clang/AST/Declara
> > > tionName.h.html#_ZN5clang20Declarationisn'tNameTableD1Ev<http://code.
> woboq
> > > .org/userspace/llvm/tools/clang/include/clang/AST/
> DeclarationName.h.html#_
> > > ZN5clang20DeclarationNameTableD1Ev>
> > +            return CreateParsedType(MemberOfType,
> > +                    Context.getTrivialTypeSourceInfo(MemberOfType,
> > NameLoc));
> >              return ParsedType::make(MemberOfType);
> >
> > Please delete the unreachable 'return' statement here.
>
> This is obviously a left over.
>
> >
> > There are 3 other 'return ParsedType::make(...);' calls in this function,
> > and they all seem to have the same issue; do these need changes too?
>
> Right, I'll change them too.
>
> > Also, you aren't including the location information from the
> CXXScopeSpec in
> > the produced TypeSourceInfo, so this still doesn't look exactly right
> (for
> > p->X::Y::~Y(), you probably won't be able to provide an appropriate
> tooltip
> > for the 'X', even with this fix, for instance).
>
> I fail to understand this.
>
> only  ~Y  here is the name of the destructor.
> If I am not mistaken, X::Y:: is the qualifier (as traversed with
> destructor-
> >getQualifierLoc())    and should not be part of the function name itself.
>

Sorry, yes, you're absolutely right.


> > The changes to DeclarationName.cpp and to DeclPrinterTest.cpp look
> > unrelated to this fix (right?), and the fix itself seems to be missing a
> > test.
>
> They are related because now that  the destructor has a full type
> information,
>  '~class Foo'  is written instead of '~Foo'  and other tests were failing
> without that change.
>

OK, it's a shame that we need to do this, but this looks fine. It'd be a
little nicer to set LangOpts.CPlusPlus rather than setting
SuppressTagKeywords.

(I wonder if we can remove the printing of the tag keyword from
TypePrinter::printTag -- it should be wrapped in an ElaboratedType if
that's necessary, and the tag keyword would get printed there.)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140116/b7fbc211/attachment.html>