diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidReferenceCoroutineParametersCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidReferenceCoroutineParametersCheck.h new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidReferenceCoroutineParametersCheck.h @@ -0,0 +1,40 @@ +//===--- AvoidReferenceCoroutineParametersCheck.h - clang-tidy --*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_AVOIDREFERENCECOROUTINEPARAMETERSCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_AVOIDREFERENCECOROUTINEPARAMETERSCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang { +namespace tidy { +namespace cppcoreguidelines { + +/// Warns on coroutines that accept reference parameters. Accessing a reference +/// after a coroutine suspension point is not safe since the reference may no +/// longer be valid. This implements CppCoreGuideline CP.53. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/avoid-reference-coroutine-parameters.html +class AvoidReferenceCoroutineParametersCheck : public ClangTidyCheck { +public: + AvoidReferenceCoroutineParametersCheck(StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus20; + } +}; + +} // namespace cppcoreguidelines +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_AVOIDREFERENCECOROUTINEPARAMETERSCHECK_H diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidReferenceCoroutineParametersCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidReferenceCoroutineParametersCheck.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidReferenceCoroutineParametersCheck.cpp @@ -0,0 +1,38 @@ +//===--- AvoidReferenceCoroutineParametersCheck.cpp - clang-tidy ----------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "AvoidReferenceCoroutineParametersCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace cppcoreguidelines { + +void AvoidReferenceCoroutineParametersCheck::registerMatchers( + MatchFinder *Finder) { + auto IsCoroMatcher = + hasDescendant(expr(anyOf(coyieldExpr(), coreturnStmt(), coawaitExpr()))); + Finder->addMatcher(parmVarDecl(hasType(type(referenceType())), + hasAncestor(functionDecl(IsCoroMatcher))) + .bind("param"), + this); +} + +void AvoidReferenceCoroutineParametersCheck::check( + const MatchFinder::MatchResult &Result) { + if (const auto *Param = Result.Nodes.getNodeAs("param")) { + diag(Param->getBeginLoc(), "coroutine parameters should not be references"); + } +} + +} // namespace cppcoreguidelines +} // namespace tidy +} // namespace clang diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt b/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt @@ -8,6 +8,7 @@ AvoidDoWhileCheck.cpp AvoidGotoCheck.cpp AvoidNonConstGlobalVariablesCheck.cpp + AvoidReferenceCoroutineParametersCheck.cpp CppCoreGuidelinesTidyModule.cpp InitVariablesCheck.cpp InterfacesGlobalInitCheck.cpp diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp @@ -18,6 +18,7 @@ #include "AvoidDoWhileCheck.h" #include "AvoidGotoCheck.h" #include "AvoidNonConstGlobalVariablesCheck.h" +#include "AvoidReferenceCoroutineParametersCheck.h" #include "InitVariablesCheck.h" #include "InterfacesGlobalInitCheck.h" #include "MacroUsageCheck.h" @@ -59,6 +60,8 @@ "cppcoreguidelines-avoid-magic-numbers"); CheckFactories.registerCheck( "cppcoreguidelines-avoid-non-const-global-variables"); + CheckFactories.registerCheck( + "cppcoreguidelines-avoid-reference-coroutine-parameters"); CheckFactories.registerCheck( "cppcoreguidelines-explicit-virtual-functions"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -118,6 +118,11 @@ Warns when using ``do-while`` loops. +- New :doc:`cppcoreguidelines-avoid-reference-coroutine-parameters + ` check. + + Warns on coroutines that accept reference parameters. + - New :doc:`misc-use-anonymous-namespace ` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-reference-coroutine-parameters.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-reference-coroutine-parameters.rst new file mode 100644 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-reference-coroutine-parameters.rst @@ -0,0 +1,20 @@ +.. title:: clang-tidy - cppcoreguidelines-avoid-reference-coroutine-parameters + +cppcoreguidelines-avoid-reference-coroutine-parameters +====================================================== + +Warns when a coroutine accepts reference parameters. After a coroutine suspend point, +references could be dangling and no longer valid. Instead, pass parameters as values. + +Examples: + +.. code-block:: c++ + + std::future someCoroutine(int& val) { + co_await ...; + // When the coroutine is resumed, 'val' might no longer be valid. + if (val) ... + } + +This check implements +`CppCoreGuideline CP.53 `_. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -182,6 +182,7 @@ `cppcoreguidelines-avoid-do-while `_, `cppcoreguidelines-avoid-goto `_, `cppcoreguidelines-avoid-non-const-global-variables `_, + `cppcoreguidelines-avoid-reference-coroutine-parameters `_, `cppcoreguidelines-init-variables `_, "Yes" `cppcoreguidelines-interfaces-global-init `_, `cppcoreguidelines-macro-usage `_, diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-reference-coroutine-parameters.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-reference-coroutine-parameters.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-reference-coroutine-parameters.cpp @@ -0,0 +1,84 @@ +// RUN: %check_clang_tidy -std=c++20 %s cppcoreguidelines-avoid-reference-coroutine-parameters %t + +// NOLINTBEGIN +namespace std { + template + struct coroutine_traits { + using promise_type = typename T::promise_type; + }; + template + struct coroutine_handle; + template <> + struct coroutine_handle { + coroutine_handle() noexcept; + coroutine_handle(decltype(nullptr)) noexcept; + static constexpr coroutine_handle from_address(void*); + }; + template + struct coroutine_handle { + coroutine_handle() noexcept; + coroutine_handle(decltype(nullptr)) noexcept; + static constexpr coroutine_handle from_address(void*); + operator coroutine_handle<>() const noexcept; + }; +} // namespace std + +struct Awaiter { + bool await_ready() noexcept; + void await_suspend(std::coroutine_handle<>) noexcept; + void await_resume() noexcept; +}; + +struct Coro { + struct promise_type { + Awaiter initial_suspend(); + Awaiter final_suspend() noexcept; + void return_void(); + Coro get_return_object(); + void unhandled_exception(); + }; +}; +// NOLINTEND + +struct Obj {}; + +Coro no_args() { + co_return; +} + +Coro no_references(int x, int* y, Obj z, const Obj w) { + co_return; +} + +Coro accepts_references(int& x, const int &y) { + // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: coroutine parameters should not be references [cppcoreguidelines-avoid-reference-coroutine-parameters] + // CHECK-MESSAGES: :[[@LINE-2]]:33: warning: coroutine parameters should not be references [cppcoreguidelines-avoid-reference-coroutine-parameters] + co_return; +} + +Coro accepts_references_and_non_references(int& x, int y) { + // CHECK-MESSAGES: :[[@LINE-1]]:44: warning: coroutine parameters should not be references [cppcoreguidelines-avoid-reference-coroutine-parameters] + co_return; +} + +Coro accepts_references_to_objects(Obj& x) { + // CHECK-MESSAGES: :[[@LINE-1]]:36: warning: coroutine parameters should not be references [cppcoreguidelines-avoid-reference-coroutine-parameters] + co_return; +} + +Coro non_coro_accepts_references(int& x) { + if (x); + return Coro{}; +} + +void defines_a_lambda() { + auto NoArgs = [](int x) -> Coro { co_return; }; + + auto NoReferences = [](int x) -> Coro { co_return; }; + + auto WithReferences = [](int& x) -> Coro { co_return; }; + // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: coroutine parameters should not be references [cppcoreguidelines-avoid-reference-coroutine-parameters] + + auto WithReferences2 = [](int&) -> Coro { co_return; }; + // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: coroutine parameters should not be references [cppcoreguidelines-avoid-reference-coroutine-parameters] +}