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

[ABataev]

Summary

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

[rjmccall]

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?

[ABataev]

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

[rjmccall]

In 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.

[ABataev]

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

[ABataev]

Update after review

[rjmccall]

lib/Sema/SemaPseudoObject.cpp

232 ↗

(On Diff #41848)

Just make this static.

464 ↗

(On Diff #41848)

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

[ABataev]

lib/Sema/SemaPseudoObject.cpp

232 ↗

(On Diff #41848)

Ok

464 ↗

(On Diff #41848)

The fact that the value can be captured must be checked in useSetterResultAsExprResult()

[ABataev]

Update after review

[rjmccall]

lib/Sema/SemaPseudoObject.cpp

464 ↗

(On Diff #42026)

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.

[ABataev]

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'

[rjmccall]

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?

[ABataev]

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

[rjmccall]

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.

[ABataev]

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

[ABataev]

Update after review

[rjmccall]

Thanks!

lib/Sema/SemaPseudoObject.cpp

249 ↗

(On Diff #42140)

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.

[ABataev]

Update after review

[rjmccall]

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

lib/Sema/SemaPseudoObject.cpp

259 ↗

(On Diff #42282)

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

[ABataev]

lib/Sema/SemaPseudoObject.cpp

259 ↗

(On Diff #42282)

Oops, thanks

cfe/trunk/lib/Sema/SemaPseudoObject.cpp

public:
     OpaqueValueExpr *captureValueAsResult(Expr *op);
 
     void setResultToLastSemantic() {
       assert(ResultIndex == PseudoObjectExpr::NoResult);
       ResultIndex = Semantics.size() - 1;
     }
 
     /// 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();
       assert(!ty->isIncompleteType());
       assert(!ty->isDependentType());
 
       if (const CXXRecordDecl *ClassDecl = ty->getAsCXXRecordDecl())
         return ClassDecl->isTriviallyCopyable();
       return true;
     }
 
     virtual Expr *rebuildAndCaptureObject(Expr *) = 0;
     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 true, 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.
   class ObjCPropertyOpBuilder : public PseudoOpBuilder {
     ObjCPropertyRefExpr *RefExpr;
     ObjCPropertyRefExpr *SyntacticRefExpr;
     OpaqueValueExpr *InstanceReceiver;
     ObjCMethodDecl *Getter;
MSPropertyOpBuilder(Sema &S, MSPropertySubscriptExpr *refExpr)
        : PseudoOpBuilder(S, refExpr->getSourceRange().getBegin()),
          InstanceBase(nullptr) {
      RefExpr = getBaseMSProperty(refExpr);
    }
 
    Expr *rebuildAndCaptureObject(Expr *) override;
    ExprResult buildGet() override;
    ExprResult buildSet(Expr *op, SourceLocation, bool) override;
+   bool captureSetValueAsResult() const override { return false; }
  };
 }
 
 /// Capture the given expression in an OpaqueValueExpr.
 OpaqueValueExpr *PseudoOpBuilder::capture(Expr *e) {
   // Make a new OVE whose source is the given expression.
   OpaqueValueExpr *captured = 
     new (S.Context) OpaqueValueExpr(GenericLoc, e->getType(),
syntactic =
                                              OK_Ordinary,
                                              opLHS.get()->getType(),
                                              result.get()->getType(),
                                              opcLoc, false);
   }
 
   // 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);
 }
 
 /// The basic skeleton for building an increment or decrement
 /// operation.
 ExprResult
 PseudoOpBuilder::buildIncDecOperation(Scope *Sc, SourceLocation opcLoc,
if (UnaryOperator::isIncrementOp(opcode)) {
     result = S.BuildBinOp(Sc, opcLoc, BO_Add, result.get(), one);
   } else {
     result = S.BuildBinOp(Sc, opcLoc, BO_Sub, result.get(), one);
   }
   if (result.isInvalid()) return ExprError();
 
   // 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,
                                   VK_LValue, OK_Ordinary, opcLoc);
   return complete(syntactic);
 }
 
 

cfe/trunk/test/CodeGenCXX/ms-property.cpp

 };
 
 template <typename T>
 class St {
 public:
   __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; }
 };
 
 template <typename T>
 void foo(T i, T j) {
   St<T> bar;
int main(int argc, char **argv) {
   // CHECK: call i32 @"\01?GetX@S@@QEAAHHH@Z"(%class.S* %{{.+}}, i32 223, i32 11)
   int j = p1->x[223][11];
   // CHECK: [[J:%.+]] = load i32, i32* %
   // CHECK-NEXT: call void @"\01?PutX@S@@QEAAXHHH@Z"(%class.S* %{{.+}}, i32 23, i32 1, i32 [[J]])
   p1->x[23][1] = j;
   // CHECK: call float @"\01?GetX@?$St@M@@QEAAMMM@Z"(%class.St* %{{.+}}, float 2.230000e+02, float 1.100000e+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.300000e+01, float 1.000000e+00, float [[J1]])
-  p2->x[23][1] = j1;
+  // CHECK-NEXT: [[CALL:%.+]] = call float @"\01?PutX@?$St@M@@QEAAMMMM@Z"(%class.St* %{{.+}}, float 2.300000e+01, float 1.000000e+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.000000e+00)
   // CHECK-NEXT: [[INC:%.+]] = fadd float [[GET]], 1.000000e+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.000000e+00, float [[INC]])
+  // CHECK-NEXT: call float @"\01?PutX@?$St@M@@QEAAMMMM@Z"(%class.St* %{{.+}}, float [[CONV]], float 1.000000e+00, float [[INC]])
   ++p2->x[idx()][1];
   // CHECK: call void @"\01??$foo@H@@YAXHH@Z"(i32 %{{.+}}, i32 %{{.+}})
   foo(argc, (int)argv[0][0]);
   // CHECK: [[P2:%.+]] = load %class.St*, %class.St** %
   // CHECK: [[T_X:%.+]] = call i32 @"\01?get_x@Test1@@QEBAHXZ"(%class.Test1* %{{.+}})
   // CHECK: [[P1:%.+]] = load %class.S*, %class.S** %
   // CHECK: [[P1_X_22_33:%.+]] = call i32 @"\01?GetX@S@@QEAAHHH@Z"(%class.S* [[P1]], i32 22, i32 33)
   // CHECK: [[CAST:%.+]] = sitofp i32 [[P1_X_22_33]] to double
   // CHECK: [[ARGC:%.+]] = load i32, i32* %
   // CHECK: [[CAST2:%.+]] = trunc i32 [[T_X]] to i8
   // CHECK: call void @"\01?PutY@?$St@M@@QEAAXDHN@Z"(%class.St* [[P2]], i8 [[CAST2]], i32 [[ARGC]], double [[CAST]])
   p2->y[t.X][argc] =  p1->x[22][33];
   // CHECK: [[P2_1:%.+]] = load %class.St*, %class.St**
   // CHECK: [[P2_2:%.+]] = load %class.St*, %class.St**
   // CHECK: [[P1:%.+]] = load %class.S*, %class.S**
   // CHECK: [[ARGC:%.+]] = load i32, i32* %
   // CHECK: [[P1_X_ARGC_0:%.+]] = call i32 @"\01?GetX@S@@QEAAHHH@Z"(%class.S* [[P1]], i32 [[ARGC]], i32 0)
   // CHECK: [[CAST:%.+]] = trunc i32 [[P1_X_ARGC_0]] to i8
   // CHECK: [[P2_Y_p1_X_ARGC_0_T:%.+]] = call i8 @"\01?GetY@?$St@M@@QEAADDVTest1@@@Z"(%class.St* [[P2_2]], i8 [[CAST]], %class.Test1* %{{.+}})
   // CHECK: [[CAST:%.+]] = sitofp i8 [[P2_Y_p1_X_ARGC_0_T]] to float
   // CHECK: [[J:%.+]] = load i32, i32* %
   // 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@@QEAAMMMM@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@@QEAAHHHH@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@@QEAAHHHH@Z"(%class.St{{.+}}* [[BAR]], i32 %{{.+}}, i32 %{{.+}}, i32 %{{.+}})
 #endif //HEADER

cfe/trunk/test/SemaCXX/ms-property-error.cpp

 // RUN: %clang_cc1 -verify -fms-compatibility %s -fsyntax-only -o -
 
 class S {
 public:
   __declspec(property(get=GetX,put=PutX)) int x[];
   int GetX(int i, int j) { return i+j; } // expected-note {{'GetX' declared here}}
   void PutX(int i, int j, int k) { j = i = k; } // expected-note {{'PutX' declared here}}
 };
 
+char *ptr;
 template <typename T>
 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'}}
   }
 };
 
 // CHECK-LABEL: main
 int main(int argc, char **argv) {
   S *p1 = 0;
   St<float> *p2 = 0;
   St<int> a; // expected-note {{in instantiation of member function 'St<int>::~St' requested here}}
   int j = (p1->x)[223][11][2]; // expected-error {{too many arguments to function call, expected 2, have 3}}
   (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}}
 }

cfe/trunk/test/SemaCXX/ms-property.cpp

public:
   void PutX(int i, int j, int k) { j = i = k; }
 };
 
 template <typename T>
 class St {
 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]; }
 };
 
 // CHECK: this->x[0][0] = this->x[1][1];
 // CHECK: this->x[0][0] = this->x[1][1];
 
 // CHECK-LABEL: main
 int main(int argc, char **argv) {
   S *p1 = 0;
   St<float> *p2 = 0;
   // CHECK: St<int> a;
   St<int> a;
   // CHECK-NEXT: int j = (p1->x)[223][11];
   int j = (p1->x)[223][11];
   // CHECK-NEXT: (p1->x[23])[1] = j;
   (p1->x[23])[1] = j;
   // CHECK-NEXT: float j1 = (p2->x[223][11]);
   float j1 = (p2->x[223][11]);
   // CHECK-NEXT: ((p2->x)[23])[1] = j1;
   ((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;
 }
 #endif // HEADER