[cfe-commits] r62934 - in /cfe/trunk: include/clang/AST/ASTContext.h include/clang/AST/Type.h include/clang/Basic/DiagnosticKinds.def include/clang/Parse/DeclSpec.h include/clang/Parse/Ownership.h lib/AST/ASTContext.cpp lib/AST/Type.cpp lib/AST/TypeSerialization.cpp lib/Parse/ParseDecl.cpp lib/Sema/SemaCXXScopeSpec.cpp lib/Sema/SemaType.cpp test/SemaCXX/member-pointer.cpp

[Douglas Gregor]

On Jan 24, 2009, at 1:16 PM, Sebastian Redl wrote:

> Author: cornedbee
> Date: Sat Jan 24 15:16:55 2009
> New Revision: 62934
>
> URL: http://llvm.org/viewvc/llvm-project?rev=62934&view=rev
> Log:
> Add support for declaring pointers to members.


This is awesome, thank you! There's a boat-load of FIXMEs regarding  
conversions and overloading that involve pointers to members, most of  
which will be relatively easy to deal with now that you've done the  
parsing and representation bits.


> +
> +  /// getMemberPointerType - Return the uniqued reference to the  
> type for a
> +  /// member pointer to the specified type in the specified class.  
> The class
> +  /// is a Type because it could be a dependent name.
> +  QualType getMemberPointerType(QualType T, const Type *Cls);

So, I have to ask: should we use the term "PointerToMember" rather  
than "MemberPointer" throughout? "Pointer to member" is the term used  
in the standard.

(There are a handful of other things I'd like to rename along these  
lines; CXXMethodDecl->MemberFunction, EnumDecl->EnumerationDecl,  
EnumConstantDecl->EnumeratorDecl, etc.)

> Modified: cfe/trunk/lib/AST/ASTContext.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=62934&r1=62933&r2=62934&view=diff
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- cfe/trunk/lib/AST/ASTContext.cpp (original)
> +++ cfe/trunk/lib/AST/ASTContext.cpp Sat Jan 24 15:16:55 2009
> @@ -374,7 +379,19 @@
>     // FIXME: This is wrong for struct layout: a reference in a  
> struct has
>     // pointer size.
>     return getTypeInfo(cast<ReferenceType>(T)->getPointeeType());
> -
> +  case Type::MemberPointer: {
> +    // Note that this is not only platform- but also ABI-dependent.  
> We follow
> +    // the GCC ABI, where pointers to data are one pointer large,  
> pointers to
> +    // functions two pointers. But if we want to support ABI  
> compatibility with
> +    // other compilers too, we need to delegate this completely to  
> TargetInfo.
> +    QualType Pointee = cast<MemberPointerType>(T)->getPointeeType();
> +    unsigned AS = Pointee.getAddressSpace();
> +    Width = Target.getPointerWidth(AS);
> +    if (Pointee->isFunctionType())
> +      Width *= 2;
> +    Align = Target.getPointerAlign(AS);
> +    // GCC aligns at single pointer width.
> +  }

Please put a FIXME: in that comment above so we don't forget to  
(eventually) move this into TargetInfo or some future ABI abstraction  
layer.

> /// isVariablyModifiedType (C99 6.7.5p3) - Return true for variable  
> length
> /// array types and types that contain variable array types in their
> /// declarator
> @@ -300,8 +318,11 @@
>   if (const Type *T = getArrayElementTypeNoTypeQual())
>     return T->isVariablyModifiedType();
>
> -  // A pointer can point to a variably modified type
> -  if (const PointerType *PT = getAsPointerType())
> +  // A pointer can point to a variably modified type.
> +  // Also, C++ references and member pointers can point to a  
> variably modified
> +  // type, where VLAs appear as an extension to C++, and should be  
> treated
> +  // correctly.
> +  if (const PointerLikeType *PT = getAsPointerLikeType())
>     return PT->getPointeeType()->isVariablyModifiedType();

The comment doesn't match the code here, since MemberPointerType isn't  
a PointerLikeType.

> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- cfe/trunk/lib/Parse/ParseDecl.cpp (original)
> +++ cfe/trunk/lib/Parse/ParseDecl.cpp Sat Jan 24 15:16:55 2009
> @@ -1550,10 +1550,10 @@
> /// isn't parsed at all, making this function effectively parse the C 
> ++
> /// ptr-operator production.
> ///
> -///       declarator: [C99 6.7.5]
> -///         pointer[opt] direct-declarator
> -/// [C++]   '&' declarator [C++ 8p4, dcl.decl]
> -/// [GNU]   '&' restrict[opt] attributes[opt] declarator
> +///       declarator: [C99 6.7.5] [C++ 8p4, dcl.decl]
> +/// [C]     pointer[opt] direct-declarator
> +/// [C++]   direct-declarator
> +/// [C++]   ptr-operator declarator
> ///
> ///       pointer: [C99 6.7.5]
> ///         '*' type-qualifier-list[opt]
> @@ -1563,11 +1563,41 @@
> ///         '*' cv-qualifier-seq[opt]
> ///         '&'
> /// [GNU]   '&' restrict[opt] attributes[opt]
> -///         '::'[opt] nested-name-specifier '*' cv-qualifier- 
> seq[opt] [TODO]
> +///         '::'[opt] nested-name-specifier '*' cv-qualifier-seq[opt]
> void Parser::ParseDeclaratorInternal(Declarator &D,
>                                      DirectDeclParseFunction  
> DirectDeclParser) {
> -  tok::TokenKind Kind = Tok.getKind();
>
> +  // C++ member pointers start with a '::' or a nested-name.
> +  // Member pointers get special handling, since there's no place  
> for the
> +  // scope spec in the generic path below.
> +  if ((Tok.is(tok::coloncolon) || Tok.is(tok::identifier) ||
> +       Tok.is(tok::annot_cxxscope)) && getLang().CPlusPlus) {
> +    CXXScopeSpec SS;
> +    if (ParseOptionalCXXScopeSpecifier(SS)) {
> +      if(Tok.isNot(tok::star)) {
> +        // The scope spec really belongs to the direct-declarator.
> +        D.getCXXScopeSpec() = SS;
> +        if (DirectDeclParser)
> +          (this->*DirectDeclParser)(D);
> +        return;
> +      }

Ah, good. I like this non-lookahead approach. Perhaps the  
getLang().CPlusPlus check should go first?

> Added: cfe/trunk/test/SemaCXX/member-pointer.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/member-pointer.cpp?rev=62934&view=auto
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- cfe/trunk/test/SemaCXX/member-pointer.cpp (added)
> +++ cfe/trunk/test/SemaCXX/member-pointer.cpp Sat Jan 24 15:16:55 2009
> @@ -0,0 +1,14 @@
> +// RUN: clang -fsyntax-only -verify %s
> +
> +struct A {};
> +enum B { Dummy };
> +namespace C {}
> +
> +int A::*pdi1;
> +int (::A::*pdi2);
> +int (A::*pfi)(int);
> +
> +int B::*pbi; // expected-error {{expected a class or namespace}}
> +int C::*pci; // expected-error {{'pci' does not point into a class}}
> +void A::*pdv; // expected-error {{'pdv' declared as a member  
> pointer to void}}
> +int& A::*pdr; // expected-error {{'pdr' declared as a pointer to a  
> reference}}


It looks like we'll also need to update the declaration/expression  
ambiguity resolver in lib/Parse/ParseTentative.cpp. Here's a test case  
for that:

void f() {
   int (A::*pf)(int, int);
}

Which currently fails with:

/Users/dgregor/Projects/llvm/tools/clang/test/SemaCXX/member- 
pointer.cpp:18:11: error: expected unqualified-id
   int (A::*pf)(int, int);
           ^
/Users/dgregor/Projects/llvm/tools/clang/test/SemaCXX/member- 
pointer.cpp:18:19: error: expected '(' for function-style cast or type  
construction
   int (A::*pf)(int, int);
                ~~~^

   - Doug