From f6eb8a9be9a70a647347bccead80791b63d050d7 Mon Sep 17 00:00:00 2001 From: Mike Aizatsky Date: Wed, 28 Oct 2026 15:31:09 -0700 Subject: [PATCH] clang-tidy-ctor-initializer-this-capture --- build/tools/clang_tidy/clang_tidy.bazelrc | 4 + tools/clang-tidy/BUILD.bazel | 21 +++ ...initializer-this-capture-negative-test.c++ | 83 +++++++++ ...initializer-this-capture-positive-test.c++ | 159 ++++++++++++++++++ .../ctor-initializer-this-capture-test.sh | 112 ++++++++++++ .../ctor-initializer-this-capture.c++ | 145 ++++++++++++++++ .../ctor-initializer-this-capture.h | 51 ++++++ tools/clang-tidy/workerd-lint.c++ | 4 + 8 files changed, 579 insertions(+) create mode 100644 tools/clang-tidy/ctor-initializer-this-capture-negative-test.c++ create mode 100644 tools/clang-tidy/ctor-initializer-this-capture-positive-test.c++ create mode 100755 tools/clang-tidy/ctor-initializer-this-capture-test.sh create mode 100644 tools/clang-tidy/ctor-initializer-this-capture.c++ create mode 100644 tools/clang-tidy/ctor-initializer-this-capture.h diff --git a/build/tools/clang_tidy/clang_tidy.bazelrc b/build/tools/clang_tidy/clang_tidy.bazelrc index d817fd47c61..c4743071857 100644 --- a/build/tools/clang_tidy/clang_tidy.bazelrc +++ b/build/tools/clang_tidy/clang_tidy.bazelrc @@ -11,3 +11,7 @@ build:clang-tidy-warnings --config=clang-tidy --aspects_parameters=clang_tidy_ar # Unsafe continuation captures: flags bare references, raw pointers, and # non-owning views captured into lambdas passed to async sinks. build:clang-tidy-unsafe-continuation-capture --config=clang-tidy --aspects_parameters=clang_tidy_args=--checks=workerd-unsafe-continuation-capture + +# Lambdas capturing `this` in constructor initializer lists, before the object +# is fully constructed. Not yet clean on existing code. +build:clang-tidy-ctor-initializer-this-capture --config=clang-tidy --aspects_parameters=clang_tidy_args=--checks=workerd-ctor-initializer-this-capture diff --git a/tools/clang-tidy/BUILD.bazel b/tools/clang-tidy/BUILD.bazel index 872c177d475..b511964c82e 100644 --- a/tools/clang-tidy/BUILD.bazel +++ b/tools/clang-tidy/BUILD.bazel @@ -10,6 +10,9 @@ WD_CONSUME are made through consume() when invoked on kj::Ptr. - `workerd-coroutine-hostile-raii`: flags hostile RAII objects that persist across coroutine suspension. + - `workerd-ctor-initializer-this-capture`: flags lambdas that capture `this` + in a constructor's mem-initializer list, before the object is fully + constructed. - `workerd-legacy-stream-alloc`: flags direct allocation of the legacy C++ ReadableStream / WritableStream (`js.alloc(...)`) outside JsReadableStream::create() / JsWritableStream::create(), the dispatch @@ -44,6 +47,7 @@ filegroup( "angled-includes.c++", "consume.c++", "coroutine-hostile-raii.c++", + "ctor-initializer-this-capture.c++", "legacy-stream-alloc.c++", "promise-ignore-result.c++", "unsafe-continuation-capture.c++", @@ -61,6 +65,7 @@ filegroup( "angled-includes.h", "consume.h", "coroutine-hostile-raii.h", + "ctor-initializer-this-capture.h", "legacy-stream-alloc.h", "promise-ignore-result.h", "unsafe-continuation-capture.h", @@ -164,6 +169,22 @@ sh_test( ], ) +sh_test( + name = "ctor-initializer-this-capture-test", + srcs = ["ctor-initializer-this-capture-test.sh"], + data = [ + ":ctor-initializer-this-capture-negative-test.c++", + ":ctor-initializer-this-capture-positive-test.c++", + ":workerd-lint", + "//tools:clang-tidy", + ], + tags = ["no-asan"], + target_compatible_with = [ + "@platforms//os:linux", + "@platforms//cpu:x86_64", + ], +) + sh_test( name = "legacy-stream-alloc-test", srcs = ["legacy-stream-alloc-test.sh"], diff --git a/tools/clang-tidy/ctor-initializer-this-capture-negative-test.c++ b/tools/clang-tidy/ctor-initializer-this-capture-negative-test.c++ new file mode 100644 index 00000000000..8e5f8979538 --- /dev/null +++ b/tools/clang-tidy/ctor-initializer-this-capture-negative-test.c++ @@ -0,0 +1,83 @@ +// Copyright (c) 2017-2026 Cloudflare, Inc. +// Licensed under the Apache 2.0 license found in the LICENSE file or at: +// https://opensource.org/licenses/Apache-2.0 + +// Negative fixtures for workerd-ctor-initializer-this-capture: none of these +// may produce a diagnostic. + +namespace fixtures { + +class Callback { + public: + Callback() = default; + template + Callback(F&& f) {} // NOLINT(google-explicit-constructor) +}; + +template +int compute(F&& f) { + return 0; +} + +class Widget { + public: + // Case N1: the lambda is created in the constructor body, after every + // member has been initialized. + explicit Widget(int) { + onEvent = [this]() { handle(); }; + } + + // Case N2: lambdas that capture nothing, or only constructor parameters. + Widget(int a, int& b): onEvent([a, &b]() { b += a; }), other([] {}), value(a) {} + + // Case N3: an immediately invoked lambda does not outlive the initializer. + explicit Widget(long v) + : onEvent([] {}), + other([] {}), + value([&]() { return v > 0 ? first : static_cast(v); }()) {} + + // Case N4: same, with an explicit `this` capture. + explicit Widget(short v): onEvent([] {}), other([] {}), value([this]() { return first; }()) {} + + // Case N5: a lambda passed to a function that captures only parameters. + explicit Widget(char v): onEvent([] {}), other([] {}), value(compute([v]() { return v; })) {} + + // Case N6: explicit suppression. + explicit Widget(bool) + : onEvent([this]() { handle(); }), // NOLINT(workerd-ctor-initializer-this-capture) + other([] {}), + value(0) {} + + void handle() { + // Case N7: capturing `this` in an ordinary member function. + onEvent = [this]() { handle(); }; + } + + private: + int first = 0; + // Case N8: default member initializers are not checked. + Callback onEvent = [this]() { handle(); }; + Callback other; + int value; +}; + +// Case N9: a lambda in a constructor body of a class nested in another +// constructor's initializer is not part of that initializer's object. +class Outer { + public: + Outer() + : callback([]() { + struct Local { + Local() { + cb = [this]() {}; + } + Callback cb; + }; + Local local; + }) {} + + private: + Callback callback; +}; + +} // namespace fixtures diff --git a/tools/clang-tidy/ctor-initializer-this-capture-positive-test.c++ b/tools/clang-tidy/ctor-initializer-this-capture-positive-test.c++ new file mode 100644 index 00000000000..4f3ecd4910d --- /dev/null +++ b/tools/clang-tidy/ctor-initializer-this-capture-positive-test.c++ @@ -0,0 +1,159 @@ +// Copyright (c) 2017-2026 Cloudflare, Inc. +// Licensed under the Apache 2.0 license found in the LICENSE file or at: +// https://opensource.org/licenses/Apache-2.0 + +// Positive fixtures for workerd-ctor-initializer-this-capture: every case must +// produce exactly one diagnostic. + +namespace fixtures { + +// Type-erased stand-in for kj::Function: stores a callable. +class Callback { + public: + template + Callback(F&& f) {} // NOLINT(google-explicit-constructor) + Callback(const Callback&) = default; +}; + +class Promise { + public: + template + Promise then(F&& f) { + return *this; + } +}; + +Promise start(); + +template +Callback wrap(F&& f) { + return Callback(static_cast(f)); +} + +class Base { + public: + explicit Base(Callback callback): callback(callback) {} + + private: + Callback callback; +}; + +class Widget: public Base { + public: + // Case P1: explicit `this` capture. + explicit Widget(int): Base([] {}), onEvent([this]() { handle(); }), task(start()), value(0) {} + + // Case P2: `*this` copies the partially constructed object. + explicit Widget(long) + : Base([] {}), + onEvent([*this]() mutable { handle(); }), + task(start()), + value(0) {} + + // Case P3: implicit capture through `[&]`. + explicit Widget(short): Base([] {}), onEvent([&]() { handle(); }), task(start()), value(0) {} + + // Case P4: implicit capture through `[=]`. + explicit Widget(char): Base([] {}), onEvent([=]() { handle(); }), task(start()), value(0) {} + + // Case P5: base-class initializer. + explicit Widget(float): Base([this]() { handle(); }), onEvent([] {}), task(start()), value(0) {} + + // Case P6: delegating initializer. + explicit Widget(double): Widget(wrap([this]() { handle(); })) {} + + // Case P7: init-capture of `this`. + explicit Widget(unsigned) + : Base([] {}), + onEvent([self = this]() { self->handle(); }), + task(start()), + value(0) {} + + // Case P8: init-capture of a reference to `*this`. + explicit Widget(unsigned long) + : Base([] {}), + onEvent([&self = *this]() { self.handle(); }), + task(start()), + value(0) {} + + // Case P9: lambda passed to a continuation inside a larger expression. + explicit Widget(unsigned short) + : Base([] {}), + onEvent([] {}), + task(start().then([this]() { handle(); })), + value(0) {} + + // Case P10: lambda that escapes from an immediately invoked lambda. + explicit Widget(unsigned char) + : Base([] {}), + onEvent([&]() { return wrap([this]() { handle(); }); }()), + task(start()), + value(0) {} + + // Case P11: lambda created by an init-capture of a lambda that does not + // itself capture `this`. + explicit Widget(bool) + : Base([] {}), + onEvent([inner = wrap([this]() { handle(); })]() {}), + task(start()), + value(0) {} + + // Case P12: explicit `this` alongside a capture default. + Widget(int, int): Base([] {}), onEvent([&, this]() { handle(); }), task(start()), value(0) {} + + void handle() {} + + private: + explicit Widget(Callback callback): Base(callback), onEvent(callback), task(start()), value(0) {} + + Callback onEvent; + Promise task; + int value; +}; + +// Case P13: constructor defined out of line. +class OutOfLine { + public: + OutOfLine(); + void handle() {} + + private: + Callback onEvent; +}; + +OutOfLine::OutOfLine(): onEvent([this]() { handle(); }) {} + +// Case P14: class template, reported once despite two instantiations. +template +class Holder { + public: + Holder(): onEvent([this]() { handle(); }) {} + void handle() {} + + private: + Callback onEvent; +}; + +// Case P15: class template whose capture default captures `this` only through +// a dependent member access. +template +class DependentHolder { + public: + DependentHolder(): onEvent([&]() { value.handle(); }) {} + + private: + T value; + Callback onEvent; +}; + +struct Handler { + void handle() {} +}; + +void instantiate() { + Holder a; + Holder b; + DependentHolder c; +} + +} // namespace fixtures diff --git a/tools/clang-tidy/ctor-initializer-this-capture-test.sh b/tools/clang-tidy/ctor-initializer-this-capture-test.sh new file mode 100755 index 00000000000..593ed81f41c --- /dev/null +++ b/tools/clang-tidy/ctor-initializer-this-capture-test.sh @@ -0,0 +1,112 @@ +#!/usr/bin/env bash + +# Copyright (c) 2017-2026 Cloudflare, Inc. +# Licensed under the Apache 2.0 license found in the LICENSE file or at: +# https://opensource.org/licenses/Apache-2.0 + +set -euo pipefail + +readonly ROOT="${TEST_SRCDIR}/${TEST_WORKSPACE}" +readonly CLANG_TIDY="${ROOT}/tools/clang_tidy" +readonly PLUGIN="${ROOT}/tools/clang-tidy/libworkerd-lint.so" +readonly POSITIVE="${ROOT}/tools/clang-tidy/ctor-initializer-this-capture-positive-test.c++" +readonly NEGATIVE="${ROOT}/tools/clang-tidy/ctor-initializer-this-capture-negative-test.c++" +readonly CHECK="workerd-ctor-initializer-this-capture" +readonly CHECKS="-*,${CHECK}" + +readonly EXPLICIT_MESSAGE="captures 'this' before the object is fully constructed" +readonly IMPLICIT_MESSAGE="implicitly captures 'this' through its capture default" + +set +e +positive_output=$("${CLANG_TIDY}" "--load=${PLUGIN}" --checks="${CHECKS}" \ + --warnings-as-errors='*' "${POSITIVE}" -- -std=c++23 2>&1) +positive_status=$? +set -e + +if [[ ${positive_status} -eq 0 ]]; then + printf '%s\n' "Expected ${CHECK} to reject the positive fixtures." >&2 + printf '%s\n' "${positive_output}" >&2 + exit 1 +fi + +# Asserts that a diagnostic with the given message is reported at the given +# fixture line and column, identifying the case that produced it. +expect_diag_at() { + local location="$1" + local message="$2" + local description="$3" + if ! grep -F "ctor-initializer-this-capture-positive-test.c++:${location}:" \ + <<<"${positive_output}" | grep -qF "${message}"; then + printf 'Missing expected diagnostic (%s) at %s: %s\n' \ + "${description}" "${location}" "${message}" >&2 + printf '%s\n' "${positive_output}" >&2 + exit 1 + fi +} + +# Resolves the line:column of a needle within a unique snippet of a fixture +# case, so the assertions below do not depend on hard-coded line numbers. +location_of() { + local snippet="$1" + local needle="$2" + local line + line=$(grep -nF -- "${snippet}" "${POSITIVE}" | head -n1 | cut -d: -f1) + if [[ -z "${line}" ]]; then + printf 'Fixture snippet not found: %s\n' "${snippet}" >&2 + exit 1 + fi + local text + text=$(sed -n "${line}p" "${POSITIVE}") + local prefix="${text%%"${needle}"*}" + printf '%s:%s' "${line}" "$((${#prefix} + 1))" +} + +expect_diag_at "$(location_of 'onEvent([this]() { handle(); }),' 'this')" \ + "'onEvent' ${EXPLICIT_MESSAGE}" "P1: explicit this" +expect_diag_at "$(location_of 'onEvent([*this]() mutable' '*this')" \ + "'onEvent' ${EXPLICIT_MESSAGE}" "P2: *this" +expect_diag_at "$(location_of 'onEvent([&]() { handle(); }),' '&')" \ + "'onEvent' ${IMPLICIT_MESSAGE}" "P3: implicit through [&]" +expect_diag_at "$(location_of 'onEvent([=]() { handle(); }),' '=')" \ + "'onEvent' ${IMPLICIT_MESSAGE}" "P4: implicit through [=]" +expect_diag_at "$(location_of ': Base([this]() { handle(); }),' 'this')" \ + "'Base' ${EXPLICIT_MESSAGE}" "P5: base initializer" +expect_diag_at "$(location_of 'Widget(wrap([this]() { handle(); }))' 'this')" \ + "'Widget' ${EXPLICIT_MESSAGE}" "P6: delegating initializer" +expect_diag_at "$(location_of 'onEvent([self = this]()' 'self')" \ + "'onEvent' ${EXPLICIT_MESSAGE}" "P7: init-capture of this" +expect_diag_at "$(location_of 'onEvent([&self = *this]()' 'self')" \ + "'onEvent' ${EXPLICIT_MESSAGE}" "P8: init-capture of *this" +expect_diag_at "$(location_of 'task(start().then([this]()' 'this')" \ + "'task' ${EXPLICIT_MESSAGE}" "P9: continuation" +expect_diag_at "$(location_of 'return wrap([this]() { handle(); }); }()),' 'this')" \ + "'onEvent' ${EXPLICIT_MESSAGE}" "P10: escapes from an immediately invoked lambda" +expect_diag_at "$(location_of 'onEvent([inner = wrap([this]()' 'this')" \ + "'onEvent' ${EXPLICIT_MESSAGE}" "P11: inside an init-capture" +expect_diag_at "$(location_of 'onEvent([&, this]()' 'this')" \ + "'onEvent' ${EXPLICIT_MESSAGE}" "P12: [&, this]" +expect_diag_at "$(location_of 'OutOfLine::OutOfLine(): onEvent([this]()' 'this')" \ + "'onEvent' ${EXPLICIT_MESSAGE}" "P13: out-of-line constructor" +expect_diag_at "$(location_of 'Holder(): onEvent([this]()' 'this')" \ + "'onEvent' ${EXPLICIT_MESSAGE}" "P14: class template" +expect_diag_at "$(location_of 'DependentHolder(): onEvent([&]()' '&')" \ + "'onEvent' ${IMPLICIT_MESSAGE}" "P15: dependent capture default" + +# Exact-count lock: one diagnostic per positive case, no more, no fewer. +expected_count=15 +actual_count=$(grep -c "\[${CHECK}" <<<"${positive_output}" || true) +if [[ "${actual_count}" -ne "${expected_count}" ]]; then + printf 'Expected exactly %s %s diagnostics, got %s\n' \ + "${expected_count}" "${CHECK}" "${actual_count}" >&2 + printf '%s\n' "${positive_output}" >&2 + exit 1 +fi + +negative_output=$("${CLANG_TIDY}" "--load=${PLUGIN}" --checks="${CHECKS}" \ + --warnings-as-errors='*' "${NEGATIVE}" -- -std=c++23 2>&1) + +if [[ "${negative_output}" == *"${CHECK}"* ]]; then + printf '%s\n' "Expected the negative fixtures to be accepted." >&2 + printf '%s\n' "${negative_output}" >&2 + exit 1 +fi diff --git a/tools/clang-tidy/ctor-initializer-this-capture.c++ b/tools/clang-tidy/ctor-initializer-this-capture.c++ new file mode 100644 index 00000000000..7d85bb5f5a8 --- /dev/null +++ b/tools/clang-tidy/ctor-initializer-this-capture.c++ @@ -0,0 +1,145 @@ +// Copyright (c) 2017-2026 Cloudflare, Inc. +// Licensed under the Apache 2.0 license found in the LICENSE file or at: +// https://opensource.org/licenses/Apache-2.0 + +#include "ctor-initializer-this-capture.h" + +#include "clang/AST/DeclCXX.h" +#include "clang/AST/ExprCXX.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" + +using namespace clang; +using namespace clang::ast_matchers; + +namespace workerd::clang_tidy { + +namespace { + +// The lambda called by `S`, if `S` calls a lambda expression directly, as in `[&]() { ... }()`. +const LambdaExpr* immediatelyInvokedLambda(const Stmt& S) { + if (const auto* Call = dyn_cast(&S)) { + if (Call->getOperator() == OO_Call && Call->getNumArgs() > 0) { + return dyn_cast(Call->getArg(0)->IgnoreUnlessSpelledInSource()); + } + return nullptr; + } + // Inside a template, a call to a lambda can stay an unresolved CallExpr whose callee is the + // lambda itself. + if (const auto* Call = dyn_cast(&S)) { + if (const Expr* Callee = Call->getCallee()) { + return dyn_cast(Callee->IgnoreUnlessSpelledInSource()); + } + } + return nullptr; +} + +// Whether `Init` evaluates to the object under construction itself: `this` or `*this`. +bool isThisObject(const Expr* Init) { + if (Init == nullptr) return false; + const Expr* E = Init->IgnoreUnlessSpelledInSource(); + if (const auto* Deref = dyn_cast(E)) { + if (Deref->getOpcode() != UO_Deref) return false; + E = Deref->getSubExpr()->IgnoreUnlessSpelledInSource(); + } + return isa(E); +} + +// Describes what `Init` initializes, for the diagnostic: a member name or a base class type. +std::string initializedName(const CXXCtorInitializer& Init, const ASTContext& Context) { + if (const FieldDecl* Member = Init.getAnyMember()) { + return Member->getNameAsString(); + } + if (const TypeSourceInfo* Type = Init.getTypeSourceInfo()) { + return Type->getType().getAsString(Context.getPrintingPolicy()); + } + return ""; +} + +} // namespace + +void CtorInitializerThisCaptureCheck::registerMatchers(MatchFinder* Finder) { + // Implicit initializers cannot contain a lambda written in the constructor, so only written ones + // are inspected. Template patterns and their instantiations are both matched, since a capture + // default may only resolve to a `this` capture once instantiated; clang-tidy merges the + // resulting duplicate diagnostics. + Finder->addMatcher(cxxConstructorDecl(forEachConstructorInitializer( + cxxCtorInitializer(isWritten()).bind("init"))), + this); +} + +void CtorInitializerThisCaptureCheck::check(const MatchFinder::MatchResult& Result) { + const auto* Init = Result.Nodes.getNodeAs("init"); + if (Init == nullptr || Init->getInit() == nullptr) return; + checkExpr(Init->getInit(), initializedName(*Init, *Result.Context)); +} + +void CtorInitializerThisCaptureCheck::checkExpr(const Stmt* S, llvm::StringRef InitializedName) { + if (S == nullptr) return; + + if (const LambdaExpr* Invoked = immediatelyInvokedLambda(*S)) { + // The lambda does not outlive the call, so its own captures are harmless, but a lambda it + // creates may still escape. Its capture initializers and the call's arguments are evaluated in + // the initializer, too. + for (const Stmt* Child: S->children()) { + const auto* ChildExpr = dyn_cast_or_null(Child); + if (ChildExpr != nullptr && ChildExpr->IgnoreUnlessSpelledInSource() == Invoked) { + for (const Stmt* LambdaChild: Invoked->children()) { + checkExpr(LambdaChild, InitializedName); + } + } else { + checkExpr(Child, InitializedName); + } + } + return; + } + + if (const auto* Lambda = dyn_cast(S)) { + checkLambda(*Lambda, InitializedName); + return; + } + + for (const Stmt* Child: S->children()) { + checkExpr(Child, InitializedName); + } +} + +void CtorInitializerThisCaptureCheck::checkLambda( + const LambdaExpr& Lambda, llvm::StringRef InitializedName) { + for (const LambdaCapture& Capture: Lambda.captures()) { + bool CapturesObject = Capture.capturesThis(); + if (!CapturesObject && Capture.capturesVariable()) { + const auto* Var = dyn_cast(Capture.getCapturedVar()); + CapturesObject = Var != nullptr && Var->isInitCapture() && isThisObject(Var->getInit()); + } + if (!CapturesObject) continue; + + if (Capture.isImplicit()) { + diag(Lambda.getCaptureDefaultLoc(), + "lambda in the initializer of '%0' implicitly captures 'this' through its capture " + "default before the object is fully constructed; create the lambda in the constructor " + "body instead") + << InitializedName << Lambda.getIntroducerRange(); + } else { + diag(Capture.getLocation(), + "lambda in the initializer of '%0' captures 'this' before the object is fully " + "constructed; create the lambda in the constructor body instead") + << InitializedName << Lambda.getIntroducerRange(); + } + // One diagnostic per lambda. Any lambda nested in this one that captures `this` needs this + // one to capture it too, so it adds nothing. + return; + } + + // A lambda that does not capture `this` cannot contain one that does, but its capture + // initializers run in the constructor initializer and may create one. + for (const LambdaCapture& Capture: Lambda.captures()) { + if (!Capture.capturesVariable()) continue; + if (const auto* Var = dyn_cast(Capture.getCapturedVar()); + Var != nullptr && Var->isInitCapture()) { + checkExpr(Var->getInit(), InitializedName); + } + } +} + +} // namespace workerd::clang_tidy diff --git a/tools/clang-tidy/ctor-initializer-this-capture.h b/tools/clang-tidy/ctor-initializer-this-capture.h new file mode 100644 index 00000000000..04da84a1533 --- /dev/null +++ b/tools/clang-tidy/ctor-initializer-this-capture.h @@ -0,0 +1,51 @@ +// Copyright (c) 2017-2026 Cloudflare, Inc. +// Licensed under the Apache 2.0 license found in the LICENSE file or at: +// https://opensource.org/licenses/Apache-2.0 + +#pragma once + +#include "clang-tidy/ClangTidyCheck.h" + +namespace workerd::clang_tidy { + +// Flags lambdas that capture `this` inside a constructor's mem-initializer list: +// +// Foo::Foo(Timer& timer) +// : task(timer.afterDelay(1 * kj::SECONDS).then([this]() { onTimeout(); })) {} +// +// While the initializer list runs, the object is only partially constructed: members declared +// after the one being initialized do not exist yet, the constructor body has not run, and a derived +// class's members are not constructed either. A lambda that holds onto `this` lets that +// partially constructed object escape, and nothing stops the lambda from running before +// construction finishes, or from running against an object whose constructor later throws. +// Creating the lambda in the constructor body instead keeps the capture after every member has +// been initialized. +// +// The check reports: +// - explicit `this` and `*this` captures, +// - implicit captures of `this` through a `[&]` or `[=]` capture default, +// - init-captures of the object itself, such as `[self = this]` or `[&self = *this]`. +// +// Base-class, member, and delegating initializers are all checked; default member initializers +// are not, since they are part of the class definition rather than of a constructor. +// +// A lambda that is called immediately, `[&]() { ... }()`, is not reported: the capture does not +// outlive the initializer expression, so it is no more dangerous than naming the members +// directly. Lambdas nested inside such a call are still checked. +class CtorInitializerThisCaptureCheck: public clang::tidy::ClangTidyCheck { + public: + CtorInitializerThisCaptureCheck(clang::StringRef Name, clang::tidy::ClangTidyContext* Context) + : ClangTidyCheck(Name, Context) {} + + bool isLanguageVersionSupported(const clang::LangOptions& LangOpts) const override { + return LangOpts.CPlusPlus11; + } + void registerMatchers(clang::ast_matchers::MatchFinder* Finder) override; + void check(const clang::ast_matchers::MatchFinder::MatchResult& Result) override; + + private: + void checkExpr(const clang::Stmt* S, llvm::StringRef InitializedName); + void checkLambda(const clang::LambdaExpr& Lambda, llvm::StringRef InitializedName); +}; + +} // namespace workerd::clang_tidy diff --git a/tools/clang-tidy/workerd-lint.c++ b/tools/clang-tidy/workerd-lint.c++ index 79a9c48cb6d..1c18350b280 100644 --- a/tools/clang-tidy/workerd-lint.c++ +++ b/tools/clang-tidy/workerd-lint.c++ @@ -10,6 +10,7 @@ // - angled-includes.c++: workerd-angled-includes check // - consume.c++: workerd-consume check // - coroutine-hostile-raii.c++: workerd-coroutine-hostile-raii check +// - ctor-initializer-this-capture.c++: workerd-ctor-initializer-this-capture check // - legacy-stream-alloc.c++: workerd-legacy-stream-alloc check // - promise-ignore-result.c++: workerd-promise-ignore-result check // - visit-for-gc.c++: jsg-visit-for-gc check @@ -20,6 +21,7 @@ #include "clang-tidy/ClangTidyModule.h" #include "consume.h" #include "coroutine-hostile-raii.h" +#include "ctor-initializer-this-capture.h" #include "legacy-stream-alloc.h" #include "promise-ignore-result.h" #include "unsafe-continuation-capture.h" @@ -36,6 +38,8 @@ class WorkerdLintModule: public clang::tidy::ClangTidyModule { CheckFactories.registerCheck("jsg-visit-for-gc"); CheckFactories.registerCheck("workerd-consume"); CheckFactories.registerCheck("workerd-coroutine-hostile-raii"); + CheckFactories.registerCheck( + "workerd-ctor-initializer-this-capture"); CheckFactories.registerCheck("workerd-legacy-stream-alloc"); CheckFactories.registerCheck("workerd-promise-ignore-result"); CheckFactories.registerCheck(