[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

[Sebastian Redl]

Douglas Gregor wrote:
> 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.
>
Yes, I'll be addressing some of those over the next few days.
>
>> +
>> +  /// 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.
The term "member pointer" is used as well; see for example 4.11p1. It's
true, though, that "pointer to member" is the more common one, and the
one in the name of the relevant standard section.
>
> (There are a handful of other things I'd like to rename along these
> lines; CXXMethodDecl->MemberFunction, EnumDecl->EnumerationDecl,
> EnumConstantDecl->EnumeratorDecl, etc.)
I find EnumDecl and EnumConstantDecl more intuitive - EnumDecl is what
you get from the enum keyword, and the Constant in EnumConstantDecl
makes it pretty clear what's meant. The word enumerator always gives me
trouble, and enumeration and enumerator look very much alike.
>
>> 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.
OK.
>
>> /// 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.
>
Oops, forgot to convert this after you convinced me to derive from Type.
>>
>> +  // 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?
I just followed the lead of the check directly below.
>
> 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);
>                ~~~^
I'll look at it. Thanks for the review.

Sebastian