Skip to content
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
b1ef740
Add clang-tidy and clang-tidy-fix targets
andreiltd Apr 25, 2025
dd5ff1d
Merge branch 'main' into clang-tidy
andreiltd Jul 15, 2025
bb3fa2e
Merge branch 'main' into clang-tidy
andreiltd Aug 18, 2025
601b01c
Make linter target depend on starling-raw.wasm
andreiltd Jul 18, 2025
a49cfeb
fix: clang-analyzer errors
andreiltd Aug 18, 2025
518911b
fix: clang-analyzer errors in script loader
andreiltd Aug 18, 2025
5ab268c
fix: enable cppcoreguidelines checks and apply auto fixes
andreiltd Aug 18, 2025
7084086
fix: enable modernize lints and apply auto fixes
andreiltd Aug 18, 2025
567cd73
fix: use js_new for RefCounted types
andreiltd Aug 18, 2025
9f6b97d
fix: fix performance lint about unnecesairly large enum base
andreiltd Aug 18, 2025
37fddbc
fix: enable performace checks and appy autofixes
andreiltd Aug 18, 2025
4bfbb7d
fix: apply linter readability fixes
andreiltd Aug 18, 2025
98c9281
fix: do not use const members
andreiltd Aug 18, 2025
c589688
fix: remove unused declarations
andreiltd Aug 18, 2025
65f3344
ci: add CI step for linter
andreiltd Aug 18, 2025
2d9f230
Merge branch 'main' into clang-tidy
andreiltd Aug 18, 2025
8d0ca13
fix: apply fixes after merging main
andreiltd Aug 18, 2025
cbd70da
fix: enable no-malloc checks and apply fixes
andreiltd Aug 19, 2025
457b6c2
fix: allow implicit pointer to bool conversions
andreiltd Aug 19, 2025
3a572f6
Apply suggestions from code review
andreiltd Aug 19, 2025
7bf4316
fix: remove clang pragmas
andreiltd Aug 19, 2025
3860451
fix: bounds constant array index
andreiltd Aug 19, 2025
f9b099a
fix: enable special member functions lint
andreiltd Aug 19, 2025
aa664cb
fix: enable type const cast check
andreiltd Aug 19, 2025
c246ba4
fix: enable bugprone checks
andreiltd Aug 20, 2025
30f2b5d
fix: correct indentation
andreiltd Aug 20, 2025
1e9e30a
Apply suggestions from code review
andreiltd Aug 21, 2025
98ba4d1
fix: align declaration arguments with impl
andreiltd Aug 21, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions .clang-tidy
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
Checks: >
clang-analyzer-*,
clang-diagnostic-*,
cppcoreguidelines-*,
modernize-*,
performance-*,
readability-*,
misc-redundant-expression,
misc-uniqueptr-reset-release,
misc-unused-alias-decls,
misc-unused-using-decls,
-clang-analyzer-optin.core.EnumCastOutOfRange,
-cppcoreguidelines-avoid-do-while,
-cppcoreguidelines-avoid-c-arrays,
-cppcoreguidelines-pro-type-reinterpret-cast,
-cppcoreguidelines-avoid-magic-numbers,
-cppcoreguidelines-pro-bounds-array-to-pointer-decay,
-cppcoreguidelines-pro-type-const-cast,
-cppcoreguidelines-no-malloc,
-cppcoreguidelines-owning-memory,
-cppcoreguidelines-avoid-non-const-global-variables,
-cppcoreguidelines-pro-type-vararg,
-cppcoreguidelines-narrowing-conversions,
-cppcoreguidelines-pro-bounds-pointer-arithmetic,
-cppcoreguidelines-pro-type-cstyle-cast,
-cppcoreguidelines-special-member-functions,
-cppcoreguidelines-init-variables,
-cppcoreguidelines-macro-usage,
-cppcoreguidelines-interfaces-global-init,
-cppcoreguidelines-use-default-member-init,
-cppcoreguidelines-pro-bounds-constant-array-index,
-cppcoreguidelines-pro-type-static-cast-downcast,
-modernize-use-trailing-return-type,
-modernize-avoid-c-arrays,
-modernize-use-ranges,
-readability-identifier-length,
-readability-static-accessed-through-instance,
-readability-function-cognitive-complexity,
-readability-magic-numbers,
-readability-simplify-boolean-expr,

