On Wed Jan 15 2014 at 2:29:52 PM, Olivier Goffart <[email protected]> wrote:
> On Wednesday 15 January 2014 13:50:49 Richard Smith wrote: > > On Wed, Jan 15, 2014 at 1:25 PM, Olivier Goffart <[email protected]> > 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.)
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
