[clang] [attributes][-Wunsafe-buffer-usage] Support adding unsafe_buffer_usage attribute to struct fields (PR #101585)

[Artem Dergachev via cfe-commits]

================
@@ -927,21 +927,28 @@ class CArrayToPtrAssignmentGadget : public FixableGadget {
 /// over one of its pointer parameters.
 class UnsafeBufferUsageAttrGadget : public WarningGadget {
   constexpr static const char *const OpTag = "call_expr";
-  const CallExpr *Op;
+  const Expr *Op;
 
 public:
   UnsafeBufferUsageAttrGadget(const MatchFinder::MatchResult &Result)
       : WarningGadget(Kind::UnsafeBufferUsageAttr),
-        Op(Result.Nodes.getNodeAs<CallExpr>(OpTag)) {}
+        Op(Result.Nodes.getNodeAs<Expr>(OpTag)) {}
 
   static bool classof(const Gadget *G) {
     return G->getKind() == Kind::UnsafeBufferUsageAttr;
   }
 
   static Matcher matcher() {
+    auto HasUnsafeFielDecl = 
+        member(fieldDecl(allOf(
+               anyOf(hasPointerType(), hasArrayType()),
----------------
haoNoQ wrote:

> Doesn't the absence of -Wunsafe-buffer-usage warning give more or less the same signal as the additional warning would provide?

> I suggest we don't implement the extra warning.

The purpose of implementing the extra warning/error would have been to avoid users writing code that will later be declared unsupported. The user may think "hmm there's a false negative, let me report a bug about the false negative but keep the code". Then they suddenly get an error later. This appears to be a strong tradition with attributes in Clang: the initial patch supports a restricted scope of use cases, contains a warning/error for uses beyond that scope, then later the scope gets expanded to more and more supported use cases later. We're expected to actively ban all uses we don't immediately support.

But it sounds like we're taking the "big hammer" approach anyway so yeah the warning is no longer necessary. Every use case is now a supported use case.

I'm slightly worried that with the "big hammer" approach we'll lose the opportunity to implement the "small hammer" approach in the future. Eg. we no longer have a way to suppress the warning when a pointer field is only used for the purposes of comparing to null, or when the size field is only read but never written. Like, we could implement that, but that would result in a "confusing hammer" that can be either big or small depending on obscure details of circumstantial evidence. So if we ultimately realize we need a smaller hammer - which is quite likely - we'll probably need to build a separate attribute.

But I think I'm ok with that. This is in line with the rest of our work: we're mostly in the business of shipping big hammers.

https://github.com/llvm/llvm-project/pull/101585