rjmccall added a comment.
Sorry for the slow review; I'm getting crushed with review requests.
================
Comment at: clang/lib/AST/Expr.cpp:3859
+
+ auto *SubscriptE = dyn_cast<ArraySubscriptExpr>(this);
+ return SubscriptE
----------------
You need to `IgnoreParens()` here.
================
Comment at: clang/lib/AST/ExprConstant.cpp:7768
+ if (E->getBase()->getMatrixFromIndexExpr(Info.getLangOpts().MatrixTypes))
+ return false;
+
----------------
This can also occur in the other evaluators due to indexing into an r-value.
================
Comment at: clang/lib/Sema/SemaExpr.cpp:5326
+ VK = VK_LValue;
+ OK = OK_VectorComponent;
} else if (RHSTy->isArrayType()) {
----------------
It should have the value kind of its base. `get_matrix_rvalue()[0][0]` is
still an r-value, so you shouldn't allow it to be assigned to. Needs tests.
I'm fine with re-using `OK_VectorComponent` here, but you should (1) rename it
to `OK_VectorOrMatrixComponent` and (2) take the opportunity to audit the
places that use it to make sure they do something sensible. I expect you'll
need to at least update some diagnostics about e.g. disallowing taking the
address of the expression.
I think you should build a new expression node instead of reusing
`ArraySubscriptExpr`, since basically none of the code that looks for an
`ArraySubscriptExpr` will do the right thing for your operation. That will
also allow you to avoid the "is this actually enabled" check, since you'll only
see this node when your feature is enabled. If you're feeling generous, you
could move vector subscripting to this new node as well and leave
`ArraySubscriptExpr` exclusively for the standard (or dependent) cases.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D76791/new/
https://reviews.llvm.org/D76791
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
