[PATCH] D15174: [MSVC] Fix for http://llvm.org/PR25636: indexed accessor property not supported correctly.

2015-12-02 Thread Alexey Bataev via cfe-commits 
ABataev created this revision.
ABataev added reviewers: rnk, rjmccall.
ABataev added a subscriber: cfe-commits.

All problems described in http://llvm.org/PR25636 are implemented except for 
return value of the 'put' property. This patch fixes this problem with the 
indexed properties

http://reviews.llvm.org/D15174

Files:
  lib/Sema/SemaPseudoObject.cpp
  test/CodeGenCXX/ms-property.cpp
  test/SemaCXX/ms-property-error.cpp
  test/SemaCXX/ms-property.cpp

Index: lib/Sema/SemaPseudoObject.cpp
===
--- lib/Sema/SemaPseudoObject.cpp
+++ lib/Sema/SemaPseudoObject.cpp
@@ -228,6 +228,11 @@
   ResultIndex = Semantics.size() - 1;
 }
 
+void setResultToNextSemantic() {
+  assert(ResultIndex == PseudoObjectExpr::NoResult);
+  ResultIndex = Semantics.size();
+}
+
 /// Return true if assignments have a non-void result.
 bool CanCaptureValue(Expr *exp) {
   if (exp->isGLValue())
@@ -1489,12 +1494,21 @@
 return ExprError();
   }
 
-  SmallVector ArgExprs;
+  SmallVector ArgExprs;
   ArgExprs.append(CallArgs.begin(), CallArgs.end());
   ArgExprs.push_back(op);
-  return S.ActOnCallExpr(S.getCurScope(), SetterExpr.get(),
- RefExpr->getSourceRange().getBegin(), ArgExprs,
- op->getSourceRange().getEnd());
+  ExprResult Res = S.ActOnCallExpr(S.getCurScope(), SetterExpr.get(),
+   RefExpr->getSourceRange().getBegin(),
+   ArgExprs, op->getSourceRange().getEnd());
+
+  if (!Res.isInvalid() && captureSetValueAsResult) {
+Expr *ResExpr = Res.get();
+if (!ResExpr->getType()->isVoidType() &&
+(ResExpr->getType()->isDependentType() || CanCaptureValue(ResExpr)))
+  setResultToNextSemantic();
+  }
+
+  return Res;
 }
 
 //===--===//
Index: test/SemaCXX/ms-property.cpp
===
--- test/SemaCXX/ms-property.cpp
+++ test/SemaCXX/ms-property.cpp
@@ -29,7 +29,7 @@
 public:
   __declspec(property(get=GetX,put=PutX)) T x[];
   T GetX(T i, T j) { return i+j; }
-  void PutX(T i, T j, T k) { j = i = k; }
+  T PutX(T i, T j, T k) { return j = i = k; }
   ~St() { x[0][0] = x[1][1]; }
 };
 
@@ -52,6 +52,8 @@
   ((p2->x)[23])[1] = j1;
   // CHECK-NEXT: ++(((p2->x)[23])[1]);
   ++(((p2->x)[23])[1]);
+  // CHECK-NEXT: j1 = ((p2->x)[23])[1] = j1;
+  j1 = ((p2->x)[23])[1] = j1;
   // CHECK-NEXT: return Test1::GetTest1()->X;
   return Test1::GetTest1()->X;
 }
Index: test/SemaCXX/ms-property-error.cpp
===
--- test/SemaCXX/ms-property-error.cpp
+++ test/SemaCXX/ms-property-error.cpp
@@ -7,17 +7,19 @@
   void PutX(int i, int j, int k) { j = i = k; } // expected-note {{'PutX' declared here}}
 };
 
+char *ptr;
 template 
 class St {
 public:
   __declspec(property(get=GetX,put=PutX)) T x[];
   T GetX(T i, T j) { return i+j; } // expected-note 3 {{'GetX' declared here}}
-  void PutX(T i, T j, T k) { j = i = k; }  // expected-note 2 {{'PutX' declared here}}
+  T PutX(T i, T j, T k) { return j = i = k; }  // expected-note 2 {{'PutX' declared here}}
   ~St() {
 x[1] = 0; // expected-error {{too few arguments to function call, expected 3, have 2}}
 x[2][3] = 4;
 ++x[2][3];
 x[1][2] = x[3][4][5]; // expected-error {{too many arguments to function call, expected 2, have 3}}
+ptr = x[1][2] = x[3][4]; // expected-error {{assigning to 'char *' from incompatible type 'int'}}
   }
 };
 
@@ -30,5 +32,6 @@
   (p1->x[23]) = argc; // expected-error {{too few arguments to function call, expected 3, have 2}}
   float j1 = (p2->x); // expected-error {{too few arguments to function call, expected 2, have 0}}
   ((p2->x)[23])[1][2] = *argv; // expected-error {{too many arguments to function call, expected 3, have 4}}
+  argv = p2->x[11][22] = argc; // expected-error {{assigning to 'char **' from incompatible type 'float'}}
   return ++(((p2->x)[23])); // expected-error {{too few arguments to function call, expected 2, have 1}}
 }
Index: test/CodeGenCXX/ms-property.cpp
===
--- test/CodeGenCXX/ms-property.cpp
+++ test/CodeGenCXX/ms-property.cpp
@@ -31,7 +31,7 @@
   __declspec(property(get=GetX,put=PutX)) T x[];
   T GetX(T i, T j) { return i+j; }
   T GetX() { return 0; }
-  void PutX(T i, T j, T k) { j = i = k; }
+  T PutX(T i, T j, T k) { return j = i = k; }
   __declspec(property(get=GetY,put=PutY)) T y[];
   char GetY(char i,  Test1 j) { return i+j.get_x(); }
   void PutY(char i, int j, double k) { j = i = k; }
@@ -61,14 +61,16 @@
   // CHECK: call float @"\01?GetX@?$St@M@@QEAAMMM@Z"(%class.St* %{{.+}}, float 2.23e+02, float 1.10e+01)
   float j1 = p2->x[223][11];
   // CHECK: [[J1:%.+]] = load float, float* %
-  // CHECK-NEXT: call void @"\01?PutX@?$St@M@@QEAAXMMM@Z

Re: [PATCH] D15174: [MSVC] Fix for http://llvm.org/PR25636: indexed accessor property not supported correctly.

2015-12-03 Thread John McCall via cfe-commits 
rjmccall added a comment.

