diff --git a/Meta/Lagom/Tools/LibJSGCVerifier/src/CellsHandler.cpp b/Meta/Lagom/Tools/LibJSGCVerifier/src/CellsHandler.cpp index f51731fe9b..adcc80a667 100644 --- a/Meta/Lagom/Tools/LibJSGCVerifier/src/CellsHandler.cpp +++ b/Meta/Lagom/Tools/LibJSGCVerifier/src/CellsHandler.cpp @@ -18,6 +18,27 @@ #include #include +template +class SimpleCollectMatchesCallback : public clang::ast_matchers::MatchFinder::MatchCallback { +public: + explicit SimpleCollectMatchesCallback(std::string name) + : m_name(std::move(name)) + { + } + + void run(clang::ast_matchers::MatchFinder::MatchResult const& result) override + { + if (auto const* node = result.Nodes.getNodeAs(m_name)) + m_matches.push_back(node); + } + + auto const& matches() const { return m_matches; } + +private: + std::string m_name; + std::vector m_matches; +}; + CollectCellsHandler::CollectCellsHandler() { using namespace clang::ast_matchers; @@ -32,7 +53,6 @@ CollectCellsHandler::CollectCellsHandler() bool CollectCellsHandler::handleBeginSource(clang::CompilerInstance& ci) { auto const& source_manager = ci.getSourceManager(); - ci.getFileManager().getNumUniqueRealFiles(); auto file_id = source_manager.getMainFileID(); auto const* file_entry = source_manager.getFileEntryForID(file_id); if (!file_entry) @@ -142,6 +162,8 @@ FieldValidationResult validate_field(clang::FieldDecl const* field_decl) void CollectCellsHandler::run(clang::ast_matchers::MatchFinder::MatchResult const& result) { + using namespace clang::ast_matchers; + clang::CXXRecordDecl const* record = result.Nodes.getNodeAs("record-decl"); if (!record || !record->isCompleteDefinition() || (!record->isClass() && !record->isStruct())) return; @@ -149,8 +171,6 @@ void CollectCellsHandler::run(clang::ast_matchers::MatchFinder::MatchResult cons auto& diag_engine = result.Context->getDiagnostics(); for (clang::FieldDecl const* field : record->fields()) { - auto const& type = field->getType(); - auto validation_results = validate_field(field); if (!validation_results.is_valid) { if (validation_results.is_wrapped_in_gcptr) { @@ -159,7 +179,7 @@ void CollectCellsHandler::run(clang::ast_matchers::MatchFinder::MatchResult cons } else { auto diag_id = diag_engine.getCustomDiagID(clang::DiagnosticsEngine::Warning, "%0 to JS::Cell type should be wrapped in %1"); auto builder = diag_engine.Report(field->getLocation(), diag_id); - if (type->isReferenceType()) { + if (field->getType()->isReferenceType()) { builder << "reference" << "JS::NonnullGCPtr"; } else { @@ -169,4 +189,41 @@ void CollectCellsHandler::run(clang::ast_matchers::MatchFinder::MatchResult cons } } } + + if (!record_inherits_from_cell(*record)) + return; + + clang::DeclarationName const name = &result.Context->Idents.get("visit_edges"); + auto const* visit_edges_method = record->lookup(name).find_first(); + if (!visit_edges_method || !visit_edges_method->getBody()) + return; + + // Search for a call to Base::visit_edges. Note that this also has the nice side effect of + // 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"); + + auto base_visit_edges_matcher = cxxMemberCallExpr( + callee(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); + + bool call_to_base_visit_edges_found = false; + + for (auto const* call_expr : base_visit_edges_callback.matches()) { + // FIXME: Can we constrain the matcher above to avoid looking directly at the source code? + auto const* source_chars = result.SourceManager->getCharacterData(call_expr->getBeginLoc()); + if (strncmp(source_chars, "Base::", 6) == 0) { + call_to_base_visit_edges_found = true; + break; + } + } + + if (!call_to_base_visit_edges_found) { + auto diag_id = diag_engine.getCustomDiagID(clang::DiagnosticsEngine::Warning, "Missing call to Base::visit_edges"); + diag_engine.Report(visit_edges_method->getBeginLoc(), diag_id); + } }