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

[endlessroad1991 at gmail.com]

Oops... I updated clang to 3.3 trunk, and found another bug...

In Sema::checkPseudoObjectIncDec(), op may be type-dependent, but this fact
can only be discovered if you do a buildGet().
So the first op->isTypeDependent() check will still fail to detect this
fact.
Sema::checkPseudoObjectAssignment() doesn't have this problem.

Override ExprResult Sema::checkPseudoObjectIncDec() to handle this
situation.

Patch & test attached.


On Tue, Apr 16, 2013 at 3:59 PM, endlessroad1991 at gmail.com <
endlessroad1991 at gmail.com> wrote:

> Well, thank you :-)
>
> I'm not planning to work on subscriptable properties, because our project
> doesn't involve that, and I still think subscriptable property is full of
> undocumented behavior and is rarely used.
>
> However, I have another patch to provide support for "for each", also
> well-tested by our codebase. However, I think this feature is not useful,
> but if someone needs it I can post it here.
>
> I want to continue work on clang C++ support on WIndows. I understand that
> the main problem is MSVC ABI. Do you have someone to refer to?
>
> Thanks.
>
>
> On Tue, Apr 16, 2013 at 3:48 PM, John McCall <rjmccall at apple.com> wrote:
>
>> On Apr 15, 2013, at 8:53 PM, endlessroad1991 at gmail.com wrote:
>>
>> Last problem:
>>
>>> 2. In Sema/SemaExpr.cpp, ActOnCallExpr():
>>> I admit that I'm not very thorough about this one...
>>> But here the CheckPlaceholderExpr() call does solve the follwing problem.
>>> === Code Start ===
>>> // If compiled with -ggdb, clang will crash and saying something about
>>> // "unexpected builtin-type" in some CG code.
>>>
>>> struct S2 {
>>> template <class T>
>>> void f(T t) {}
>>> };
>>>
>>> template <class T>
>>> struct S
>>> {
>>> __declspec(property(get=GetV)) int V;
>>> int GetV() { return 123; }
>>> void f() { S2 s2; s2.f(V); }
>>> };
>>>
>>> int main() {
>>> S<int> s;
>>> s.f();
>>> }
>>> === Code End ===
>>>
>>>
>>> Ah, I think the problem here is that overload resolution and especially
>>> template argument deduction aren't lowering out placeholder types.
>>>  Please
>>> hoist this loop out of the C++ block (for obscure reasons; just trust me)
>>> and check:
>>>   hasPlaceholderType(BuiltinType::PseudoObject)
>>> instead of checking for a particular expression kind.
>>>
>>> And please add this as a test case. :)
>>>
>>
>> This change leads to a test failure:
>> test/SemaObjc/property-user-setter.m, Line 88:
>> void g(int); // expected-note {{passing argument to parameter here}}
>> this note is no longer printed.
>> If it's fine, wen can just remove the expected-note comment, but I don't
>> know if it's appropriate.
>>
>>
>> The note's not particularly important, no.  The context of initializing
>> a parameter doesn't really have much to do with the failure to load
>> the property.
>>
>> This looked really good, so I spent a little time tweaking it and
>> then committed it as r179585, thanks!
>>
>> The main changes I made were:
>>   - I tweaked a lot of the diagnostic text to just talk about properties
>> instead of saying __declspec(property).
>>   - I also changed the diagnostic names to make it clear that they were
>> specific to MS properties.
>>   - I reworked the parsing code to avoid some redundant diagnostics and
>> recover nicely in some ways.  For example, if you try to say set=setX,
>> it'll tell you to write put=setX.
>>   - Properties have to be bare identifiers, so there is no point in
>> storing a full DeclarationNameInfo;  it's always just a SourceLocation.
>>
>> Please don't let the number of changes dishearten you;  it was all just
>> small tweaks, and this was really great work!
>>
>> Are you planning to work on subscriptable properties, or was this enough
>> to get your project ported?
>>
>> John.
>>
>
>
>
> --
> Best Regards, Tong Shen (沈彤)
>



-- 
Best Regards, Tong Shen (沈彤)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130416/99bbfa5b/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 1.patch
Type: application/octet-stream
Size: 2367 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130416/99bbfa5b/attachment.obj>