================
@@ -222,6 +222,49 @@ bool Expr::isFlexibleArrayMemberLike(
IgnoreTemplateOrMacroSubstitution);
}
+namespace {
+
+/// MemberExprVisitor - Find the MemberExpr through all of the casts, array
+/// subscripts, and unary ops. This intentionally avoids all of them because
+/// we're interested only in the MemberExpr to check if it's a flexible array
+/// member.
+class MemberExprVisitor
+ : public ConstStmtVisitor<MemberExprVisitor, const Expr *> {
+public:
+
//===--------------------------------------------------------------------===//
+ // Visitor Methods
+
//===--------------------------------------------------------------------===//
+
+ const Expr *Visit(const Expr *E) {
+ return ConstStmtVisitor<MemberExprVisitor, const Expr *>::Visit(E);
+ }
+ const Expr *VisitStmt(const Stmt *S) { return nullptr; }
+
+ const Expr *VisitMemberExpr(const MemberExpr *E) { return E; }
+
+ const Expr *VisitArraySubscriptExpr(const ArraySubscriptExpr *E) {
+ return Visit(E->getBase());
+ }
+ const Expr *VisitCastExpr(const CastExpr *E) {
+ return Visit(E->getSubExpr());
+ }
+ const Expr *VisitParenExpr(const ParenExpr *E) {
+ return Visit(E->getSubExpr());
+ }
+ const Expr *VisitUnaryAddrOf(const UnaryOperator *E) {
+ return Visit(E->getSubExpr());
+ }
+ const Expr *VisitUnaryDeref(const UnaryOperator *E) {
+ return Visit(E->getSubExpr());
+ }
----------------
Sirraide wrote:
I probably would have just checked if it’s either a `MemberExor`—or possibly a
`UnaryOperator` w/ `UO_AddrOf` that contains an `ArraySubscriptExpr` whose LHS
is a `MemberExpr`, with a `IgnoreParenImpCasts` between checking for each of
these (if you really want to support `&foo->x[0]` too, but are there any
real-world use cases for that?).
(I don’t think there are *so* many different patterns here that we’d need a
visitor because the way it’s written rn, it can also lead to false positives,
e.g. `__builtin_get_counted_by(**foo->x)`, which seems like it should be
nonsense to me)
If, as the result of that, you end up w/ a `MemberExpr`, then evaluate the
builtin (i.e. return `null` or the count).
If you *don’t* have a `MemberExpr`, i.e. the user entered garbage like
`*foo->x` or something else entirely, or if it is a `MemberExpr` that doesn’t
point to a FAM, then I would straight-up error.
> it worries me that the behavior becomes unknowable.
Doing that should also take care of this: if you pass in what boils down to a
`MemberExpr` to a FAM, you get either `null` or a pointer to the count; if you
pass in anything else, you get an error.
Imo that behaviour is reasonable enough. I don’t think ‘ok, here is something
that somehow remotely resembles or involves a member access, so please do
*something* with it’ is something we should support—especially because this
could ‘silently’ fail in that case if the user did something wrong (by
returning null). I’d like to minimise the possibility of that happening.
Also, this is a new builtin, so it’s not like we need to be too worried about
breaking anyone’s code because no-one is using it because it’s new. That said,
if you end up running into a lot of instances of one particular pattern in e.g.
the kernel that this doesn’t support, then it’d make perfect sense to add that
too—I just feel like a conservative approach to what we accept would make more
sense for the time being.
https://github.com/llvm/llvm-project/pull/102549
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
