[cfe-dev] [clang] declspec(property...) advice

[John McCall]

On Feb 7, 2011, at 10:15 AM, Argyrios Kyrtzidis wrote:
> On Feb 6, 2011, at 8:40 AM, Eric Niebler wrote:
> On 2/4/2011 4:00 PM, Eric Niebler wrote:
>>> (resurrecting this old thread....)
>>> 
>>> On 12/9/2010 5:16 AM, John McCall wrote:
>>>> I'm a little worried about adopting our current representation for a
>>>> C++-oriented feature.  Presumably we'd need to allow filling in default
>>>> arguments, etc., plus a wide range of stuff that we technically have to
>>>> support with ObjC++ properties but actually haven't gotten around to
>>>> yet, like what happens when we're doing a compound assignment on an
>>>> implicit property reference and the getter and setter have really
>>>> different types.  Really I think this is an argument for improving the
>>>> representation of all property expressions.
>>> 
>>> Indeed.
>>> 
>>>> My intuition here is to just rename OK_ObjCProperty to OK_Property
>>>> instead of differentiating the two cases.  
>>> 
>>> Yes.
>>> 
>>>> It can mean "this is an
>>>> entity which can be both read and written to, but requires special code
>>>> to do so".  The existing hook in Sema for loads
>>>> (ConvertPropertyForRValue) can easily be modified to handle the MS
>>>> properties, and we can make BuildBinOp check for OK_Property LHS's and
>>>> just immediately defer to some better hook.
>>> 
>>> And BuildUnaryOp for pre- and post-increment/decrement.
>>> 
>>> Doug, I've found a compelling reason why at least setter methods cannot
>>> be resolved until the point of use. Consider:
>>> 
>>> #include <cstdio>
>>> 
>>> struct T {};
>>> struct U {};
>>> 
>>> struct S {
>>>   void setI( T ) { std::printf("T\n"); }
>>>   void setI( U ) { std::printf("U\n"); }
>>>   __declspec(property(put=setI)) int I;
>>> };
>>> 
>>> int main() {
>>>   S s;
>>>   s.I = T();
>>>   s.I = U();
>>> }
>>> 
>>> This prints:
>>> 
>>> T
>>> U
>>> 
>>> Nasty, eh? I don't think the existing property stuff can handle this at
>>> all, so like John, I feel this needs a major reworking.
>>> 
>>> Any and all suggestions welcome. This has become my #1 priority, and I
>>> want to make forward progress quickly, but I want to do it right.
>> 
>> I'd like to share my current thinking on the property issue. I encourage
>> any and all interested parties to jump in.
>> 
>> Above, John McCall suggests modifying BuildBinOp to Do Something. I
>> think that something is to programatically rewrite the AST into the
>> appropriate getter and setter calls. Ditto for BuildUnaryOp. So for
>> instance, for this code:
>> 
>> ++obj.property;
>> 
>> when BuildUnaryOp gets passed a PropertyRefExpr and the
>> pre-increment op code, it builds the AST for this:
>> 
>> obj.setProperty(obj.getPropery() + 1)
> 
> I'm not sure I understand why we have to have so expicit AST nodes, traditionally the nodes were as close to the source code as possible.
> Can't we do semantic checking otherwise ? Do we need to simplify it to make it easier for codegen ?
> One could argue that we should rewrite all
> ++variable;
> to
> variable = variable + 1;

Unfortunately, compound assignments have semantics that are somewhat more complicated than a simple top-down walk through a syntax tree:  we evaluate the LHS as an l-value, coerce the loaded value to the type of the operation, perform the operation, coerce the result back to the l-value type, and store that in.  We kindof finesse this right now — clients like IR generation have to duplicate the coercion logic that normally Sema would do.  That's acceptable for most things only because there's a limit to how complicated this analysis can get.  With property reference l-values, however, the l-value accesses are secretly function calls;  we're not getting all the cases right even for Objective-C property references (e.g. with getters and setters of asymmetric type or of C++ class type), and I expect that's probably a non-starter for a C++-centered property system.

John.


> 
> -Argiris
> 
>> 
>> Well, actually, it /should/ return the AST for the unary op as usual,
>> but hidden away inside it somewhere should be the AST for the above
>> rewrite. Why both? The AST should accurately reflect the fact that the
>> user wrote it as "++obj.property". However, creating the rewritten AST
>> nodes has the effect of type-checking the expression, AND the rewritten
>> AST can be used for codegen later (as long as it's accessible somehow).
>> 
>> I've been looking into how to generate those AST nodes. First,
>> obj.setProperty gets parsed into a MemberExpr, then, the paren and the
>> arguments are parsed, and the whole thing gets passed to
>> BuildCallToMemberFunction. I figure if we do all of that in the right
>> places and it type checks, we're golden. Stuff the result into the AST
>> somewhere reasonable and that should do it.
>> 
>> So that's my current thinking. This, however, is a radical departure
>> from how properties are currently handled in clang, and will require
>> some pretty invasive surgery.
>> 
>> Thoughts?
>> 
>> -- 
>> Eric Niebler
>> BoostPro Computing
>> http://www.boostpro.com
>> 
>> _______________________________________________
>> cfe-dev mailing list
>> cfe-dev at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>