[clang-tools-extra] b06b248 - [clang-tidy] Implement CppCoreGuideline CP.53
Carlos Galvez via cfe-commits
cfe-commits at lists.llvm.org
Thu Jan 5 05:57:33 PST 2023
Author: Chris Cotter
Date: 2023-01-05T13:57:22Z
New Revision: b06b248ad9dc1b50dcf474616c3d586d47270a01
URL: https://github.com/llvm/llvm-project/commit/b06b248ad9dc1b50dcf474616c3d586d47270a01
DIFF: https://github.com/llvm/llvm-project/commit/b06b248ad9dc1b50dcf474616c3d586d47270a01.diff
LOG: [clang-tidy] Implement CppCoreGuideline CP.53
Implement CppCoreGuideline CP.53 to warn when a coroutine accepts
references parameters. Although the guideline mentions that it is safe
to access a reference parameter before suspension points, the guideline
recommends flagging all coroutine parameter references.
Reviewed By: carlosgalvezp
Differential Revision: https://reviews.llvm.org/D140793
Added:
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidReferenceCoroutineParametersCheck.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidReferenceCoroutineParametersCheck.h
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-reference-coroutine-parameters.rst
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-reference-coroutine-parameters.cpp
Modified:
clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidReferenceCoroutineParametersCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidReferenceCoroutineParametersCheck.cpp
new file mode 100644
index 0000000000000..018aaad9a8d7a
--- /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<ParmVarDecl>("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/AvoidReferenceCoroutineParametersCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidReferenceCoroutineParametersCheck.h
new file mode 100644
index 0000000000000..502b5717893b3
--- /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/CMakeLists.txt b/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
index 6fb11858e44a1..a8f657a45f250 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
@@ -8,6 +8,7 @@ add_clang_library(clangTidyCppCoreGuidelinesModule
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
index bd34ac442b0dd..13a0cc73c4143 100644
--- 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 @@ class CppCoreGuidelinesModule : public ClangTidyModule {
"cppcoreguidelines-avoid-magic-numbers");
CheckFactories.registerCheck<AvoidNonConstGlobalVariablesCheck>(
"cppcoreguidelines-avoid-non-const-global-variables");
+ CheckFactories.registerCheck<AvoidReferenceCoroutineParametersCheck>(
+ "cppcoreguidelines-avoid-reference-coroutine-parameters");
CheckFactories.registerCheck<modernize::UseOverrideCheck>(
"cppcoreguidelines-explicit-virtual-functions");
CheckFactories.registerCheck<InitVariablesCheck>(
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 580776b76fd14..989f4b52d4ac8 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -118,6 +118,11 @@ New checks
Warns when using ``do-while`` loops.
+- New :doc:`cppcoreguidelines-avoid-reference-coroutine-parameters
+ <clang-tidy/checks/cppcoreguidelines/avoid-reference-coroutine-parameters>` check.
+
+ Warns on coroutines that accept reference parameters.
+
- New :doc:`misc-use-anonymous-namespace
<clang-tidy/checks/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
index 0000000000000..d6186fdc301c7
--- /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<int> someCoroutine(int& val) {
+ co_await ...;
+ // When the coroutine is resumed, 'val' might no longer be valid.
+ if (val) ...
+ }
+
+This check implements
+`CppCoreGuideline CP.53 <https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rcoro-reference-parameters>`_.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index b57ebe0397d57..ec59eb87ad135 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -182,6 +182,7 @@ Clang-Tidy Checks
`cppcoreguidelines-avoid-do-while <cppcoreguidelines/avoid-do-while.html>`_,
`cppcoreguidelines-avoid-goto <cppcoreguidelines/avoid-goto.html>`_,
`cppcoreguidelines-avoid-non-const-global-variables <cppcoreguidelines/avoid-non-const-global-variables.html>`_,
+ `cppcoreguidelines-avoid-reference-coroutine-parameters <cppcoreguidelines/avoid-reference-coroutine-parameters.html>`_,
`cppcoreguidelines-init-variables <cppcoreguidelines/init-variables.html>`_, "Yes"
`cppcoreguidelines-interfaces-global-init <cppcoreguidelines/interfaces-global-init.html>`_,
`cppcoreguidelines-macro-usage <cppcoreguidelines/macro-usage.html>`_,
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
index 0000000000000..abeb7fc770155
--- /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 <typename T, typename... Args>
+ struct coroutine_traits {
+ using promise_type = typename T::promise_type;
+ };
+ template <typename T = void>
+ struct coroutine_handle;
+ template <>
+ struct coroutine_handle<void> {
+ coroutine_handle() noexcept;
+ coroutine_handle(decltype(nullptr)) noexcept;
+ static constexpr coroutine_handle from_address(void*);
+ };
+ template <typename T>
+ 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]
+}
More information about the cfe-commits
mailing list