[cfe-commits] [PATCH] Fix bad highlight range when diagnosing non-class friend usage

[Richard Smith]

Thanks, r164273.

On Fri, Sep 14, 2012 at 5:03 PM, Magee, Josh <Joshua.Magee at am.sony.com>wrote:

> New patch is attached with the requested changes.  I don't have commit
> access, so if you or anyone else could commit it then that would be
> great.  Thanks!
>
> At 1347566293 seconds past the Epoch, Richard Smith wrote:
> > Please change the name of the first parameter to CheckFriendTypeDecl to
> be
> > LocStart or similar, and fix the call in SemaTemplateInstantiateDecl to
> > pass in D->getLocStart(), then this LGTM. Do you have commit access, or
> do
> > you need someone to commit it for you?
> >
> > On Wed, Sep 12, 2012 at 6:49 PM, Magee, Josh <Joshua.Magee at am.sony.com
> >wrote:
> >
> > >
> > > At 1347336906 seconds past the Epoch, Richard Smith wrote:
> > > > On Mon, Sep 10, 2012 at 6:41 PM, Magee, Josh <
> Joshua.Magee at am.sony.com
> > > >wrote:
> > > > > --- a/include/clang/Basic/DiagnosticSemaKinds.td
> > > > > +++ b/include/clang/Basic/DiagnosticSemaKinds.td
> > > >
> > > > For your future consideration: Convention on these lists is to use
> -p0
> > > > patches instead of -p1 patches. You can configure git with '[diff]
> > > noprefix
> > > > = true' for this.
> > > Thanks for the tip, I updated my git config.
> > >
> > > > > +++ b/lib/Sema/SemaDeclCXX.cpp
> > > > > @@ -9879,7 +9880,7 @@ FriendDecl
> > > > *Sema::CheckFriendTypeDecl(SourceLocation Loc,
> > > > >               diag::warn_cxx98_compat_nonclass_type_friend :
> > > > >               diag::ext_nonclass_type_friend)
> > > > >          << T
> > > > > -        << SourceRange(FriendLoc, TypeRange.getEnd());
> > > > > +        << SourceRange(TypeRange.getBegin(), TypeRange.getEnd());
> > > >
> > > > "<< TypeRange" should work just as well here (2x).
> > > Done.
> > >
> > > > > +      if (DSContext == DSC_class) {
> > > > > +        bool FriendFirst = !DS.hasTypeSpecifier();
> > > > > +        isInvalid = DS.SetFriendSpec(Loc, PrevSpec, DiagID,
> > > FriendFirst);
> > > >
> > > > hasTypeSpecifier doesn't check for cv-qualifiers (and also some
> Altivec
> > > > oddball cases like "__vector friend __pixel;"). IIRC, even
> > > > DeclSpec::isEmpty misses the latter case. You could instead check
> for a
> > > > leading 'friend' in ParseDeclarationSpecifiers, or by comparing
> FriendLoc
> > > > against Range.getBeginLoc().
> > > Okay, comparing against FriendLoc is much simpler than what I was doing
> > > and it handles cv-qualifiers and the Altivec __vector cases.
> > >
> > >
> > > > > +++ b/test/SemaCXX/friend-diagnostic-crash.cpp
> > > >
> > > > Please add this test to test/SemaCXX/friend.cpp instead. Extra test
> files
> > > > cost more than extra code in existing tests.
> > > > test/CXX/class/class.friend/p3.cpp is fine, though.
> > > >
> > > I removed the test from the latest patch entirely for now.  The purpose
> > > of the test was to make sure that there wasn't a crash when printing
> out
> > > the
> > > highlight range.  Adding it to test/SemaCXX/friend.cpp doesn't catch
> > > this since the test is run with -verify which avoids the crash
> entirely.
> > > I could add an extra run line to friend.cpp to run again without
> > > -verify, but I don't know if that is preferable to having a separate
> > > test or not.  Maybe there is a better place for the test entirely?
> > >
> > > test/SemaCXX/cxx98-compat.cpp already handles checking for the
> > > warning "non-class friend type 'x' is a C++11 extension".
> > >
> > >
> > >
> > > Thanks,
> > > Josh
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120919/e700361b/attachment.html>