From 65232b6681f33fd42508adf167100c155559274d Mon Sep 17 00:00:00 2001 From: Daniel Bertalan Date: Sun, 13 Aug 2023 12:53:46 +0200 Subject: [PATCH] LibJS: Mark classes and virtual functions `final` where possible These cases were found with GCC's `-Wsuggest-final-{types,methods}` warnings, which catch calls that could have been devirtualized had we declared the functions `final` in the source. To reproduce, Link Time Optimization needs to be enabled. The easiest way to achieve this is to set the `CMAKE_INTERPROCEDURAL_OPTIMIZATION` cache variable to `ON`. The `.incbin` directive in LibCompress' Brotli decompressor might needs to be changed to an absolute path for this to work. This commit also removes a pair of unused virtual functions. --- Userland/Libraries/LibJS/AST.h | 2 +- Userland/Libraries/LibJS/CyclicModule.h | 8 ++++---- Userland/Libraries/LibJS/Heap/Cell.h | 1 - Userland/Libraries/LibJS/Heap/MarkedVector.h | 2 +- Userland/Libraries/LibJS/Runtime/Array.h | 8 ++++---- .../Libraries/LibJS/Runtime/DeclarativeEnvironment.h | 10 +++++----- Userland/Libraries/LibJS/Runtime/Environment.h | 2 -- Userland/Libraries/LibJS/Runtime/Object.h | 2 +- Userland/Libraries/LibJS/Runtime/ObjectEnvironment.h | 2 +- 9 files changed, 17 insertions(+), 20 deletions(-) diff --git a/Userland/Libraries/LibJS/AST.h b/Userland/Libraries/LibJS/AST.h index 68ac11575d..00d45fbf1f 100644 --- a/Userland/Libraries/LibJS/AST.h +++ b/Userland/Libraries/LibJS/AST.h @@ -156,7 +156,7 @@ public: }; // 14.13 Labelled Statements, https://tc39.es/ecma262/#sec-labelled-statements -class LabelledStatement : public Statement { +class LabelledStatement final : public Statement { public: LabelledStatement(SourceRange source_range, DeprecatedFlyString label, NonnullRefPtr labelled_item) : Statement(move(source_range)) diff --git a/Userland/Libraries/LibJS/CyclicModule.h b/Userland/Libraries/LibJS/CyclicModule.h index 06d2a3f32d..55ce09ffdc 100644 --- a/Userland/Libraries/LibJS/CyclicModule.h +++ b/Userland/Libraries/LibJS/CyclicModule.h @@ -27,8 +27,8 @@ class CyclicModule : public Module { public: // Note: Do not call these methods directly unless you are HostResolveImportedModule. // Badges cannot be used because other hosts must be able to call this (and it is called recursively) - virtual ThrowCompletionOr link(VM& vm) override; - virtual ThrowCompletionOr evaluate(VM& vm) override; + virtual ThrowCompletionOr link(VM& vm) override final; + virtual ThrowCompletionOr evaluate(VM& vm) override final; Vector const& requested_modules() const { return m_requested_modules; } @@ -37,8 +37,8 @@ protected: virtual void visit_edges(Cell::Visitor&) override; - virtual ThrowCompletionOr inner_module_linking(VM& vm, Vector& stack, u32 index) override; - virtual ThrowCompletionOr inner_module_evaluation(VM& vm, Vector& stack, u32 index) override; + virtual ThrowCompletionOr inner_module_linking(VM& vm, Vector& stack, u32 index) override final; + virtual ThrowCompletionOr inner_module_evaluation(VM& vm, Vector& stack, u32 index) override final; virtual ThrowCompletionOr initialize_environment(VM& vm); virtual ThrowCompletionOr execute_module(VM& vm, GCPtr capability = {}); diff --git a/Userland/Libraries/LibJS/Heap/Cell.h b/Userland/Libraries/LibJS/Heap/Cell.h index 46f0bffa5a..e7fe042b87 100644 --- a/Userland/Libraries/LibJS/Heap/Cell.h +++ b/Userland/Libraries/LibJS/Heap/Cell.h @@ -87,7 +87,6 @@ public: virtual ~Visitor() = default; }; - virtual bool is_environment() const { return false; } virtual void visit_edges(Visitor&) { } // This will be called on unmarked objects by the garbage collector in a separate pass before destruction. diff --git a/Userland/Libraries/LibJS/Heap/MarkedVector.h b/Userland/Libraries/LibJS/Heap/MarkedVector.h index 8710c32689..3a28d8462b 100644 --- a/Userland/Libraries/LibJS/Heap/MarkedVector.h +++ b/Userland/Libraries/LibJS/Heap/MarkedVector.h @@ -33,7 +33,7 @@ public: }; template -class MarkedVector +class MarkedVector final : public MarkedVectorBase , public Vector { diff --git a/Userland/Libraries/LibJS/Runtime/Array.h b/Userland/Libraries/LibJS/Runtime/Array.h index 4b010a4615..9da03e401e 100644 --- a/Userland/Libraries/LibJS/Runtime/Array.h +++ b/Userland/Libraries/LibJS/Runtime/Array.h @@ -40,10 +40,10 @@ public: virtual ~Array() override = default; - virtual ThrowCompletionOr> internal_get_own_property(PropertyKey const&) const override; - virtual ThrowCompletionOr internal_define_own_property(PropertyKey const&, PropertyDescriptor const&) override; - virtual ThrowCompletionOr internal_delete(PropertyKey const&) override; - virtual ThrowCompletionOr> internal_own_property_keys() const override; + virtual ThrowCompletionOr> internal_get_own_property(PropertyKey const&) const override final; + virtual ThrowCompletionOr internal_define_own_property(PropertyKey const&, PropertyDescriptor const&) override final; + virtual ThrowCompletionOr internal_delete(PropertyKey const&) override final; + virtual ThrowCompletionOr> internal_own_property_keys() const override final; [[nodiscard]] bool length_is_writable() const { return m_length_writable; } diff --git a/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.h b/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.h index f03aabc057..b167deb7b1 100644 --- a/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.h +++ b/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.h @@ -32,11 +32,11 @@ public: virtual ~DeclarativeEnvironment() override = default; - virtual ThrowCompletionOr has_binding(DeprecatedFlyString const& name, Optional* = nullptr) const override; - virtual ThrowCompletionOr create_mutable_binding(VM&, DeprecatedFlyString const& name, bool can_be_deleted) override; - virtual ThrowCompletionOr create_immutable_binding(VM&, DeprecatedFlyString const& name, bool strict) override; - virtual ThrowCompletionOr initialize_binding(VM&, DeprecatedFlyString const& name, Value, InitializeBindingHint) override; - virtual ThrowCompletionOr set_mutable_binding(VM&, DeprecatedFlyString const& name, Value, bool strict) override; + virtual ThrowCompletionOr has_binding(DeprecatedFlyString const& name, Optional* = nullptr) const override final; + virtual ThrowCompletionOr create_mutable_binding(VM&, DeprecatedFlyString const& name, bool can_be_deleted) override final; + virtual ThrowCompletionOr create_immutable_binding(VM&, DeprecatedFlyString const& name, bool strict) override final; + virtual ThrowCompletionOr initialize_binding(VM&, DeprecatedFlyString const& name, Value, InitializeBindingHint) override final; + virtual ThrowCompletionOr set_mutable_binding(VM&, DeprecatedFlyString const& name, Value, bool strict) override final; virtual ThrowCompletionOr get_binding_value(VM&, DeprecatedFlyString const& name, bool strict) override; virtual ThrowCompletionOr delete_binding(VM&, DeprecatedFlyString const& name) override; diff --git a/Userland/Libraries/LibJS/Runtime/Environment.h b/Userland/Libraries/LibJS/Runtime/Environment.h index f79dc9f183..480057abca 100644 --- a/Userland/Libraries/LibJS/Runtime/Environment.h +++ b/Userland/Libraries/LibJS/Runtime/Environment.h @@ -63,8 +63,6 @@ protected: virtual void visit_edges(Visitor&) override; private: - virtual bool is_environment() const final { return true; } - bool m_permanently_screwed_by_eval { false }; GCPtr m_outer_environment; diff --git a/Userland/Libraries/LibJS/Runtime/Object.h b/Userland/Libraries/LibJS/Runtime/Object.h index ce55de9e59..b414c6b64a 100644 --- a/Userland/Libraries/LibJS/Runtime/Object.h +++ b/Userland/Libraries/LibJS/Runtime/Object.h @@ -164,7 +164,7 @@ public: void define_direct_accessor(PropertyKey const&, FunctionObject* getter, FunctionObject* setter, PropertyAttributes attributes); using IntrinsicAccessor = Value (*)(Realm&); - virtual void define_intrinsic_accessor(PropertyKey const&, PropertyAttributes attributes, IntrinsicAccessor accessor); + void define_intrinsic_accessor(PropertyKey const&, PropertyAttributes attributes, IntrinsicAccessor accessor); void define_native_function(Realm&, PropertyKey const&, SafeFunction(VM&)>, i32 length, PropertyAttributes attributes); void define_native_accessor(Realm&, PropertyKey const&, SafeFunction(VM&)> getter, SafeFunction(VM&)> setter, PropertyAttributes attributes); diff --git a/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.h b/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.h index cc5d600b28..f38fdd75d6 100644 --- a/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.h +++ b/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.h @@ -10,7 +10,7 @@ namespace JS { -class ObjectEnvironment : public Environment { +class ObjectEnvironment final : public Environment { JS_ENVIRONMENT(ObjectEnvironment, Environment); public: