[cfe-dev] [RFC] Preliminary patch to support MSVC __declspec(property)

[John McCall]

On Dec 29, 2012, at 1:03 AM, Francois Pichet <pichet2000 at gmail.com> wrote:
> On Sat, Dec 22, 2012 at 10:11 AM, endlessroad1991 at gmail.com <endlessroad1991 at gmail.com> wrote:
> Some guy from PathScale told me that AMP headers doesn't contain subscriptable property. And I looked at ATL headers, neither do they. So for these purposed, subscriptable property is not necessary.
> And I still think that subscriptable property is kind of messy...
> So I only implement non-subscriptable property.
> 
> Brief:
> - Add a MSPropertyDecl, parallel to FieldDecl. Now property is separated from Field.
> - Add a MSPropertyRefExpr. It seems to be necessary, because we can't just create a PseudoObjectExpr.
> 
> Current status:
> - Non-subscriptable property only.
> - Property can be inherited.
> - Get, Set, =, +=/-=/..., ++/-- works correctly.
> - Property type can be incomplete type, as function return type.
> 
> Patch, and a "difficult" testcase attached.
> I'm quite happy with this version, because it's much simpler and cleaner.
> John - Thank you very much, for the MSPropertyDecl and PseudoObjectExpr tips. They are really crucial to this patch.
> 
> 
> More comments:
> 
> +  NamedDecl *NewFD;
> +  AttributeList *MSPropertyAttr = 0;
> +  for (auto it = D.getDeclSpec().getAttributes().getList();
> +       it != 0; it = it->getNext())
> +    if (it->getName()->getName() == "property") {
> +      MSPropertyAttr = it;
> +      break;
> +    }
> +  if (MSPropertyAttr) {
> +    llvm::StringRef GetterName = MSPropertyAttr->getScopeName() ?
> +      MSPropertyAttr->getScopeName()->getName() : StringRef();
> +    llvm::StringRef SetterName = MSPropertyAttr->getParameterName() ?
> +      MSPropertyAttr->getParameterName()->getName() : StringRef();
> +    NewFD = new (Context) MSPropertyDecl(Decl::MSProperty, Record, Loc,
> +                                         II, T, TInfo, TSSL,
> +                                         GetterName, SetterName);
> +    NewFD->setAccess(AS);
> +  } else {
> +    NewFD = CheckFieldDecl(II, T, TInfo, Record, Loc, Mutable, BitWidth,
> +                           InitStyle, TSSL, AS, PrevDecl, &D);
> +  }
> 
> I think this code should be in Sema::ActOnCXXMemberDeclarator and not in Sema::HandleField. A property is not a field and HandleField should continue to return a FieldDecl*. 
> 
> Also you need to protect this code with Sema.getOptions().MicrosoftExt
> 
>  
> +static ExprResult
> +BuildMSPropertyRefExpr(ASTContext &Context, Expr *BaseExpr, bool IsArrow,
> +                       const DeclarationNameInfo &MemberNameInfo,
> +                       MSPropertyDecl *PD) {
> +  return new (Context) MSPropertyRefExpr(BaseExpr, PD, IsArrow,
> +                                         Context.PseudoObjectTy, VK_LValue,
> 
> The type of the expression should be PD->getType() and not Context.PseudoObjectTy.
> Otherwise you won't be able to use a MSPropertyRefExpr in the context of a larger expression.
> 
> For example:
> class C {
> public:
>   int operator[](int i){ return i;}
> };
> 
> struct S {
>    C v;
>    C getprop() { return v; }
>    __declspec(property(get = getprop)) C the_prop;
> };
> int main()
> {
>    S s;
>    int a = s.the_prop[3];
>   printf("%d", a);
> }
> this must print 3.
> 
> This is *not* a property indexer. The result of the MSPropertyRefExpr expression must be class C to allow for the operator[] to be used. C++ AMP which use ms property extensively will contains code like that.

No, it is correct for the expression to have pseudo-object type here.  In most contexts, expressions of pseudo-object type automatically undergo lvalue-to-rvalue conversion before any further analysis is performed.  The subscript-expression code will have to be modified to specially check for possible property indexers when the base has pseudo-object type, and (if so) propagate the pseudo-object type "outwards".

John.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20130101/5e022717/attachment.html>