Hmm.  I think a better approach would be for buildAssignmentOperation to do 
this; but before we figure out how to do that, we should make sure of the 
language semantics we're implementing.  Are the semantics that the result of an 
assignment is always the result of the Put operation, or, if that's void, 
should the result be the assigned value?  And what should happen with 
pre-increment and post-increment?


http://reviews.llvm.org/D15174



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D15174: [MSVC] Fix for http://llvm.org/PR25636: indexed accessor property not supported correctly.

2015-12-04 Thread Alexey Bataev via cfe-commits 
John,
the result is always the result of Put operation. For pre-increment we 
have to return new value, for the post-increment - previous value 
returned by Get (checked it on MSVC).
So, currently postincrement works correctly, pre-increment and 
assignment not. For pre-increment and assignment we have to capture the 
result of Put operation as a result.
It means, that in your solution we need to modify

buildAssignmentOperation() and buildIncDecOperation() and the worst thing is 
that these functions must be changed only for MSPropertySubscriptExpr.

Best regards,
Alexey Bataev
=
Software Engineer
Intel Compiler Team

03.12.2015 20:18, John McCall пишет:
> rjmccall added a comment.
>
> Hmm.  I think a better approach would be for buildAssignmentOperation to do 
> this; but before we figure out how to do that, we should make sure of the 
> language semantics we're implementing.  Are the semantics that the result of 
> an assignment is always the result of the Put operation, or, if that's void, 
> should the result be the assigned value?  And what should happen with 
> pre-increment and post-increment?
>
>
> http://reviews.llvm.org/D15174
>
>
>

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D15174: [MSVC] Fix for http://llvm.org/PR25636: indexed accessor property not supported correctly.

2015-12-04 Thread John McCall via cfe-commits 
rjmccall added a comment.

In http://reviews.llvm.org/D15174#302248, @ABataev wrote:

> John,
>  the result is always the result of Put operation. For pre-increment we 
>  have to return new value, for the post-increment - previous value 
>  returned by Get (checked it on MSVC).
>  So, currently postincrement works correctly, pre-increment and 
>  assignment not. For pre-increment and assignment we have to capture the 
>  result of Put operation as a result.
>  It means, that in your solution we need to modify
>
> buildAssignmentOperation() and buildIncDecOperation() and the worst thing is 
> that these functions must be changed only for MSPropertySubscriptExpr.


Just property subscripts and not MS property references in general?

Fortunately, PseudoOpBuilder is already a dynamic class with 
expression-specific subclasses, so hooking these processes is quite easy; you 
should be able to just add a virtual method which tells them whether the result 
of these compound operations should be the value that's passed to the setter or 
the formal result of the setter.  Something like this:

  virtual bool useSetterResultAsExprResult() const { return false; }

If that returns false, they should try to capture the set value as the result; 
otherwise, they should try to capture the setter result.


http://reviews.llvm.org/D15174



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D15174: [MSVC] Fix for http://llvm.org/PR25636: indexed accessor property not supported correctly.

2015-12-04 Thread Alexey Bataev via cfe-commits 
John,
Actually both, subscripts and references.
Ok, got will do.

Best regards,
Alexey Bataev
=
Software Engineer
Intel Compiler Team

04.12.2015 12:00, John McCall пишет:
> rjmccall added a comment.
>
> In http://reviews.llvm.org/D15174#302248, @ABataev wrote:
>
>> John,
>>   the result is always the result of Put operation. For pre-increment we
>>   have to return new value, for the post-increment - previous value
>>   returned by Get (checked it on MSVC).
>>   So, currently postincrement works correctly, pre-increment and
>>   assignment not. For pre-increment and assignment we have to capture the
>>   result of Put operation as a result.
>>   It means, that in your solution we need to modify
>>
>> buildAssignmentOperation() and buildIncDecOperation() and the worst thing is 
>> that these functions must be changed only for MSPropertySubscriptExpr.
>
> Just property subscripts and not MS property references in general?
>
> Fortunately, PseudoOpBuilder is already a dynamic class with 
> expression-specific subclasses, so hooking these processes is quite easy; you 
> should be able to just add a virtual method which tells them whether the 
> result of these compound operations should be the value that's passed to the 
> setter or the formal result of the setter.  Something like this:
>
>virtual bool useSetterResultAsExprResult() const { return false; }
>
> If that returns false, they should try to capture the set value as the 
> result; otherwise, they should try to capture the setter result.
>
>
> http://reviews.llvm.org/D15174
>
>
>

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D15174: [MSVC] Fix for http://llvm.org/PR25636: indexed accessor property not supported correctly.

2015-12-04 Thread Alexey Bataev via cfe-commits 
ABataev updated this revision to Diff 41848.
ABataev added a comment.

Update after review


http://reviews.llvm.org/D15174

Files:
  lib/Sema/SemaPseudoObject.cpp
  test/CodeGenCXX/ms-property.cpp
  test/SemaCXX/ms-property-error.cpp
  test/SemaCXX/ms-property.cpp

Index: lib/Sema/SemaPseudoObject.cpp
===
--- lib/Sema/SemaPseudoObject.cpp
+++ lib/Sema/SemaPseudoObject.cpp
@@ -229,7 +229,7 @@
 }
 
 /// Return true if assignments have a non-void result.
-bool CanCaptureValue(Expr *exp) {
+bool CanCaptureValue(Expr *exp) const {
   if (exp->isGLValue())
 return true;
   QualType ty = exp->getType();
@@ -245,6 +245,7 @@
 virtual ExprResult buildGet() = 0;
 virtual ExprResult buildSet(Expr *, SourceLocation,
 bool captureSetValueAsResult) = 0;
+virtual bool useSetterResultAsExprResult(Expr *) const { return false; }
   };
 
   /// A PseudoOpBuilder for Objective-C \@properties.
@@ -339,6 +340,7 @@
Expr *rebuildAndCaptureObject(Expr *) override;
ExprResult buildGet() override;
ExprResult buildSet(Expr *op, SourceLocation, bool) override;
+   bool useSetterResultAsExprResult(Expr *) const override;
  };
 }
 
@@ -458,6 +460,8 @@
   result = buildSet(result.get(), opcLoc, /*captureSetValueAsResult*/ true);
   if (result.isInvalid()) return ExprError();
   addSemanticExpr(result.get());
