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:
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
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:
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
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
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:
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?
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
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.
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.
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
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
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())
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
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
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
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:
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())
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
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
20 matches
Mail list logo
