mirror of
https://github.com/RGBCube/serenity
synced 2025-07-27 06:27:45 +00:00
Lagom: Enforce uniform calls to Base::visit_edges in LibJSGCVerifier
This ensures that all visit_edges implementations include a call to Base::visit_edges. In particular, this gives three nice benefits: - The call can't be forgotten (the main benefit, of course). - All of the calls look the same. In other words, always use "Base" instead of the actual concrete class. - Ensure the object has a call to JS_CELL or JS_OBJECT in the definition. Otherwise, Base will not be defined and the call will not compile.
This commit is contained in:
parent
3f22919eb5
commit
98ed74087f
1 changed files with 61 additions and 4 deletions
|
@ -18,6 +18,27 @@
|
||||||
#include <unordered_set>
|
#include <unordered_set>
|
||||||
#include <vector>
|
#include <vector>
|
||||||
|
|
||||||
|
template<typename T>
|
||||||
|
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<T>(m_name))
|
||||||
|
m_matches.push_back(node);
|
||||||
|
}
|
||||||
|
|
||||||
|
auto const& matches() const { return m_matches; }
|
||||||
|
|
||||||
|
private:
|
||||||
|
std::string m_name;
|
||||||
|
std::vector<T const*> m_matches;
|
||||||
|
};
|
||||||
|
|
||||||
CollectCellsHandler::CollectCellsHandler()
|
CollectCellsHandler::CollectCellsHandler()
|
||||||
{
|
{
|
||||||
using namespace clang::ast_matchers;
|
using namespace clang::ast_matchers;
|
||||||
|
@ -32,7 +53,6 @@ CollectCellsHandler::CollectCellsHandler()
|
||||||
bool CollectCellsHandler::handleBeginSource(clang::CompilerInstance& ci)
|
bool CollectCellsHandler::handleBeginSource(clang::CompilerInstance& ci)
|
||||||
{
|
{
|
||||||
auto const& source_manager = ci.getSourceManager();
|
auto const& source_manager = ci.getSourceManager();
|
||||||
ci.getFileManager().getNumUniqueRealFiles();
|
|
||||||
auto file_id = source_manager.getMainFileID();
|
auto file_id = source_manager.getMainFileID();
|
||||||
auto const* file_entry = source_manager.getFileEntryForID(file_id);
|
auto const* file_entry = source_manager.getFileEntryForID(file_id);
|
||||||
if (!file_entry)
|
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)
|
void CollectCellsHandler::run(clang::ast_matchers::MatchFinder::MatchResult const& result)
|
||||||
{
|
{
|
||||||
|
using namespace clang::ast_matchers;
|
||||||
|
|
||||||
clang::CXXRecordDecl const* record = result.Nodes.getNodeAs<clang::CXXRecordDecl>("record-decl");
|
clang::CXXRecordDecl const* record = result.Nodes.getNodeAs<clang::CXXRecordDecl>("record-decl");
|
||||||
if (!record || !record->isCompleteDefinition() || (!record->isClass() && !record->isStruct()))
|
if (!record || !record->isCompleteDefinition() || (!record->isClass() && !record->isStruct()))
|
||||||
return;
|
return;
|
||||||
|
@ -149,8 +171,6 @@ void CollectCellsHandler::run(clang::ast_matchers::MatchFinder::MatchResult cons
|
||||||
auto& diag_engine = result.Context->getDiagnostics();
|
auto& diag_engine = result.Context->getDiagnostics();
|
||||||
|
|
||||||
for (clang::FieldDecl const* field : record->fields()) {
|
for (clang::FieldDecl const* field : record->fields()) {
|
||||||
auto const& type = field->getType();
|
|
||||||
|
|
||||||
auto validation_results = validate_field(field);
|
auto validation_results = validate_field(field);
|
||||||
if (!validation_results.is_valid) {
|
if (!validation_results.is_valid) {
|
||||||
if (validation_results.is_wrapped_in_gcptr) {
|
if (validation_results.is_wrapped_in_gcptr) {
|
||||||
|
@ -159,7 +179,7 @@ void CollectCellsHandler::run(clang::ast_matchers::MatchFinder::MatchResult cons
|
||||||
} else {
|
} else {
|
||||||
auto diag_id = diag_engine.getCustomDiagID(clang::DiagnosticsEngine::Warning, "%0 to JS::Cell type should be wrapped in %1");
|
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);
|
auto builder = diag_engine.Report(field->getLocation(), diag_id);
|
||||||
if (type->isReferenceType()) {
|
if (field->getType()->isReferenceType()) {
|
||||||
builder << "reference"
|
builder << "reference"
|
||||||
<< "JS::NonnullGCPtr";
|
<< "JS::NonnullGCPtr";
|
||||||
} else {
|
} 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<clang::CXXMethodDecl>();
|
||||||
|
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<clang::CXXMemberCallExpr> 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);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue