[PATCH] Fix source range of destructor name
Olivier Goffart
ogoffart at kde.org
Wed Jan 15 14:29:41 PST 2014
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.
> 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.
--
Olivier
More information about the cfe-commits
mailing list