# Treat all warnings as errors
WarningsAsErrors: '*'

# Only warn on headers in the code tree
HeaderFilterRegex: 'builtins/|runtime/'

# Use the same style as clang-format (look for .clang-format)
FormatStyle: file

# Explicitly exclude system/dependency headers
SystemHeaders: false
7 changes: 7 additions & 0 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,13 @@ jobs:
name: spidermonkey-${{ matrix.build }}
path: spidermonkey-dist-${{ matrix.build }}/*

- name: Install Just
uses: taiki-e/install-action@just

- name: Run clang linter
if: matrix.build == 'debug'
run: just lint

release-spidermonkey:
needs: test
if: needs.test.outputs.SM_TAG_EXISTS == 'false' && (github.event_name == 'push' &&
Expand Down
1 change: 1 addition & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ include("spidermonkey")
include("openssl")
include("${HOST_API}/host_api.cmake")
include("build-crates")
include("lint")

option(ENABLE_JS_DEBUGGER "Enable support for debugging content via a socket connection" ON)

Expand Down
18 changes: 9 additions & 9 deletions builtins/web/abort/abort-controller.cpp
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
#include "abort-controller.h"
#include "abort-signal.h"

namespace builtins {
namespace web {
namespace abort {


namespace builtins::web::abort {

const JSFunctionSpec AbortController::static_methods[] = {
JS_FS_END,
Expand Down Expand Up @@ -35,7 +35,7 @@ bool AbortController::abort(JSContext *cx, unsigned argc, JS::Value *vp) {

RootedValue reason(cx, args.get(0));
RootedObject signal(cx, JS::GetReservedSlot(self, Slots::Signal).toObjectOrNull());
if (!signal) {
if (signal == nullptr) {
return false;
}

Expand All @@ -46,12 +46,12 @@ bool AbortController::constructor(JSContext *cx, unsigned argc, JS::Value *vp) {
CTOR_HEADER("AbortController", 0);

RootedObject self(cx, JS_NewObjectForConstructor(cx, &class_, args));
if (!self) {
if (self == nullptr) {
return false;
}

RootedObject signal(cx, AbortSignal::create(cx));
if (!signal) {
if (signal == nullptr) {
return false;
}

Expand All @@ -65,6 +65,6 @@ bool AbortController::init_class(JSContext *cx, JS::HandleObject global) {
return init_class_impl(cx, global);
}

} // namespace abort
} // namespace web
} // namespace builtins
} // namespace builtins::web::abort


14 changes: 7 additions & 7 deletions builtins/web/abort/abort-controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@

#include "builtin.h"

namespace builtins {
namespace web {
namespace abort {


namespace builtins::web::abort {

class AbortController : public BuiltinImpl<AbortController> {
static bool signal_get(JSContext *cx, unsigned argc, JS::Value *vp);
Expand All @@ -17,7 +17,7 @@ class AbortController : public BuiltinImpl<AbortController> {
static constexpr const char *class_name = "AbortController";
static constexpr unsigned ctor_length = 0;

enum Slots { Signal = 0, Count };
enum Slots : uint8_t { Signal = 0, Count };

static const JSFunctionSpec static_methods[];
static const JSPropertySpec static_properties[];
Expand All @@ -28,9 +28,9 @@ class AbortController : public BuiltinImpl<AbortController> {
static bool constructor(JSContext *cx, unsigned argc, Value *vp);
};

} // namespace abort
} // namespace web
} // namespace builtins
} // namespace builtins::web::abort





Expand Down
63 changes: 30 additions & 33 deletions builtins/web/abort/abort-signal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@
#include "../event/event.h"
#include "../timers.h"

namespace builtins {
namespace web {
namespace abort {


namespace builtins::web::abort {

using event::Event;
using event::EventTarget;
Expand Down Expand Up @@ -91,7 +91,7 @@ bool AbortSignal::timeout(JSContext *cx, unsigned argc, JS::Value *vp) {
}

RootedObject self(cx, create_with_timeout(cx, args.get(0)));
if (!self) {
if (self == nullptr) {
return false;
}

Expand All @@ -108,7 +108,7 @@ bool AbortSignal::abort(JSContext *cx, unsigned argc, JS::Value *vp) {

// The abort() method steps are inlined in the AbortSignal::create_with_reason method.
RootedObject self(cx, create_with_reason(cx, args.get(0)));
if (!self) {
if (self == nullptr) {
return false;
}

Expand All @@ -125,7 +125,7 @@ bool AbortSignal::any(JSContext *cx, unsigned argc, JS::Value *vp) {

// The any() method steps are inlined in the AbortSignal::create_with_signals method.
RootedObject self(cx, create_with_signals(cx, args));
if (!self) {
if (self == nullptr) {
return false;
}

Expand Down Expand Up @@ -195,7 +195,7 @@ bool AbortSignal::add_algorithm(JSObject *self, js::UniquePtr<AbortAlgorithm> al
}

// 2. Append algorithm to signal's abort algorithms.
auto algorithms = AbortSignal::algorithms(self);
auto *algorithms = AbortSignal::algorithms(self);
return algorithms->append(std::move(algorithm));
}

Expand Down Expand Up @@ -228,7 +228,7 @@ bool AbortSignal::abort(JSContext *cx, HandleObject self, HandleValue reason) {
JS::RootedObjectVector dep_signals_to_abort(cx);

// 4. For each dependentSignal of signal's dependent signals:
auto dep_signals = dependent_signals(self);
auto *dep_signals = dependent_signals(self);
for (auto const &sig : dep_signals->items()) {
RootedObject signal(cx, sig);
// 1. If dependentSignal is not aborted:
Expand Down Expand Up @@ -259,9 +259,10 @@ bool AbortSignal::abort(JSContext *cx, HandleObject self, HandleValue reason) {
bool AbortSignal::run_abort_steps(JSContext *cx, HandleObject self) {
// To run the abort steps for an AbortSignal signal:
// 1. For each algorithm of signal's abort algorithms: run algorithm.
auto algorithms = AbortSignal::algorithms(self);
auto *algorithms = AbortSignal::algorithms(self);
for (auto &algorithm : *algorithms) {
if (!algorithm->run(cx)) return false;
if (!algorithm->run(cx)) { return false;
}
}

// 2. Empty signals's abort algorithms.
Expand All @@ -274,11 +275,7 @@ bool AbortSignal::run_abort_steps(JSContext *cx, HandleObject self) {
RootedObject event(cx, Event::create(cx, type_val, JS::NullHandleValue));
RootedValue event_val(cx, JS::ObjectValue(*event));

if (!EventTarget::dispatch_event(cx, self, event_val, &res_val)) {
return false;
}

return true;
return EventTarget::dispatch_event(cx, self, event_val, &res_val);
}

// Set signal's abort reason to reason if it is given; otherwise to a new "AbortError" DOMException.
Expand All @@ -287,7 +284,7 @@ bool AbortSignal::set_reason(JSContext *cx, HandleObject self, HandleValue reaso
SetReservedSlot(self, Slots::Reason, reason);
} else {
RootedObject exception(cx, dom_exception::DOMException::create(cx, "AbortError", "AbortError"));
if (!exception) {
if (exception == nullptr) {
return false;
}

Expand All @@ -300,7 +297,7 @@ bool AbortSignal::set_reason(JSContext *cx, HandleObject self, HandleValue reaso
// https://dom.spec.whatwg.org/#interface-AbortSignal
JSObject *AbortSignal::create(JSContext *cx) {
RootedObject self(cx, JS_NewObjectWithGivenProto(cx, &class_, proto_obj));
if (!self) {
if (self == nullptr) {
return nullptr;
}

Expand Down Expand Up @@ -331,7 +328,7 @@ JSObject *AbortSignal::create(JSContext *cx) {
JSObject *AbortSignal::create_with_reason(JSContext *cx, HandleValue reason) {
// 1. Let signal be a new AbortSignal object.
RootedObject self(cx, create(cx));
if (!self) {
if (self == nullptr) {
return nullptr;
}

Expand All @@ -351,7 +348,7 @@ JSObject *AbortSignal::create_with_reason(JSContext *cx, HandleValue reason) {
JSObject *AbortSignal::create_with_timeout(JSContext *cx, HandleValue timeout) {
// 1. Let signal be a new AbortSignal object.
RootedObject self(cx, create(cx));
if (!self) {
if (self == nullptr) {
return nullptr;
}

Expand All @@ -371,11 +368,11 @@ JSObject *AbortSignal::create_with_timeout(JSContext *cx, HandleValue timeout) {
int32_t timer_id = 0;

JS::RootedObject exception(cx, dom_exception::DOMException::create(cx, "TimeoutError", "TimeoutError"));
if (!exception) {
if (exception == nullptr) {
return nullptr;
}
JS::RootedFunction on_timeout(cx, JS_NewFunction(cx, AbortSignal::on_timeout, 2, 0, nullptr));
if (!on_timeout) {
if (on_timeout == nullptr) {
return nullptr;
}

Expand Down Expand Up @@ -403,7 +400,7 @@ JSObject *AbortSignal::create_with_signals(JSContext *cx, HandleValueArray signa

// 1. Let resultSignal be a new object implementing signalInterface using realm.
RootedObject self(cx, create(cx));
if (!self) {
if (self == nullptr) {
return nullptr;
}

Expand All @@ -420,7 +417,7 @@ JSObject *AbortSignal::create_with_signals(JSContext *cx, HandleValueArray signa

// 3. Set resultSignal's dependent to true.
SetReservedSlot(self, Slots::Dependent, JS::TrueValue());
auto our_signals = source_signals(self);
auto *our_signals = source_signals(self);

// 4. For each signal of signals:
for (size_t i = 0; i < signals.length(); ++i) {
Expand All @@ -431,20 +428,20 @@ JSObject *AbortSignal::create_with_signals(JSContext *cx, HandleValueArray signa
// 1. Append signal to resultSignal's source signals.
our_signals->insert(signal);
// 2. Append resultSignal to signal's dependent signals.
auto their_signals = dependent_signals(signal);
auto *their_signals = dependent_signals(signal);
their_signals->insert(self);
}
// 2. Otherwise...
else {
auto src_signals = source_signals(signal);
auto *src_signals = source_signals(signal);
// for each sourceSignal of signal's source signals:
for (auto const &source : src_signals->items()) {
// 1. Assert: sourceSignal is not aborted and not dependent.
MOZ_ASSERT(!is_aborted(source) && !is_dependent(source));
// 2. Append sourceSignal to resultSignal's source signals.
our_signals->insert(source);
// 3. Append resultSignal to sourceSignal's dependent signals.
auto their_signals = dependent_signals(source);
auto *their_signals = dependent_signals(source);
their_signals->insert(self);
};
}
Expand All @@ -469,21 +466,21 @@ void AbortSignal::trace(JSTracer *trc, JSObject *self) {

auto has_sources = !JS::GetReservedSlot(self, Slots::SourceSignals).isNullOrUndefined();
if (has_sources) {
auto srcsig = source_signals(self);
auto *srcsig = source_signals(self);
srcsig->trace(trc);
srcsig->traceWeak(trc);
}

auto has_deps = !JS::GetReservedSlot(self, Slots::DependentSignals).isNullOrUndefined();
if (has_deps) {
auto depsig = dependent_signals(self);
auto *depsig = dependent_signals(self);
depsig->trace(trc);
depsig->traceWeak(trc);
}

auto has_algorithms = !JS::GetReservedSlot(self, Slots::Algorithms).isNullOrUndefined();
if (has_algorithms) {
auto algorithms = AbortSignal::algorithms(self);
auto *algorithms = AbortSignal::algorithms(self);
algorithms->trace(trc);
}
}
Expand All @@ -495,7 +492,7 @@ bool AbortSignal::init_class(JSContext *cx, JS::HandleObject global) {
return false;
}

if (!(abort_type_atom = JS_AtomizeAndPinString(cx, "abort"))) {
if ((abort_type_atom = JS_AtomizeAndPinString(cx, "abort")) == nullptr) {
return false;
}

Expand All @@ -515,6 +512,6 @@ bool install(api::Engine *engine) {

JSString *AbortSignal::abort_type_atom = nullptr;

} // namespace abort
} // namespace web
} // namespace builtins
} // namespace builtins::web::abort


Loading