+  if (useSetterResultAsExprResult(result.get()))
+setResultToLastSemantic();
 
   return complete(syntactic);
 }
@@ -502,6 +506,9 @@
   result = buildSet(result.get(), opcLoc, UnaryOperator::isPrefix(opcode));
   if (result.isInvalid()) return ExprError();
   addSemanticExpr(result.get());
+  if (UnaryOperator::isPrefix(opcode) &&
+  useSetterResultAsExprResult(result.get()))
+setResultToLastSemantic();
 
   UnaryOperator *syntactic =
 new (S.Context) UnaryOperator(syntacticOp, opcode, resultType,
@@ -1497,6 +1504,12 @@
  op->getSourceRange().getEnd());
 }
 
+bool MSPropertyOpBuilder::useSetterResultAsExprResult(Expr *ResExpr) const {
+  assert(ResExpr);
+  return !ResExpr->getType()->isVoidType() &&
+ (ResExpr->getType()->isDependentType() || CanCaptureValue(ResExpr));
+}
+
 //===--===//
 //  General Sema routines.
 //===--===//
Index: test/SemaCXX/ms-property.cpp
===
--- test/SemaCXX/ms-property.cpp
+++ test/SemaCXX/ms-property.cpp
@@ -29,7 +29,7 @@
 public:
   __declspec(property(get=GetX,put=PutX)) T x[];
   T GetX(T i, T j) { return i+j; }
-  void PutX(T i, T j, T k) { j = i = k; }
+  T PutX(T i, T j, T k) { return j = i = k; }
   ~St() { x[0][0] = x[1][1]; }
 };
 
@@ -52,6 +52,8 @@
   ((p2->x)[23])[1] = j1;
   // CHECK-NEXT: ++(((p2->x)[23])[1]);
   ++(((p2->x)[23])[1]);
+  // CHECK-NEXT: j1 = ((p2->x)[23])[1] = j1;
+  j1 = ((p2->x)[23])[1] = j1;
   // CHECK-NEXT: return Test1::GetTest1()->X;
   return Test1::GetTest1()->X;
 }
Index: test/SemaCXX/ms-property-error.cpp
===
--- test/SemaCXX/ms-property-error.cpp
+++ test/SemaCXX/ms-property-error.cpp
@@ -7,17 +7,19 @@
   void PutX(int i, int j, int k) { j = i = k; } // expected-note {{'PutX' declared here}}
 };
 
+char *ptr;
 template 
 class St {
 public:
   __declspec(property(get=GetX,put=PutX)) T x[];
   T GetX(T i, T j) { return i+j; } // expected-note 3 {{'GetX' declared here}}
-  void PutX(T i, T j, T k) { j = i = k; }  // expected-note 2 {{'PutX' declared here}}
+  T PutX(T i, T j, T k) { return j = i = k; }  // expected-note 2 {{'PutX' declared here}}
   ~St() {
 x[1] = 0; // expected-error {{too few arguments to function call, expected 3, have 2}}
 x[2][3] = 4;
 ++x[2][3];
 x[1][2] = x[3][4][5]; // expected-error {{too many arguments to function call, expected 2, have 3}}
+ptr = x[1][2] = x[3][4]; // expected-error {{assigning to 'char *' from incompatible type 'int'}}
   }
 };
 
@@ -30,5 +32,6 @@
   (p1->x[23]) = argc; // expected-error {{too few arguments to function call, expected 3, have 2}}
   float j1 = (p2->x); // expected-error {{too few arguments to function call, expected 2, have 0}}
   ((p2->x)[23])[1][2] = *argv; // expected-error {{too many arguments to function call, expected 3, have 4}}
+  argv = p2->x[11][22] = argc; // expected-error {{assigning to 'char **' from incompatible type 'float'}}
   return ++(((p2->x)[23])); // expected-error {{too few arguments to function call, expected 2, have 1}}
 }
Index: test/CodeGenCXX/ms-property.cpp
===
--- test/CodeGenCXX/ms-property.cpp
+++ test/CodeGenCXX/ms-property.cpp
@@ -31,7 +31,7 @@
   __declspec(property(get=GetX,put=PutX)) T x[];
   T GetX(T i, T j) { return i+j; }

Re: [PATCH] D15174: [MSVC] Fix for http://llvm.org/PR25636: indexed accessor property not supported correctly.

2015-12-04 Thread John McCall via cfe-commits 
rjmccall added inline comments.


Comment at: lib/Sema/SemaPseudoObject.cpp:232
@@ -231,3 +231,3 @@
 /// Return true if assignments have a non-void result.
