This revision was automatically updated to reflect the committed changes.
Closed by commit rL254067: [MSVC] 'property' with an empty array in array
subscript expression. (authored by ABataev).
Changed prior to commit:
http://reviews.llvm.org/D13336?vs=41018=41125#toc
Repository:
rL LLVM
ABataev added a comment.
John, thanks a lot for the review!
Committed version has a fix according to your last comment.
Repository:
rL LLVM
http://reviews.llvm.org/D13336
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
rjmccall added a comment.
This looks fantastic, thanks for doing all that. Approved with the one minor
change.
Comment at: lib/Sema/SemaPseudoObject.cpp:175
@@ -145,1 +174,3 @@
+PSE->getType(), PSE->getValueKind(), PSE->getObjectKind(),
+
ABataev updated this revision to Diff 41018.
ABataev added a comment.
Update after review
http://reviews.llvm.org/D13336
Files:
include/clang/AST/ExprCXX.h
include/clang/AST/RecursiveASTVisitor.h
include/clang/Basic/StmtNodes.td
include/clang/Serialization/ASTBitCodes.h
ABataev updated this revision to Diff 40901.
ABataev marked an inline comment as done.
http://reviews.llvm.org/D13336
Files:
include/clang/AST/DataRecursiveASTVisitor.h
include/clang/AST/ExprCXX.h
include/clang/AST/RecursiveASTVisitor.h
include/clang/Basic/StmtNodes.td
ABataev marked an inline comment as done.
Comment at: lib/Sema/SemaPseudoObject.cpp:1627
@@ -1579,1 +1626,3 @@
+cast(Base->IgnoreParens())->getBaseExpr());
+return MSPropertyRefRebuilder(S, BaseOVE->getSourceExpr()).rebuild(E);
} else {
rjmccall
rjmccall added inline comments.
Comment at: lib/Sema/SemaPseudoObject.cpp:57
@@ +56,3 @@
+ // with a base limits the number of cases here.
+ assert(refExpr->isObjectReceiver());
+
Well, it should be okay to call this on something that doesn't have a
ABataev updated this revision to Diff 40253.
ABataev marked 3 inline comments as done.
ABataev added a comment.
Update after review
http://reviews.llvm.org/D13336
Files:
include/clang/AST/DataRecursiveASTVisitor.h
include/clang/AST/ExprCXX.h
include/clang/AST/RecursiveASTVisitor.h
rjmccall added inline comments.
Comment at: include/clang/AST/ExprCXX.h:689
@@ +688,3 @@
+/// indices. In this case, i=p->x[a][b] will be turned into i=p->GetX(a, b),
and
+/// p->x[a][b] = i will be turned into p->PutX(a, b, i).
+class MSPropertySubscriptExpr : public Expr {
ABataev updated this revision to Diff 38232.
ABataev marked 4 inline comments as done.
ABataev added a comment.
Update after review
http://reviews.llvm.org/D13336
Files:
include/clang/AST/DataRecursiveASTVisitor.h
include/clang/AST/ExprCXX.h
include/clang/AST/RecursiveASTVisitor.h
ABataev updated this revision to Diff 37854.
ABataev added a comment.
Update after review
http://reviews.llvm.org/D13336
Files:
include/clang/AST/ASTContext.h
include/clang/AST/BuiltinTypes.def
include/clang/Serialization/ASTBitCodes.h
lib/AST/ASTContext.cpp
lib/AST/Type.cpp
rjmccall added a comment.
Needs more tests.
1. Dependent template instantiation.
2. Non-dependent template instantiation.
3. Indexes which are themselves pseudo-objects.
4. Templated getters and setters.
5. Non-POD by-value argument types.
Comment at:
rnk added a subscriber: rnk.
rnk added a comment.
I spent some time reading through the pseudo-object implementation in clang,
but I still don't understand it. We should ask John if he can review this.
My gut feeling is that we shouldn't have a global DenseMap in Sema just for
this. Should we
rjmccall added a comment.
I agree with Reid that you should not be adding a DenseMap to Sema for this.
Just build a SubscriptExpr for the syntactic form and have it yield an
expression of pseudo-object type; or you can make your own AST node for it if
that makes things easier.
ABataev added a comment.
In http://reviews.llvm.org/D13336#257628, @rnk wrote:
> I think fundamentally we are doing too much declspec property lowering in
> Sema. We might want to back up and figure out how to do it in IRGen. Right
> now we have bugs like this, which are probably more
ABataev created this revision.
ABataev added a reviewer: rnk.
ABataev added a subscriber: cfe-commits.
MSVC supports 'property' attribute and allows to apply it to the declaration of
an empty array in a class or structure definition.
For example:
```
__declspec(property(get=GetX, put=PutX)) int
rnk added a comment.
I think fundamentally we are doing too much declspec property lowering in Sema.
We might want to back up and figure out how to do it in IRGen. Right now we
have bugs like this, which are probably more important than new functionality:
17 matches
Mail list logo
