[cfe-commits] [PATCH] Fix bad highlight range when diagnosing non-class friend usage
Richard Smith
richard at metafoo.co.uk
Mon Sep 10 20:15:06 PDT 2012
On Mon, Sep 10, 2012 at 6:41 PM, Magee, Josh <Joshua.Magee at am.sony.com>wrote:
> Attached is a new patch that (a) fixes the assertion when emitting the
> warning dialogue in C++98 mode, and (b) implements C++11
> [class.friend]p3 (PR9834).
>
Thanks!
> --- 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.
> For (a) I removed FriendLoc from the highlight range. As noted, the
> caret is pointing to FriendLoc anyway.
>
> +++ 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).
> For (b): During parsing, I flagged whether or not the friend keyword
> appears first in friend declarations. Unfortunately, since the standard
> only requires friend to be the first token in non-function friend
> declarations, it isn't possible to generate an error at this point.
> Instead, in Semantic Analysis, generate an error if the FriendFirst flag
> is not set when checking a friend type (i.e., non-function) declaration.
>
> + 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().
> +++ 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.
Thanks,
> Josh
>
>
> At 1346342316 seconds past the Epoch, Richard Smith wrote:
> > I wonder if we should even get as far as producing that diagnostic in
> this
> > case. The code is ill-formed: for a non-function friend declaration,
> > 'friend' must be the first token in the declaration (see C++11
> > [class.friend]p3).
> >
> > + // Check whether the "friend" specifier is before or after the
> > + // type specifiers to determine the highlighted range.
> > + SourceRange HighlightRange = TypeRange.getEnd() < FriendLoc ?
> > + SourceRange(TypeRange.getBegin(),
> > PP.getLocForEndOfToken(FriendLoc)) :
> > + SourceRange(FriendLoc, TypeRange.getEnd());
> >
> > operator< on SourceLocation does not do what you expect here, and this
> > doesn't form the correct range in cases where the 'friend' keyword is
> > neither at the start nor at the end of the range, for instance "unsigned
> > friend int;". How about just not including FriendLoc in the highlighted
> > range at all? We're already pointing the caret at it.
> >
> > On Wed, Aug 29, 2012 at 5:50 PM, Magee, Josh <Joshua.Magee at am.sony.com
> >wrote:
> >
> > > Hi,
> > >
> > > Attached is a patch to fix an assertion that occurs when printing
> > > diagnostic
> > > warnings for non-class friend usage.
> > >
> > > Example:
> > > This case prints fine:
> > > struct { friend int; } a;
> > > warning: non-class friend type 'int' is a C++11 extension
> > > [-Wc++11-extensions]
> > > friend int;
> > > ^~~~~~~~~~
> > >
> > > This case causes an assertion:
> > > struct {int friend; } a;
> > > warning: non-class friend type 'int' is a C++11 extension
> > > [-Wc++11-extensions]
> > > ...
> > > Assertion `StartColNo <= EndColNo && "Invalid range!"' failed.
> > >
> > > The problem is that when printing the diagnostic the possibility that
> the
> > > friend specifier may appear after the type specifier is not considered.
> > > The
> > > fix is to check whether the friend specifier appears after the type and
> > > adjust
> > > the highlight range accordingly.
> > >
> > >
> > > Can someone review and, if everything is OK, commit?
> > >
> > > Thanks!
> > > Josh
> > > _______________________________________________
> > > cfe-commits mailing list
> > > cfe-commits at cs.uiuc.edu
> > > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> > >
> > >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120910/5393c985/attachment.html>
More information about the cfe-commits
mailing list