-bool CanCaptureValue(Expr *exp) {
+bool CanCaptureValue(Expr *exp) const {
   if (exp->isGLValue())

Just make this static.


Comment at: lib/Sema/SemaPseudoObject.cpp:464
@@ -461,1 +463,3 @@
+  if (useSetterResultAsExprResult(result.get()))
+setResultToLastSemantic();
 

This will leave the result as the captured set value if it can't capture the 
setter result.  Is that desired?


http://reviews.llvm.org/D15174



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D15174: [MSVC] Fix for http://llvm.org/PR25636: indexed accessor property not supported correctly.

2015-12-06 Thread Alexey Bataev via cfe-commits 
ABataev marked 2 inline comments as done.


Comment at: lib/Sema/SemaPseudoObject.cpp:232
@@ -231,3 +231,3 @@
 /// Return true if assignments have a non-void result.
-bool CanCaptureValue(Expr *exp) {
+bool CanCaptureValue(Expr *exp) const {
   if (exp->isGLValue())

rjmccall wrote:
> Just make this static.
Ok


Comment at: lib/Sema/SemaPseudoObject.cpp:464
@@ -461,1 +463,3 @@
+  if (useSetterResultAsExprResult(result.get()))
+setResultToLastSemantic();
 

rjmccall wrote:
> This will leave the result as the captured set value if it can't capture the 
> setter result.  Is that desired?
The fact that the value can be captured must be checked in 
useSetterResultAsExprResult()


http://reviews.llvm.org/D15174



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D15174: [MSVC] Fix for http://llvm.org/PR25636: indexed accessor property not supported correctly.

2015-12-06 Thread Alexey Bataev via cfe-commits 
ABataev updated this revision to Diff 42026.
ABataev marked 2 inline comments as done.
ABataev added a comment.

Update after review


http://reviews.llvm.org/D15174

Files:
  lib/Sema/SemaPseudoObject.cpp
  test/CodeGenCXX/ms-property.cpp
  test/SemaCXX/ms-property-error.cpp
  test/SemaCXX/ms-property.cpp

Index: lib/Sema/SemaPseudoObject.cpp
===
--- lib/Sema/SemaPseudoObject.cpp
+++ lib/Sema/SemaPseudoObject.cpp
@@ -229,7 +229,7 @@
 }
 
 /// Return true if assignments have a non-void result.
-bool CanCaptureValue(Expr *exp) {
+static bool CanCaptureValue(Expr *exp) {
   if (exp->isGLValue())
 return true;
   QualType ty = exp->getType();
@@ -245,6 +245,7 @@
 virtual ExprResult buildGet() = 0;
 virtual ExprResult buildSet(Expr *, SourceLocation,
 bool captureSetValueAsResult) = 0;
+virtual bool useSetterResultAsExprResult(Expr *) const { return false; }
   };
 
   /// A PseudoOpBuilder for Objective-C \@properties.
@@ -339,6 +340,7 @@
Expr *rebuildAndCaptureObject(Expr *) override;
ExprResult buildGet() override;
ExprResult buildSet(Expr *op, SourceLocation, bool) override;
+   bool useSetterResultAsExprResult(Expr *) const override;
  };
 }
 
@@ -458,6 +460,8 @@
   result = buildSet(result.get(), opcLoc, /*captureSetValueAsResult*/ true);
   if (result.isInvalid()) return ExprError();
   addSemanticExpr(result.get());
+  if (useSetterResultAsExprResult(result.get()))
+setResultToLastSemantic();
 
   return complete(syntactic);
 }
@@ -502,6 +506,9 @@
   result = buildSet(result.get(), opcLoc, UnaryOperator::isPrefix(opcode));
   if (result.isInvalid()) return ExprError();
   addSemanticExpr(result.get());
+  if (UnaryOperator::isPrefix(opcode) &&
+  useSetterResultAsExprResult(result.get()))
+setResultToLastSemantic();
 
   UnaryOperator *syntactic =
 new (S.Context) UnaryOperator(syntacticOp, opcode, resultType,
@@ -1497,6 +1504,12 @@
  op->getSourceRange().getEnd());
 }
 
+bool MSPropertyOpBuilder::useSetterResultAsExprResult(Expr *ResExpr) const {
+  assert(ResExpr);
+  return !ResExpr->getType()->isVoidType() &&
+ (ResExpr->getType()->isDependentType() || CanCaptureValue(ResExpr));
+}
+
 //===--===//
 //  General Sema routines.
 //===--===//
Index: test/SemaCXX/ms-property.cpp
===
--- test/SemaCXX/ms-property.cpp
+++ test/SemaCXX/ms-property.cpp
@@ -29,7 +29,7 @@
 public:
   __declspec(property(get=GetX,put=PutX)) T x[];
   T GetX(T i, T j) { return i+j; }
-  void PutX(T i, T j, T k) { j = i = k; }
+  T PutX(T i, T j, T k) { return j = i = k; }
   ~St() { x[0][0] = x[1][1]; }
 };
 
@@ -52,6 +52,8 @@
   ((p2->x)[23])[1] = j1;
   // CHECK-NEXT: ++(((p2->x)[23])[1]);
   ++(((p2->x)[23])[1]);
+  // CHECK-NEXT: j1 = ((p2->x)[23])[1] = j1;
+  j1 = ((p2->x)[23])[1] = j1;
   // CHECK-NEXT: return Test1::GetTest1()->X;
   return Test1::GetTest1()->X;
 }
Index: test/SemaCXX/ms-property-error.cpp
===
--- test/SemaCXX/ms-property-error.cpp
+++ test/SemaCXX/ms-property-error.cpp
@@ -7,17 +7,19 @@
   void PutX(int i, int j, int k) { j = i = k; } // expected-note {{'PutX' declared here}}
 };
 
+char *ptr;
 template 
 class St {
 public:
   __declspec(property(get=GetX,put=PutX)) T x[];
   T GetX(T i, T j) { return i+j; } // expected-note 3 {{'GetX' declared here}}
-  void PutX(T i, T j, T k) { j = i = k; }  // expected-note 2 {{'PutX' declared here}}
+  T PutX(T i, T j, T k) { return j = i = k; }  // expected-note 2 {{'PutX' declared here}}
   ~St() {
 x[1] = 0; // expected-error {{too few arguments to function call, expected 3, have 2}}
 x[2][3] = 4;
 ++x[2][3];
 x[1][2] = x[3][4][5]; // expected-error {{too many arguments to function call, expected 2, have 3}}
+ptr = x[1][2] = x[3][4]; // expected-error {{assigning to 'char *' from incompatible type 'int'}}
   }
 };
 
@@ -30,5 +32,6 @@
   (p1->x[23]) = argc; // expected-error {{too few arguments to function call, expected 3, have 2}}
   float j1 = (p2->x); // expected-error {{too few arguments to function call, expected 2, have 0}}
   ((p2->x)[23])[1][2] = *argv; // expected-error {{too many arguments to function call, expected 3, have 4}}
+  argv = p2->x[11][22] = argc; // expected-error {{assigning to 'char **' from incompatible type 'float'}}
   return ++(((p2->x)[23])); // expected-error {{too few arguments to function call, expected 2, have 1}}
 }
Index: test/CodeGenCXX/ms-property.cpp
===
--- test/CodeGenCXX/ms-property.cpp
+++ test/CodeGenCXX/ms-property.cpp
@@ -31,7 +31,7 @@
   __declspec(property(get=GetX,put=PutX)) T

Re: [PATCH] D15174: [MSVC] Fix for http://llvm.org/PR25636: indexed accessor property not supported correctly.

2015-12-06 Thread John McCall via cfe-commits 
rjmccall added inline comments.


Comment at: lib/Sema/SemaPseudoObject.cpp:464
@@ -461,1 +463,3 @@
+  if (useSetterResultAsExprResult(result.get()))
+setResultToLastSemantic();
 

Sure, but that's not what I'm saying.  I'm saying that the code, as you've 
written it, will use the value passed to the setter (if capturable) as the 
result of the expression if it can't capture the setter result.

That is, given:

  struct A {
__declspec(property(get=GetX,put=PutX)) int x;
int GetX() const { return 0; }
void SetX(long y) {}
  };

