[clang] [Clang] Add __builtin_get_counted_by builtin (PR #102549)

[via cfe-commits]

================
@@ -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