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 <[email protected]>wrote: > > > > > At 1347336906 seconds past the Epoch, Richard Smith wrote: > > > On Mon, Sep 10, 2012 at 6:41 PM, Magee, Josh <[email protected] > > >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
friends-first-latest.diff
Description: friends-first-latest.diff
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