this expression will have type long instead of void, because it's implicitly 
falling back on capturing the value passed to the setter:

  a.x = 5;

I suspect that that's not the MSVC semantics, and the language rule is that you 
should always use the setter result.  If that's true, you should be asking the 
subclass which rule to use abstractly, i.e. before constructing the setter, and 
then (1) suppressing the capture of the set value and (2) using the setter 
result (if capturable) or else nothing.


http://reviews.llvm.org/D15174



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D15174: [MSVC] Fix for http://llvm.org/PR25636: indexed accessor property not supported correctly.

2015-12-06 Thread Alexey Bataev via cfe-commits 
John,
Your example won't be compiled at all. Clang and MSVC emit error 
messages in this case.
The next code

  struct A {
 __declspec(property(get=GetX,put=SetX)) int x;
 int GetX() const { return 0; }
 void SetX(long y) {}
   };

a.x = 5;
compiled fine by MSVC and clang and provides correct result on clang 
(i.e. the result value can't be captured at all, because SetX() returns 
void)
return a.x = 5;
clang: error: cannot initialize return object of type 'int' with an 
rvalue of type 'void'
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D15174: [MSVC] Fix for http://llvm.org/PR25636: indexed accessor property not supported correctly.

2015-12-07 Thread John McCall via cfe-commits 
rjmccall added a comment.

I don't understand why that's true.  buildSet is called with 
captureSetValueAsResult=true, and the set value is definitely capturable, so 
that value should be set as the result; and you're not overriding it.  Why does 
the expression end up having type void?


http://reviews.llvm.org/D15174



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D15174: [MSVC] Fix for http://llvm.org/PR25636: indexed accessor property not supported correctly.

2015-12-07 Thread Alexey Bataev via cfe-commits 
Set value is a call to SetX() function, neither the argument of the 
SetX(), nor the result of the GetX(). So, expression a.x=5 always emits 
the following code as a result a.Setx(5). 
MSPropertyRefBuilder::buildSet() does not capture results at all. It 
just ignores

captureSetValueAsResult

value. That's why we have to capture final result after 'set' is built 
completely.

Best regards,
Alexey Bataev
=
Software Engineer
Intel Compiler Team

07.12.2015 11:07, John McCall пишет:
> rjmccall added a comment.
>
> I don't understand why that's true.  buildSet is called with 
> captureSetValueAsResult=true, and the set value is definitely capturable, so 
> that value should be set as the result; and you're not overriding it.  Why 
> does the expression end up having type void?
>
>
> http://reviews.llvm.org/D15174
>
>
>

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D15174: [MSVC] Fix for http://llvm.org/PR25636: indexed accessor property not supported correctly.

2015-12-07 Thread John McCall via cfe-commits 
rjmccall added a comment.

Oh, I see, of course.

It would be clearer if you asked the subclass which value to use abstractly 
(i.e. independently of the actual set expression) and then passed down 
captureSetValueAsResult=false when the subclass says to use the setter result.


http://reviews.llvm.org/D15174



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D15174: [MSVC] Fix for http://llvm.org/PR25636: indexed accessor property not supported correctly.

2015-12-07 Thread Alexey Bataev via cfe-commits 
Ok, I will implement this logic

Best regards,
Alexey Bataev
=
Software Engineer
Intel Compiler Team

07.12.2015 22:30, John McCall пишет:
> rjmccall added a comment.
>
> Oh, I see, of course.
>
> It would be clearer if you asked the subclass which value to use abstractly 
> (i.e. independently of the actual set expression) and then passed down 
> captureSetValueAsResult=false when the subclass says to use the setter result.
>
>
> http://reviews.llvm.org/D15174
>
>
>

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D15174: [MSVC] Fix for http://llvm.org/PR25636: indexed accessor property not supported correctly.

2015-12-07 Thread Alexey Bataev via cfe-commits 
ABataev updated this revision to Diff 42140.
ABataev added a comment.

Update after review


http://reviews.llvm.org/D15174

Files:
  lib/Sema/SemaPseudoObject.cpp
  test/CodeGenCXX/ms-property.cpp
  test/SemaCXX/ms-property-error.cpp
  test/SemaCXX/ms-property.cpp

Index: lib/Sema/SemaPseudoObject.cpp
===
--- lib/Sema/SemaPseudoObject.cpp
+++ lib/Sema/SemaPseudoObject.cpp
@@ -229,7 +229,7 @@
 }
 
 /// Return true if assignments have a non-void result.
-bool CanCaptureValue(Expr *exp) {
+static bool CanCaptureValue(Expr *exp) {
   if (exp->isGLValue())
 return true;
   QualType ty = exp->getType();
@@ -245,6 +245,8 @@
 virtual ExprResult buildGet() = 0;
 virtual ExprResult buildSet(Expr *, SourceLocation,
 bool captureSetValueAsResult) = 0;
+virtual bool useSetterResultAsExprResult(Expr *) const { return false; }
+virtual bool captureSetValueAsResult() const { return true; }
   };
 
   /// A PseudoOpBuilder for Objective-C \@properties.
@@ -339,6 +341,8 @@
Expr *rebuildAndCaptureObject(Expr *) override;
ExprResult buildGet() override;
ExprResult buildSet(Expr *op, SourceLocation, bool) override;
+   bool useSetterResultAsExprResult(Expr *) const override;
+   bool captureSetValueAsResult() const override { return false; }
  };
 }
 
@@ -455,9 +459,11 @@
 
   // The result of the assignment, if not void, is the value set into
   // the l-value.
-  result = buildSet(result.get(), opcLoc, /*captureSetValueAsResult*/ true);
+  result = buildSet(result.get(), opcLoc, captureSetValueAsResult());
   if (result.isInvalid()) return ExprError();
   addSemanticExpr(result.get());
+  if (useSetterResultAsExprResult(result.get()))
+setResultToLastSemantic();
 
   return complete(syntactic);
 }
@@ -499,9 +505,13 @@
 
   // Store that back into the result.  The value stored is the result
   // of a prefix operation.
-  result = buildSet(result.get(), opcLoc, UnaryOperator::isPrefix(opcode));
+  result = buildSet(result.get(), opcLoc, UnaryOperator::isPrefix(opcode) &&
+  captureSetValueAsResult());
   if (result.isInvalid()) return ExprError();
   addSemanticExpr(result.get());
