Hello John! Thanks for the review, I have to provide some explanation (see below). Which changes do you think are worth making then?
On 17 April 2013 03:12, John McCall <[email protected]> wrote: > > > This is the right general idea, thanks! > > + int ChunkIndex = state.getCurrentChunkIndex(); > + while (ChunkIndex != 0 && > + D.getTypeObject(ChunkIndex).Kind == DeclaratorChunk::Paren) > + ChunkIndex--; > > Isn't the current ChunkIndex always a DeclaratorChunk::Function? Nope, it can be DeclaratorChunk::Paren for typedefs, for example: typedef (__stdcall *ptr)(); will have pointer, paren and function chunks and the checking is called on paren or for variable declarations, for example void (*__thiscall foo)(); and the checking is called on pointer. > I think you want D.getTypeObject(ChunkIndex - 1) here, although I guess > I might be off-by-one somehow. if (D.getTypeObject(ChunkIndex).Kind == DeclaratorChunk::Paren) --ChunkIndex; works for all my tests and frankly I can't figure out a declarator with two consecutive paren chunks. However, most places with similar analysis in SemaType allow skipping multiple consecutive paren chunks. > > > + bool IsCXXMethod = > + D.getTypeObject(ChunkIndex).Kind == DeclaratorChunk::MemberPointer; > + > + // Non-friend functions declared in member context are methods. > + IsCXXMethod = IsCXXMethod || (D.getContext() == Declarator::MemberContext > + && D.isFunctionDeclarator() > + && !D.getDeclSpec().isFriendSpecified()); > > I think the right thing to do here is more like: > bool IsCXXMethod; > if (ChunkIndex == 0) { // CC applies to the function being declared > assert(D.isFunctionDeclarator()); This is not necessarily a function declarator, consider for example variable decl void (*__fastcall fastpfunc)(); chunks are: pointer, paren, function and the checking is called for the the first one. > IsCXXMethod = (D.getContext() == Declarator::MemberContext && > !D.getDeclSpec().isFriendSpecified() && > D.getDeclSpec().getStorageClassSpecifier() != SC_Static); > } else { > IsCXXMethod = D.getTypeObject(ChunkIndex - 1).Kind == > DeclaratorChunk::MemberPointer; > } > > For out-of-line method declarations, we should just be able rely on > redeclaration checking. It looks like at this point we don't have redeclaration info yet. Types are computed before looking up previous declaration in HandleDeclarator. On the other hand, friend declarations will not be handled correctly this way since we can no longer differentiate between friend free functions and friend methods from other classes. And we have no diagnostic if friend declaration uses different CC than original declaration. -- Alex _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
