From f489c3d9c2c0207720eec72e69333b89bb8cc1b4 Mon Sep 17 00:00:00 2001 From: 0x4261756D <38735823+0x4261756D@users.noreply.github.com> Date: Wed, 3 Apr 2024 15:44:52 +0200 Subject: [PATCH] LibJSGCVerifier: Fix false positives in HeapFunction::visit_edges() clang doesn't make all `Base::visit_edges()` calls CXXMemberCallExprs This would lead to false positives like in HeapFunction, where the matcher would fail to match and report a warning. Also previously the matcher would succeed if the visited class is missing the call to `Base::visit_edges()` but an included class has a correct method. The new matcher checks the current class for `visit_edges`-overrides and matches all `visit_edges`-memberExprs inside, checking those for starting with `Base::`. This seems to get rid of the false positives and should be more correct detecting missing calls. --- .../Tools/LibJSGCVerifier/src/CellsHandler.cpp | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/Meta/Lagom/Tools/LibJSGCVerifier/src/CellsHandler.cpp b/Meta/Lagom/Tools/LibJSGCVerifier/src/CellsHandler.cpp index fb6cb0a6bd4..711c516d1c1 100644 --- a/Meta/Lagom/Tools/LibJSGCVerifier/src/CellsHandler.cpp +++ b/Meta/Lagom/Tools/LibJSGCVerifier/src/CellsHandler.cpp @@ -176,7 +176,8 @@ void CollectCellsHandler::run(clang::ast_matchers::MatchFinder::MatchResult cons // Cell triggers a bunch of warnings for its empty visit_edges implementation, but // it doesn't have any members anyways so it's fine to just ignore. - if (record->getQualifiedNameAsString() == "JS::Cell") + auto qualified_name = record->getQualifiedNameAsString(); + if (qualified_name == "JS::Cell") return; auto& diag_engine = result.Context->getDiagnostics(); @@ -216,11 +217,13 @@ void CollectCellsHandler::run(clang::ast_matchers::MatchFinder::MatchResult cons // ensuring the classes use JS_CELL/JS_OBJECT, as Base will not be defined if they do not. MatchFinder base_visit_edges_finder; - SimpleCollectMatchesCallback base_visit_edges_callback("member-call"); + SimpleCollectMatchesCallback base_visit_edges_callback("member-call"); - auto base_visit_edges_matcher = cxxMemberCallExpr( - callee(memberExpr(member(hasName("visit_edges"))))) - .bind("member-call"); + auto base_visit_edges_matcher = cxxMethodDecl( + ofClass(hasName(qualified_name)), + functionDecl(hasName("visit_edges")), + isOverride(), + hasDescendant(memberExpr(member(hasName("visit_edges"))).bind("member-call"))); base_visit_edges_finder.addMatcher(base_visit_edges_matcher, &base_visit_edges_callback); base_visit_edges_finder.matchAST(*result.Context);