+  if (UnaryOperator::isPrefix(opcode) &&
+  useSetterResultAsExprResult(result.get()))
+setResultToLastSemantic();
 
   UnaryOperator *syntactic =
 new (S.Context) UnaryOperator(syntacticOp, opcode, resultType,
@@ -1497,6 +1507,12 @@
  op->getSourceRange().getEnd());
 }
 
+bool MSPropertyOpBuilder::useSetterResultAsExprResult(Expr *ResExpr) const {
+  assert(ResExpr);
+  return !ResExpr->getType()->isVoidType() &&
+ (ResExpr->getType()->isDependentType() || CanCaptureValue(ResExpr));
+}
+
 //===--===//
 //  General Sema routines.
 //===--===//
Index: test/SemaCXX/ms-property.cpp
===
--- test/SemaCXX/ms-property.cpp
+++ test/SemaCXX/ms-property.cpp
@@ -29,7 +29,7 @@
 public:
   __declspec(property(get=GetX,put=PutX)) T x[];
   T GetX(T i, T j) { return i+j; }
-  void PutX(T i, T j, T k) { j = i = k; }
+  T PutX(T i, T j, T k) { return j = i = k; }
   ~St() { x[0][0] = x[1][1]; }
 };
 
@@ -52,6 +52,8 @@
   ((p2->x)[23])[1] = j1;
   // CHECK-NEXT: ++(((p2->x)[23])[1]);
   ++(((p2->x)[23])[1]);
+  // CHECK-NEXT: j1 = ((p2->x)[23])[1] = j1;
+  j1 = ((p2->x)[23])[1] = j1;
   // CHECK-NEXT: return Test1::GetTest1()->X;
   return Test1::GetTest1()->X;
 }
Index: test/SemaCXX/ms-property-error.cpp
===
--- test/SemaCXX/ms-property-error.cpp
+++ test/SemaCXX/ms-property-error.cpp
@@ -7,17 +7,19 @@
   void PutX(int i, int j, int k) { j = i = k; } // expected-note {{'PutX' declared here}}
 };
 
+char *ptr;
 template 
 class St {
 public:
   __declspec(property(get=GetX,put=PutX)) T x[];
   T GetX(T i, T j) { return i+j; } // expected-note 3 {{'GetX' declared here}}
-  void PutX(T i, T j, T k) { j = i = k; }  // expected-note 2 {{'PutX' declared here}}
+  T PutX(T i, T j, T k) { return j = i = k; }  // expected-note 2 {{'PutX' declared here}}
   ~St() {
 x[1] = 0; // expected-error {{too few arguments to function call, expected 3, have 2}}
 x[2][3] = 4;
 ++x[2][3];
 x[1][2] = x[3][4][5]; // expected-error {{too many arguments to function call, expected 2, have 3}}
+ptr = x[1][2] = x[3][4]; // expected-error {{assigning to 'char *' from incompatible type 'int'}}
   }
 };
 
@@ -30,5 +32,6 @@
   (p1->x[23]) = argc; // expected-error {{too few arguments to function call, expected 3, have 2}}
   float j1 = (p2->x); // expected-error {{too few arguments to function call, expected 2, have 0}}
   ((p2->x)[23])[1][2] = *argv; // expected-error {{too

Re: [PATCH] D15174: [MSVC] Fix for http://llvm.org/PR25636: indexed accessor property not supported correctly.

2015-12-08 Thread John McCall via cfe-commits 
rjmccall added a comment.

Thanks!



Comment at: lib/Sema/SemaPseudoObject.cpp:249
@@ -248,1 +248,3 @@
+virtual bool useSetterResultAsExprResult(Expr *) const { return false; }
+virtual bool captureSetValueAsResult() const { return true; }
   };

I think you just need one of these.  If useSetterResultAsExprResult() returns 
true, buildAssignmentOperation and buildIncDecOperation should try to capture 
the setter result; otherwise, they should try to capture the set value.  And it 
doesn't need to take an Expr* anymore.

Please add a comment explaining that; something like this:

  /// Should the result of an assignment be the formal result of the setter
  /// call or the value that was passed to the setter?
  ///
  /// Different pseudo-object language features use different language rules 
for this.
  /// The default is to use the set value.  Currently, this affects the 
behavior of simple
  /// assignments, compound assignments, and prefix increment and decrement.
  /// Postfix increment and decrement always use the getter result as the 
expression
  /// result.
  ///
  /// If this method returns false, and the set value isn't capturable for some
  /// reason, the result of the expression will be void.


http://reviews.llvm.org/D15174



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D15174: [MSVC] Fix for http://llvm.org/PR25636: indexed accessor property not supported correctly.

2015-12-09 Thread Alexey Bataev via cfe-commits 
ABataev updated this revision to Diff 42282.
ABataev added a comment.

Update after review


http://reviews.llvm.org/D15174

Files:
  lib/Sema/SemaPseudoObject.cpp
  test/CodeGenCXX/ms-property.cpp
  test/SemaCXX/ms-property-error.cpp
  test/SemaCXX/ms-property.cpp

Index: lib/Sema/SemaPseudoObject.cpp
===
--- lib/Sema/SemaPseudoObject.cpp
+++ lib/Sema/SemaPseudoObject.cpp
@@ -229,7 +229,7 @@
 }
 
 /// Return true if assignments have a non-void result.
-bool CanCaptureValue(Expr *exp) {
+static bool CanCaptureValue(Expr *exp) {
   if (exp->isGLValue())
 return true;
   QualType ty = exp->getType();
@@ -245,6 +245,20 @@
 virtual ExprResult buildGet() = 0;
 virtual ExprResult buildSet(Expr *, SourceLocation,
 bool captureSetValueAsResult) = 0;
+/// \brief Should the result of an assignment be the formal result of the
+/// setter call or the value that was passed to the setter?
+///
+/// Different pseudo-object language features use different language rules
+/// for this.
+/// The default is to use the set value.  Currently, this affects the
+/// behavior of simple assignments, compound assignments, and prefix
+/// increment and decrement.
+/// Postfix increment and decrement always use the getter result as the
+/// expression result.
+///
+/// If this method returns false, and the set value isn't capturable for
+/// some reason, the result of the expression will be void.
+virtual bool captureSetValueAsResult() const { return true; }
   };
 
   /// A PseudoOpBuilder for Objective-C \@properties.
@@ -339,6 +353,7 @@
Expr *rebuildAndCaptureObject(Expr *) override;
ExprResult buildGet() override;
ExprResult buildSet(Expr *op, SourceLocation, bool) override;
+   bool captureSetValueAsResult() const override { return false; }
  };
 }
 
