Browse Source

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.
Matthew Olsson 1 year ago
parent
commit
6a4938a524

+ 1 - 1
Meta/Lagom/ClangPlugins/LambdaCapturePluginAction.cpp

@@ -106,7 +106,7 @@ public:
             if (capture->capturesThis() || capture->getCaptureKind() != clang::LCK_ByRef)
             if (capture->capturesThis() || capture->getCaptureKind() != clang::LCK_ByRef)
                 return;
                 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);
             diag_engine.Report(capture->getLocation(), diag_id);
 
 
             clang::SourceLocation captured_var_location;
             clang::SourceLocation captured_var_location;

+ 5 - 5
Meta/Lagom/ClangPlugins/LibJSGCPluginAction.cpp

@@ -155,10 +155,10 @@ bool LibJSGCVisitor::VisitCXXRecordDecl(clang::CXXRecordDecl* record)
         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) {
-                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);
                 diag_engine.Report(field->getLocation(), diag_id);
             } 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::Error, "%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 (field->getType()->isReferenceType()) {
                 if (field->getType()->isReferenceType()) {
                     builder << "reference"
                     builder << "reference"
@@ -179,7 +179,7 @@ bool LibJSGCVisitor::VisitCXXRecordDecl(clang::CXXRecordDecl* record)
     clang::DeclarationName name = &m_context.Idents.get("visit_edges");
     clang::DeclarationName name = &m_context.Idents.get("visit_edges");
     auto const* visit_edges_method = record->lookup(name).find_first<clang::CXXMethodDecl>();
     auto const* visit_edges_method = record->lookup(name).find_first<clang::CXXMethodDecl>();
     if (!visit_edges_method && !fields_that_need_visiting.empty()) {
     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);
         auto builder = diag_engine.Report(record->getLocation(), diag_id);
         builder << record->getName()
         builder << record->getName()
                 << fields_that_need_visiting[0];
                 << fields_that_need_visiting[0];
@@ -214,7 +214,7 @@ bool LibJSGCVisitor::VisitCXXRecordDecl(clang::CXXRecordDecl* record)
     }
     }
 
 
     if (!call_to_base_visit_edges_found) {
     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);
         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())
     for (auto const* member_expr : field_access_callback.matches())
         fields_that_are_visited.insert(member_expr->getMemberNameInfo().getAsString());
         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) {
     for (auto const* field : fields_that_need_visiting) {
         if (!fields_that_are_visited.contains(field->getNameAsString())) {
         if (!fields_that_are_visited.contains(field->getNameAsString())) {

+ 1 - 1
Tests/ClangPlugins/LambdaTests/lambda_capture_local_by_ref.cpp

@@ -20,7 +20,7 @@ void test()
         (void)a;
         (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] {
     take_fn_escaping([&a] {
         (void)a;
         (void)a;
     });
     });

+ 1 - 1
Tests/ClangPlugins/LibJSGCTests/calling_base_visit_edges_not_through_using_decl.cpp

@@ -11,7 +11,7 @@
 class TestClass : public JS::Object {
 class TestClass : public JS::Object {
     JS_OBJECT(TestClass, 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
     virtual void visit_edges(Visitor& visitor) override
     {
     {
         JS::Object::visit_edges(visitor);
         JS::Object::visit_edges(visitor);

+ 10 - 10
Tests/ClangPlugins/LibJSGCTests/cell_member_not_wrapped.cpp

@@ -30,15 +30,15 @@ private:
         visitor.visit(m_object_ptr);
         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;
     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;
     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<JS::Object*> m_objects;
     Vector<JS::Object*> 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;
     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;
     NewType2* m_newtype_2;
 };
 };
 
 
@@ -50,14 +50,14 @@ public:
     }
     }
 
 
 private:
 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;
     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;
     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<JS::Object*> m_objects;
     Vector<JS::Object*> 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;
     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;
     NewType2* m_newtype_2;
 };
 };

+ 1 - 1
Tests/ClangPlugins/LibJSGCTests/missing_call_to_base_visit_edges.cpp

@@ -11,7 +11,7 @@
 class TestClass : public JS::Object {
 class TestClass : public JS::Object {
     JS_OBJECT(TestClass, 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
     virtual void visit_edges(Visitor&) override
     {
     {
     }
     }

+ 1 - 1
Tests/ClangPlugins/LibJSGCTests/missing_member_in_visit_edges.cpp

@@ -16,6 +16,6 @@ class TestClass : public JS::Object {
         Base::visit_edges(visitor);
         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<JS::Object> m_object;
     JS::GCPtr<JS::Object> m_object;
 };
 };

+ 1 - 1
Tests/ClangPlugins/LibJSGCTests/missing_visit_edges_method.cpp

@@ -8,7 +8,7 @@
 
 
 #include <LibJS/Runtime/Object.h>
 #include <LibJS/Runtime/Object.h>
 
 
-// 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 {
 class TestClass : public JS::Object {
     JS_OBJECT(TestClass, JS::Object);
     JS_OBJECT(TestClass, JS::Object);
 
 

+ 3 - 3
Tests/ClangPlugins/LibJSGCTests/wrapping_non_cell_member.cpp

@@ -12,10 +12,10 @@
 struct NotACell { };
 struct NotACell { };
 
 
 class TestClass {
 class TestClass {
-    // expected-warning@+1 {{Specialization type must inherit from JS::Cell}}
+    // expected-error@+1 {{Specialization type must inherit from JS::Cell}}
     JS::GCPtr<NotACell> m_member_1;
     JS::GCPtr<NotACell> 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<NotACell> m_member_2;
     JS::NonnullGCPtr<NotACell> 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<NotACell> m_member_3;
     JS::RawGCPtr<NotACell> m_member_3;
 };
 };