[PATCH] Fix source range of destructor name

[Richard Smith]

On Wed Jan 15 2014 at 1:51:41 PM, Justin Bogner <mail at justinbogner.com>
wrote:

> Olivier Goffart <ogoffart at kde.org> writes:
> > From d131af55d2dc2cb6b2f7242697eb44d2e5d042da Mon Sep 17 00:00:00 2001
> > From: Olivier Goffart <ogoffart at woboq.com>
> > Date: Wed, 15 Jan 2014 19:45:12 +0100
> > Subject: [PATCH] Fix source range of the destructor name.
> >
> > The problem is that the destructor's DeclarationNameInfo do not have
> > a TypeSourceInfo because Sema::GetNameForDeclarator requires the
> > ParsedType to be a LocInfoType.
> >
> > Setting a proper TypeSourceInfo to the destructor changes the way it
> > it printed (from '~Foo' to '~struct Foo'.  Hence the change in
> > DeclarationName.cpp which also fix a bug when printing operator names.
> >
> > http://llvm.org/bugs/show_bug.cgi?id=15125
>
> Please add a test with this. Some minor comments below.
>
> > ---
> >  lib/AST/DeclarationName.cpp       | 5 ++++-
> >  lib/Sema/SemaExprCXX.cpp          | 5 ++++-
> >  unittests/AST/DeclPrinterTest.cpp | 2 +-
> >  3 files changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/lib/AST/DeclarationName.cpp b/lib/AST/DeclarationName.cpp
> > index e064e23..3811bbf 100644
> > --- a/lib/AST/DeclarationName.cpp
> > +++ b/lib/AST/DeclarationName.cpp
> > @@ -537,7 +537,10 @@ void DeclarationNameInfo::printName(raw_ostream
> &OS) const {
> >          OS << '~';
> >        else if (Name.getNameKind() == DeclarationName::
> CXXConversionFunctionName)
> >          OS << "operator ";
> > -      OS << TInfo->getType().getAsString();
> > +      LangOptions LO;
> > +      PrintingPolicy SubPolicy(LO);
> > +      SubPolicy.SuppressTagKeyword = true;
> > +      OS << TInfo->getType().getAsString(SubPolicy);
> >      } else
> >        OS << Name;
> >      return;
> > diff --git a/lib/Sema/SemaExprCXX.cpp b/lib/Sema/SemaExprCXX.cpp
> > index a0c123f..7e4f5f3 100644
> > --- a/lib/Sema/SemaExprCXX.cpp
> > +++ b/lib/Sema/SemaExprCXX.cpp
> > @@ -209,7 +209,8 @@ ParsedType Sema::getDestructorName(SourceLocation
> TildeLoc,
> >            Context.hasSameUnqualifiedType(T, SearchType)) {
> >          // We found our type!
> >
> > -        return ParsedType::make(T);
> > +        return CreateParsedType(T, Context.getTrivialTypeSourceInfo(T,
> > +
>  NameLoc));
>
> This reads better as
>
>         return CreateParsedType(T,
>                                 Context.getTrivialTypeSourceInfo(T,
> NameLoc));
>
> >        }
> >
> >        if (!SearchType.isNull())
> > @@ -245,6 +246,8 @@ ParsedType Sema::getDestructorName(SourceLocation
> TildeLoc,
> >                = dyn_cast<ClassTemplateSpecializationDecl>(Record->getDecl()))
> {
> >            if (Spec->getSpecializedTemplate()->getCanonicalDecl() ==
> >                  Template->getCanonicalDecl())
> > +            return CreateParsedType(MemberOfType,
> > +                    Context.getTrivialTypeSourceInfo(MemberOfType,
> NameLoc));
>
> This is a strange way to indent this. According to clang-format, this
> should be:
>
>             return CreateParsedType(
>                 MemberOfType,
>                 Context.getTrivialTypeSourceInfo(MemberOfType, NameLoc));
>
> >              return ParsedType::make(MemberOfType);
> >          }
> >
> > diff --git a/unittests/AST/DeclPrinterTest.cpp b/unittests/AST/
> DeclPrinterTest.cpp
> > index 44fa742..2e335e3 100644
> > --- a/unittests/AST/DeclPrinterTest.cpp
> > +++ b/unittests/AST/DeclPrinterTest.cpp
> > @@ -558,7 +558,7 @@ TEST(DeclPrinter, TestCXXConversionDecl3) {
> >      "  operator Z();"
> >      "};",
> >      methodDecl(ofClass(hasName("A"))).bind("id"),
> > -    "Z operator struct Z()"));
> > +    "Z operator Z()"));
> >      // WRONG; Should be: "operator Z();"
> >  }
>
> Looks like the comment here needs updating.
>

The comment is still appropriate: we shouldn't be printing a return type.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140115/4f71c45a/attachment.html>