@@ -455,9 +470,12 @@
 
   // The result of the assignment, if not void, is the value set into
   // the l-value.
-  result = buildSet(result.get(), opcLoc, /*captureSetValueAsResult*/ true);
+  result = buildSet(result.get(), opcLoc, captureSetValueAsResult());
   if (result.isInvalid()) return ExprError();
   addSemanticExpr(result.get());
+  if (!captureSetValueAsResult() && !result.get()->getType()->isVoidType() &&
+  (result.get()->isTypeDependent() || CanCaptureValue(result.get(
+setResultToLastSemantic();
 
   return complete(syntactic);
 }
@@ -499,9 +517,14 @@
 
   // Store that back into the result.  The value stored is the result
   // of a prefix operation.
-  result = buildSet(result.get(), opcLoc, UnaryOperator::isPrefix(opcode));
+  result = buildSet(result.get(), opcLoc, UnaryOperator::isPrefix(opcode) &&
+  captureSetValueAsResult());
   if (result.isInvalid()) return ExprError();
   addSemanticExpr(result.get());
+  if (UnaryOperator::isPrefix(opcode) && !captureSetValueAsResult() &&
+  !result.get()->getType()->isVoidType() &&
+  (result.get()->isTypeDependent() || CanCaptureValue(result.get(
+setResultToLastSemantic();
 
   UnaryOperator *syntactic =
 new (S.Context) UnaryOperator(syntacticOp, opcode, resultType,
Index: test/SemaCXX/ms-property.cpp
===
--- test/SemaCXX/ms-property.cpp
+++ test/SemaCXX/ms-property.cpp
@@ -29,7 +29,7 @@
 public:
   __declspec(property(get=GetX,put=PutX)) T x[];
   T GetX(T i, T j) { return i+j; }
-  void PutX(T i, T j, T k) { j = i = k; }
+  T PutX(T i, T j, T k) { return j = i = k; }
   ~St() { x[0][0] = x[1][1]; }
 };
 
@@ -52,6 +52,8 @@
   ((p2->x)[23])[1] = j1;
   // CHECK-NEXT: ++(((p2->x)[23])[1]);
   ++(((p2->x)[23])[1]);
+  // CHECK-NEXT: j1 = ((p2->x)[23])[1] = j1;
+  j1 = ((p2->x)[23])[1] = j1;
   // CHECK-NEXT: return Test1::GetTest1()->X;
   return Test1::GetTest1()->X;
 }
Index: test/SemaCXX/ms-property-error.cpp
===
--- test/SemaCXX/ms-property-error.cpp
+++ test/SemaCXX/ms-property-error.cpp
@@ -7,17 +7,19 @@
   void PutX(int i, int j, int k) { j = i = k; } // expected-note {{'PutX' declared here}}
 };
 
+char *ptr;
 template 
 class St {
 public:
   __declspec(property(get=GetX,put=PutX)) T x[];
   T GetX(T i, T j) { return i+j; } // expected-note 3 {{'GetX' declared here}}
-  void PutX(T i, T j, T k) { j = i = k; }  // expected-note 2 {{'PutX' declared here}}
+  T PutX(T i, T j, T k) { return j = i = k; }  // expected-note 2 {{'PutX' declared here}}
   ~St() {
 x[1] = 0; // expected-error {{too few arguments to function call, expected 3, have 2}}
 x[2][3] = 4;
 ++x[2][3];
 x[1][2] = x[3][4][5]; // expected-error {{too many arguments to function call, expected 2, have 3}}
+ptr = x[1][2] = x[3][4]; // expected-error {{assigning to 'char *' from incompatible type 'int'}}
   }
 };
 
@@ -30,5 +32,6 @@

Re: [PATCH] D15174: [MSVC] Fix for http://llvm.org/PR25636: indexed accessor property not supported correctly.

2015-12-09 Thread John McCall via cfe-commits 
rjmccall added a comment.

Thanks!  Just a slight tweak to the comment, then LGTM.



Comment at: lib/Sema/SemaPseudoObject.cpp:259
@@ +258,3 @@
+///
+/// If this method returns false, and the set value isn't capturable for
+/// some reason, the result of the expression will be void.

"If this method returns *true*", since you picked this one.


http://reviews.llvm.org/D15174



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D15174: [MSVC] Fix for http://llvm.org/PR25636: indexed accessor property not supported correctly.

2015-12-09 Thread Alexey Bataev via cfe-commits 
ABataev marked an inline comment as done.


Comment at: lib/Sema/SemaPseudoObject.cpp:259
@@ +258,3 @@
+///
+/// If this method returns false, and the set value isn't capturable for
+/// some reason, the result of the expression will be void.

rjmccall wrote:
> "If this method returns *true*", since you picked this one.
Oops, thanks


http://reviews.llvm.org/D15174



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D15174: [MSVC] Fix for http://llvm.org/PR25636: indexed accessor property not supported correctly.

2015-12-09 Thread Alexey Bataev via cfe-commits 
This revision was automatically updated to reflect the committed changes.
ABataev marked an inline comment as done.
Closed by commit rL255218: [MSVC] Fix for http://llvm.org/PR25636: indexed 
accessor property not… (authored by ABataev).

Changed prior to commit:
  http://reviews.llvm.org/D15174?vs=42282&id=42373#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D15174

Files:
  cfe/trunk/lib/Sema/SemaPseudoObject.cpp
  cfe/trunk/test/CodeGenCXX/ms-property.cpp
  cfe/trunk/test/SemaCXX/ms-property-error.cpp
  cfe/trunk/test/SemaCXX/ms-property.cpp

Index: cfe/trunk/test/CodeGenCXX/ms-property.cpp
===
--- cfe/trunk/test/CodeGenCXX/ms-property.cpp
+++ cfe/trunk/test/CodeGenCXX/ms-property.cpp
@@ -31,7 +31,7 @@
   __declspec(property(get=GetX,put=PutX)) T x[];
   T GetX(T i, T j) { return i+j; }
   T GetX() { return 0; }
-  void PutX(T i, T j, T k) { j = i = k; }
+  T PutX(T i, T j, T k) { return j = i = k; }
   __declspec(property(get=GetY,put=PutY)) T y[];
   char GetY(char i,  Test1 j) { return i+j.get_x(); }
   void PutY(char i, int j, double k) { j = i = k; }
@@ -61,14 +61,16 @@
   // CHECK: call float @"\01?GetX@?$St@M@@QEAAMMM@Z"(%class.St* %{{.+}}, float 2.23e+02, float 1.10e+01)
   float j1 = p2->x[223][11];
   // CHECK: [[J1:%.+]] = load float, float* %
-  // CHECK-NEXT: call void @"\01?PutX@?$St@M@@QEAAXMMM@Z"(%class.St* %{{.+}}, float 2.30e+01, float 1.00e+00, float [[J1]])
-  p2->x[23][1] = j1;
+  // CHECK-NEXT: [[CALL:%.+]] = call float @"\01?PutX@?$St@M@@QEAA@Z"(%class.St* %{{.+}}, float 2.30e+01, float 1.00e+00, float [[J1]])
+  // CHECK-NEXT: [[CONV:%.+]] = fptosi float [[CALL]] to i32
+  // CHECK-NEXT: store i32 [[CONV]], i32*
+  argc = p2->x[23][1] = j1;
   // CHECK: [[IDX:%.+]] = call i32 @"\01?idx@@YAHXZ"()
   // CHECK-NEXT: [[CONV:%.+]] = sitofp i32 [[IDX]] to float
   // CHECK-NEXT: [[GET:%.+]] = call float @"\01?GetX@?$St@M@@QEAAMMM@Z"(%class.St* %{{.+}}, float [[CONV]], float 1.00e+00)
   // CHECK-NEXT: [[INC:%.+]] = fadd float [[GET]], 1.00e+00
   // CHECK-NEXT: [[CONV:%.+]] = sitofp i32 [[IDX]] to float
-  // CHECK-NEXT: call void @"\01?PutX@?$St@M@@QEAAXMMM@Z"(%class.St* %{{.+}}, float [[CONV]], float 1.00e+00, float [[INC]])
+  // CHECK-NEXT: call float @"\01?PutX@?$St@M@@QEAA@Z"(%class.St* %{{.+}}, float [[CONV]], float 1.00e+00, float [[INC]])
   ++p2->x[idx()][1];
   // CHECK: call void @"\01??$foo@H@@YAXHH@Z"(i32 %{{.+}}, i32 %{{.+}})
   foo(argc, (int)argv[0][0]);
@@ -93,19 +95,19 @@
   // CHECK: [[CAST1:%.+]] = sitofp i32 [[J]] to float
   // CHECK: [[J:%.+]] = load i32, i32* %
   // CHECK: [[CAST2:%.+]] = sitofp i32 [[J]] to float
-  // CHECK: call void @"\01?PutX@?$St@M@@QEAAXMMM@Z"(%class.St* [[P2_1]], float [[CAST2]], float [[CAST1]], float [[CAST]])
+  // CHECK: call float @"\01?PutX@?$St@M@@QEAA@Z"(%class.St* [[P2_1]], float [[CAST2]], float [[CAST1]], float [[CAST]])
   p2->x[j][j] = p2->y[p1->x[argc][0]][t];
   // CHECK: [[CALL:%.+]] = call %class.Test1* @"\01?GetTest1@Test1@@SAPEAV1@XZ"()
   // CHECK-NEXT: call i32 @"\01?get_x@Test1@@QEBAHXZ"(%class.Test1* [[CALL]])
   return Test1::GetTest1()->X;
 }
 
 // CHECK: define linkonce_odr void @"\01??$foo@H@@YAXHH@Z"(i32 %{{.+}}, i32 %{{.+}})
 // CHECK: call i32 @"\01?GetX@?$St@H@@QEAAHHH@Z"(%class.St{{.+}}* [[BAR:%.+]], i32 %{{.+}} i32 %{{.+}})
-// CHECK: call void @"\01?PutX@?$St@H@@QEAAXHHH@Z"(%class.St{{.+}}* [[BAR]], i32 %{{.+}}, i32 %{{.+}}, i32 %{{.+}})
+// CHECK: call i32 @"\01?PutX@?$St@H@@QEAA@Z"(%class.St{{.+}}* [[BAR]], i32 %{{.+}}, i32 %{{.+}}, i32 %{{.+}})
 // CHECK: call i32 @"\01?GetX@?$St@H@@QEAAHHH@Z"(%class.St{{.+}}* [[BAR]], i32 %{{.+}} i32 %{{.+}})
 // CHECK: call void @"\01?PutY@?$St@H@@QEAAXDHN@Z"(%class.St{{.+}}* [[BAR]], i8 %{{.+}}, i32 %{{.+}}, double %{{.+}}
 // CHECK: call i32 @"\01?GetX@?$St@H@@QEAAHHH@Z"(%class.St{{.+}}* [[BAR]], i32 %{{.+}} i32 %{{.+}})
 // CHECK: call i8 @"\01?GetY@?$St@H@@QEAADDVTest1@@@Z"(%class.St{{.+}}* [[BAR]], i8 %{{.+}}, %class.Test1* %{{.+}})
-// CHECK: call void @"\01?PutX@?$St@H@@QEAAXHHH@Z"(%class.St{{.+}}* [[BAR]], i32 %{{.+}}, i32 %{{.+}}, i32 %{{.+}})
+// CHECK: call i32 @"\01?PutX@?$St@H@@QEAA@Z"(%class.St{{.+}}* [[BAR]], i32 %{{.+}}, i32 %{{.+}}, i32 %{{.+}})
 #endif //HEADER
Index: cfe/trunk/test/SemaCXX/ms-property-error.cpp
===
--- cfe/trunk/test/SemaCXX/ms-property-error.cpp
+++ cfe/trunk/test/SemaCXX/ms-property-error.cpp
@@ -7,17 +7,19 @@
   void PutX(int i, int j, int k) { j = i = k; } // expected-note {{'PutX' declared here}}
 };
 
+char *ptr;
 template 
 class St {
 public:
   __declspec(property(get=GetX,put=PutX)) T x[];
   T GetX(T i, T j) { return i+j; } // expected-note 3 {{'GetX' declared here}}
-  void PutX(T i, T j, T k) { j = i = k; }  // expected-note 2 {{'PutX' declared here}}
+  T PutX(T i, T j, T k) { return j = i = k; }  // expected-note 2 {{'PutX'