From 6a4938a52405d99577507c2a7163a5ee2b0f45a4 Mon Sep 17 00:00:00 2001 From: Matthew Olsson Date: Sat, 18 May 2024 07:08:23 -0700 Subject: [PATCH] ClangPlugins: Convert all warnings to errors Now that the lambda capture plugin isn't full of false-positives, we can make the jump and start halting builds for these errors. It also allows these plugins to be useful in CI. --- .../LambdaCapturePluginAction.cpp | 2 +- .../ClangPlugins/LibJSGCPluginAction.cpp | 10 +++++----- .../lambda_capture_local_by_ref.cpp | 2 +- ...ase_visit_edges_not_through_using_decl.cpp | 2 +- .../LibJSGCTests/cell_member_not_wrapped.cpp | 20 +++++++++---------- .../missing_call_to_base_visit_edges.cpp | 2 +- .../missing_member_in_visit_edges.cpp | 2 +- .../missing_visit_edges_method.cpp | 2 +- .../LibJSGCTests/wrapping_non_cell_member.cpp | 6 +++--- 9 files changed, 24 insertions(+), 24 deletions(-) diff --git a/Meta/Lagom/ClangPlugins/LambdaCapturePluginAction.cpp b/Meta/Lagom/ClangPlugins/LambdaCapturePluginAction.cpp index 6ca31fdb70..b4dbdb52b3 100644 --- a/Meta/Lagom/ClangPlugins/LambdaCapturePluginAction.cpp +++ b/Meta/Lagom/ClangPlugins/LambdaCapturePluginAction.cpp @@ -106,7 +106,7 @@ public: if (capture->capturesThis() || capture->getCaptureKind() != clang::LCK_ByRef) return; - auto diag_id = diag_engine.getCustomDiagID(clang::DiagnosticsEngine::Warning, "Variable with local storage is captured by reference in a lambda marked ESCAPING"); + auto diag_id = diag_engine.getCustomDiagID(clang::DiagnosticsEngine::Error, "Variable with local storage is captured by reference in a lambda marked ESCAPING"); diag_engine.Report(capture->getLocation(), diag_id); clang::SourceLocation captured_var_location; diff --git a/Meta/Lagom/ClangPlugins/LibJSGCPluginAction.cpp b/Meta/Lagom/ClangPlugins/LibJSGCPluginAction.cpp index 7826dff3b9..4a4cff01f4 100644 --- a/Meta/Lagom/ClangPlugins/LibJSGCPluginAction.cpp +++ b/Meta/Lagom/ClangPlugins/LibJSGCPluginAction.cpp @@ -155,10 +155,10 @@ bool LibJSGCVisitor::VisitCXXRecordDecl(clang::CXXRecordDecl* record) auto validation_results = validate_field(field); if (!validation_results.is_valid) { if (validation_results.is_wrapped_in_gcptr) { - auto diag_id = diag_engine.getCustomDiagID(clang::DiagnosticsEngine::Warning, "Specialization type must inherit from JS::Cell"); + auto diag_id = diag_engine.getCustomDiagID(clang::DiagnosticsEngine::Error, "Specialization type must inherit from JS::Cell"); diag_engine.Report(field->getLocation(), diag_id); } 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::Error, "%0 to JS::Cell type should be wrapped in %1"); auto builder = diag_engine.Report(field->getLocation(), diag_id); if (field->getType()->isReferenceType()) { builder << "reference" @@ -179,7 +179,7 @@ bool LibJSGCVisitor::VisitCXXRecordDecl(clang::CXXRecordDecl* record) clang::DeclarationName name = &m_context.Idents.get("visit_edges"); auto const* visit_edges_method = record->lookup(name).find_first(); if (!visit_edges_method && !fields_that_need_visiting.empty()) { - auto diag_id = diag_engine.getCustomDiagID(clang::DiagnosticsEngine::Warning, "JS::Cell-inheriting class %0 contains a GC-allocated member %1 but has no visit_edges method"); + auto diag_id = diag_engine.getCustomDiagID(clang::DiagnosticsEngine::Error, "JS::Cell-inheriting class %0 contains a GC-allocated member %1 but has no visit_edges method"); auto builder = diag_engine.Report(record->getLocation(), diag_id); builder << record->getName() << fields_that_need_visiting[0]; @@ -214,7 +214,7 @@ bool LibJSGCVisitor::VisitCXXRecordDecl(clang::CXXRecordDecl* record) } if (!call_to_base_visit_edges_found) { - auto diag_id = diag_engine.getCustomDiagID(clang::DiagnosticsEngine::Warning, "Missing call to Base::visit_edges"); + auto diag_id = diag_engine.getCustomDiagID(clang::DiagnosticsEngine::Error, "Missing call to Base::visit_edges"); diag_engine.Report(visit_edges_method->getBeginLoc(), diag_id); } @@ -240,7 +240,7 @@ bool LibJSGCVisitor::VisitCXXRecordDecl(clang::CXXRecordDecl* record) for (auto const* member_expr : field_access_callback.matches()) fields_that_are_visited.insert(member_expr->getMemberNameInfo().getAsString()); - auto diag_id = diag_engine.getCustomDiagID(clang::DiagnosticsEngine::Warning, "GC-allocated member is not visited in %0::visit_edges"); + auto diag_id = diag_engine.getCustomDiagID(clang::DiagnosticsEngine::Error, "GC-allocated member is not visited in %0::visit_edges"); for (auto const* field : fields_that_need_visiting) { if (!fields_that_are_visited.contains(field->getNameAsString())) { diff --git a/Tests/ClangPlugins/LambdaTests/lambda_capture_local_by_ref.cpp b/Tests/ClangPlugins/LambdaTests/lambda_capture_local_by_ref.cpp index 5c1c29ff68..7861ed54b2 100644 --- a/Tests/ClangPlugins/LambdaTests/lambda_capture_local_by_ref.cpp +++ b/Tests/ClangPlugins/LambdaTests/lambda_capture_local_by_ref.cpp @@ -20,7 +20,7 @@ void test() (void)a; }); - // expected-warning@+1 {{Variable with local storage is captured by reference in a lambda marked ESCAPING}} + // expected-error@+1 {{Variable with local storage is captured by reference in a lambda marked ESCAPING}} take_fn_escaping([&a] { (void)a; }); diff --git a/Tests/ClangPlugins/LibJSGCTests/calling_base_visit_edges_not_through_using_decl.cpp b/Tests/ClangPlugins/LibJSGCTests/calling_base_visit_edges_not_through_using_decl.cpp index 9356b8e12f..22f92c5218 100644 --- a/Tests/ClangPlugins/LibJSGCTests/calling_base_visit_edges_not_through_using_decl.cpp +++ b/Tests/ClangPlugins/LibJSGCTests/calling_base_visit_edges_not_through_using_decl.cpp @@ -11,7 +11,7 @@ class TestClass : public JS::Object { JS_OBJECT(TestClass, JS::Object); - // expected-warning@+1 {{Missing call to Base::visit_edges}} + // expected-error@+1 {{Missing call to Base::visit_edges}} virtual void visit_edges(Visitor& visitor) override { JS::Object::visit_edges(visitor); diff --git a/Tests/ClangPlugins/LibJSGCTests/cell_member_not_wrapped.cpp b/Tests/ClangPlugins/LibJSGCTests/cell_member_not_wrapped.cpp index 0a28276455..bd66f08964 100644 --- a/Tests/ClangPlugins/LibJSGCTests/cell_member_not_wrapped.cpp +++ b/Tests/ClangPlugins/LibJSGCTests/cell_member_not_wrapped.cpp @@ -30,15 +30,15 @@ private: visitor.visit(m_object_ptr); } - // expected-warning@+1 {{reference to JS::Cell type should be wrapped in JS::NonnullGCPtr}} + // expected-error@+1 {{reference to JS::Cell type should be wrapped in JS::NonnullGCPtr}} JS::Object& m_object_ref; - // expected-warning@+1 {{pointer to JS::Cell type should be wrapped in JS::GCPtr}} + // expected-error@+1 {{pointer to JS::Cell type should be wrapped in JS::GCPtr}} JS::Object* m_object_ptr; - // expected-warning@+1 {{pointer to JS::Cell type should be wrapped in JS::GCPtr}} + // expected-error@+1 {{pointer to JS::Cell type should be wrapped in JS::GCPtr}} Vector m_objects; - // expected-warning@+1 {{pointer to JS::Cell type should be wrapped in JS::GCPtr}} + // expected-error@+1 {{pointer to JS::Cell type should be wrapped in JS::GCPtr}} NewType1* m_newtype_1; - // expected-warning@+1 {{pointer to JS::Cell type should be wrapped in JS::GCPtr}} + // expected-error@+1 {{pointer to JS::Cell type should be wrapped in JS::GCPtr}} NewType2* m_newtype_2; }; @@ -50,14 +50,14 @@ public: } private: - // expected-warning@+1 {{reference to JS::Cell type should be wrapped in JS::NonnullGCPtr}} + // expected-error@+1 {{reference to JS::Cell type should be wrapped in JS::NonnullGCPtr}} JS::Object& m_object_ref; - // expected-warning@+1 {{pointer to JS::Cell type should be wrapped in JS::GCPtr}} + // expected-error@+1 {{pointer to JS::Cell type should be wrapped in JS::GCPtr}} JS::Object* m_object_ptr; - // expected-warning@+1 {{pointer to JS::Cell type should be wrapped in JS::GCPtr}} + // expected-error@+1 {{pointer to JS::Cell type should be wrapped in JS::GCPtr}} Vector m_objects; - // expected-warning@+1 {{pointer to JS::Cell type should be wrapped in JS::GCPtr}} + // expected-error@+1 {{pointer to JS::Cell type should be wrapped in JS::GCPtr}} NewType1* m_newtype_1; - // expected-warning@+1 {{pointer to JS::Cell type should be wrapped in JS::GCPtr}} + // expected-error@+1 {{pointer to JS::Cell type should be wrapped in JS::GCPtr}} NewType2* m_newtype_2; }; diff --git a/Tests/ClangPlugins/LibJSGCTests/missing_call_to_base_visit_edges.cpp b/Tests/ClangPlugins/LibJSGCTests/missing_call_to_base_visit_edges.cpp index ce45cc2cd8..67143f7914 100644 --- a/Tests/ClangPlugins/LibJSGCTests/missing_call_to_base_visit_edges.cpp +++ b/Tests/ClangPlugins/LibJSGCTests/missing_call_to_base_visit_edges.cpp @@ -11,7 +11,7 @@ class TestClass : public JS::Object { JS_OBJECT(TestClass, JS::Object); - // expected-warning@+1 {{Missing call to Base::visit_edges}} + // expected-error@+1 {{Missing call to Base::visit_edges}} virtual void visit_edges(Visitor&) override { } diff --git a/Tests/ClangPlugins/LibJSGCTests/missing_member_in_visit_edges.cpp b/Tests/ClangPlugins/LibJSGCTests/missing_member_in_visit_edges.cpp index e6ea571ad1..5f41cedbaa 100644 --- a/Tests/ClangPlugins/LibJSGCTests/missing_member_in_visit_edges.cpp +++ b/Tests/ClangPlugins/LibJSGCTests/missing_member_in_visit_edges.cpp @@ -16,6 +16,6 @@ class TestClass : public JS::Object { Base::visit_edges(visitor); } - // expected-warning@+1 {{GC-allocated member is not visited in TestClass::visit_edges}} + // expected-error@+1 {{GC-allocated member is not visited in TestClass::visit_edges}} JS::GCPtr m_object; }; diff --git a/Tests/ClangPlugins/LibJSGCTests/missing_visit_edges_method.cpp b/Tests/ClangPlugins/LibJSGCTests/missing_visit_edges_method.cpp index fb810bbb03..8f49ba0e4c 100644 --- a/Tests/ClangPlugins/LibJSGCTests/missing_visit_edges_method.cpp +++ b/Tests/ClangPlugins/LibJSGCTests/missing_visit_edges_method.cpp @@ -8,7 +8,7 @@ #include -// expected-warning@+1 {{JS::Cell-inheriting class TestClass contains a GC-allocated member 'm_cell' but has no visit_edges method}} +// expected-error@+1 {{JS::Cell-inheriting class TestClass contains a GC-allocated member 'm_cell' but has no visit_edges method}} class TestClass : public JS::Object { JS_OBJECT(TestClass, JS::Object); diff --git a/Tests/ClangPlugins/LibJSGCTests/wrapping_non_cell_member.cpp b/Tests/ClangPlugins/LibJSGCTests/wrapping_non_cell_member.cpp index 1eebe4cdc4..ce524e66d4 100644 --- a/Tests/ClangPlugins/LibJSGCTests/wrapping_non_cell_member.cpp +++ b/Tests/ClangPlugins/LibJSGCTests/wrapping_non_cell_member.cpp @@ -12,10 +12,10 @@ struct NotACell { }; class TestClass { - // expected-warning@+1 {{Specialization type must inherit from JS::Cell}} + // expected-error@+1 {{Specialization type must inherit from JS::Cell}} JS::GCPtr m_member_1; - // expected-warning@+1 {{Specialization type must inherit from JS::Cell}} + // expected-error@+1 {{Specialization type must inherit from JS::Cell}} JS::NonnullGCPtr m_member_2; - // expected-warning@+1 {{Specialization type must inherit from JS::Cell}} + // expected-error@+1 {{Specialization type must inherit from JS::Cell}} JS::RawGCPtr m_member_3; };