Avoid FieldDecl visitor

We can visit CXXDefaultInitExprs to find construction through
[[unsafe_buffer_usage]] constructors of field initializers.

Author: danakj

clang/lib/Analysis/UnsafeBufferUsage.cpp

@@ -171,6 +171,12 @@ class MatchDescendantVisitor
     return VisitorBase::TraverseCXXTypeidExpr(Node);
   }
 
+  bool TraverseCXXDefaultInitExpr(CXXDefaultInitExpr *Node) {
+    if (!TraverseStmt(Node->getExpr()))
+      return false;
+    return VisitorBase::TraverseCXXDefaultInitExpr(Node);
+  }
+
   bool TraverseStmt(Stmt *Node, DataRecursionQueue *Queue = nullptr) {
     if (!Node)
       return true;
@@ -3927,16 +3933,6 @@ void clang::checkUnsafeBufferUsage(const Decl *D,
         Stmts.push_back(CI->getInit());
       }
     }
-  } else if (const auto *FlD = dyn_cast<FieldDecl>(D)) {
-    // Visit in-class initializers for fields.
-    if (!FlD->hasInClassInitializer())
-      return;
-    Stmts.push_back(FlD->getInClassInitializer());
-    // In a FieldDecl there is no function body, there is only a single
-    // statement, and the suggestions machinery is not set up to handle that
-    // kind of structure yet and would give poor suggestions or likely even hit
-    // asserts.
-    EmitSuggestions = false;
   } else if (isa<BlockDecl>(D) || isa<ObjCMethodDecl>(D)) {
     Stmts.push_back(D->getBody());
   }

clang/lib/Sema/AnalysisBasedWarnings.cpp

@@ -2510,38 +2510,6 @@ class CallableVisitor : public RecursiveASTVisitor<CallableVisitor> {
   CallableVisitor(llvm::function_ref<void(const Decl *)> Callback)
       : Callback(Callback) {}
 
-  bool VisitFieldDecl(FieldDecl *Node) {
-    DeclContext *DeclCtx;
-    if (auto *ID = dyn_cast<ObjCIvarDecl>(Node)) {
-      DeclCtx = cast<DeclContext>(ID->getContainingInterface());
-    } else {
-      RecordDecl *RD = Node->getParent();
-      if (auto *CCRD = dyn_cast<CXXRecordDecl>(RD);
-          CCRD && CCRD->isAggregate()) {
-        // Aggregates have implicitly generated constructors which are not
-        // traversed by our AST visitors at this time. The field initializers in
-        // an aggregate may never be used if the callers always explicitly
-        // initialize them, in which case we do not need to warn on unsafe
-        // buffer usage in the initializer.
-        //
-        // FIXME: We should go through the implicit aggregate initialization
-        // (either an implicit CXXConstructExpr for value-init or an implicit
-        // ListInitExpr for aggregate-init) to determine if a field's default
-        // initializer (CXXDefaultInitExpr) is actually used. If it is, then we
-        // should visit it, while retaining a reference to the caller for
-        // showing the path to the use of the default initializer in the
-        // warning.
-        return true;
-      }
-      DeclCtx = cast<DeclContext>(RD);
-    }
-    if (DeclCtx->isDependentContext())
-      return true; // Not to analyze dependent decl
-    if (Node->hasInClassInitializer())
-      Callback(Node);
-    return true;
-  }
-
   bool VisitFunctionDecl(FunctionDecl *Node) {
     if (cast<DeclContext>(Node)->isDependentContext())
       return true; // Not to analyze dependent decl

clang/test/SemaCXX/warn-unsafe-buffer-usage-function-attr.cpp

@@ -128,7 +128,7 @@ struct HoldsUnsafeMembers {
 
     UnsafeMembers FromCtor;
     UnsafeMembers FromCtor2;
-    UnsafeMembers FromField{3};  // expected-warning{{function introduces unsafe buffer manipulation}}
+    UnsafeMembers FromField{3};  // expected-warning 2{{function introduces unsafe buffer manipulation}}
 };
 
 struct SubclassUnsafeMembers : public UnsafeMembers {

clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-span-construct.cpp

@@ -158,12 +158,22 @@ namespace test_flag {
   }
 } //namespace test_flag
 
-struct HoldsStdSpan {
+struct HoldsStdSpanAndInitializedInCtor {
   char* Ptr;
   unsigned Size;
-  std::span<char> Span{Ptr, Size};  // expected-warning{{the two-parameter std::span construction is unsafe as it can introduce mismatch between buffer size and the bound information}}
+  std::span<char> Span{Ptr, Size};  // no-warning (this code is unreachable)
 
-  HoldsStdSpan(char* P, unsigned S)
+  HoldsStdSpanAndInitializedInCtor(char* P, unsigned S)
       : Span(P, S)  // expected-warning{{the two-parameter std::span construction is unsafe as it can introduce mismatch between buffer size and the bound information}}
   {}
 };
+
+struct HoldsStdSpanAndNotInitializedInCtor {
+  char* Ptr;
+  unsigned Size;
+  std::span<char> Span{Ptr, Size}; // expected-warning{{the two-parameter std::span construction is unsafe as it can introduce mismatch between buffer size and the bound information}}
+
+  HoldsStdSpanAndNotInitializedInCtor(char* P, unsigned S)
+      : Ptr(P), Size(S)
+  {}
+};