Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang-tidy] Remove cert-dcl21-cpp check #80181

Merged
merged 1 commit into from
Jan 31, 2024

Conversation

carlosgalvezp
Copy link
Contributor

Deprecated since clang-tidy 17. The rule DCL21-CPP has been removed from the CERT guidelines, so it does not make sense to keep the check.

Fixes #42788

Deprecated since clang-tidy 17. The rule DCL21-CPP has been removed
from the CERT guidelines, so it does not make sense to keep the check.

Fixes llvm#42788
@llvmbot llvmbot added clang Clang issues not falling into any other category clang-tools-extra clangd clang-tidy labels Jan 31, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 31, 2024

@llvm/pr-subscribers-clang-tools-extra
@llvm/pr-subscribers-clangd
@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-tidy

Author: Carlos Galvez (carlosgalvezp)

Changes

Deprecated since clang-tidy 17. The rule DCL21-CPP has been removed from the CERT guidelines, so it does not make sense to keep the check.

Fixes #42788


Full diff: https://github.com/llvm/llvm-project/pull/80181.diff

11 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp (-3)
  • (modified) clang-tools-extra/clang-tidy/cert/CMakeLists.txt (-1)
  • (removed) clang-tools-extra/clang-tidy/cert/PostfixOperatorCheck.cpp (-80)
  • (removed) clang-tools-extra/clang-tidy/cert/PostfixOperatorCheck.h (-34)
  • (modified) clang-tools-extra/clangd/TidyFastChecks.inc (-1)
  • (modified) clang-tools-extra/docs/ReleaseNotes.rst (+3)
  • (removed) clang-tools-extra/docs/clang-tidy/checks/cert/dcl21-cpp.rst (-28)
  • (modified) clang-tools-extra/docs/clang-tidy/checks/list.rst (-1)
  • (removed) clang-tools-extra/test/clang-tidy/checkers/cert/dcl21-cpp.cpp (-134)
  • (modified) clang/docs/tools/clang-formatted-files.txt (-2)
  • (modified) llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/cert/BUILD.gn (-1)
diff --git a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
index d334ab8c437d3..b06a903f92b3e 100644
--- a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
@@ -33,7 +33,6 @@
 #include "LimitedRandomnessCheck.h"
 #include "MutatingCopyCheck.h"
 #include "NonTrivialTypesLibcMemoryCallsCheck.h"
-#include "PostfixOperatorCheck.h"
 #include "ProperlySeededRandomGeneratorCheck.h"
 #include "SetLongJmpCheck.h"
 #include "StaticObjectExceptionCheck.h"
@@ -239,8 +238,6 @@ class CERTModule : public ClangTidyModule {
     CheckFactories.registerCheck<bugprone::SpuriouslyWakeUpFunctionsCheck>(
         "cert-con54-cpp");
     // DCL
-    CheckFactories.registerCheck<PostfixOperatorCheck>(
-        "cert-dcl21-cpp");
     CheckFactories.registerCheck<VariadicFunctionDefCheck>("cert-dcl50-cpp");
     CheckFactories.registerCheck<bugprone::ReservedIdentifierCheck>(
         "cert-dcl51-cpp");
diff --git a/clang-tools-extra/clang-tidy/cert/CMakeLists.txt b/clang-tools-extra/clang-tidy/cert/CMakeLists.txt
index 889180f62fde9..882735c9d1e0d 100644
--- a/clang-tools-extra/clang-tidy/cert/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/cert/CMakeLists.txt
@@ -12,7 +12,6 @@ add_clang_library(clangTidyCERTModule
   LimitedRandomnessCheck.cpp
   MutatingCopyCheck.cpp
   NonTrivialTypesLibcMemoryCallsCheck.cpp
-  PostfixOperatorCheck.cpp
   ProperlySeededRandomGeneratorCheck.cpp
   SetLongJmpCheck.cpp
   StaticObjectExceptionCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/cert/PostfixOperatorCheck.cpp b/clang-tools-extra/clang-tidy/cert/PostfixOperatorCheck.cpp
deleted file mode 100644
index 8ff63ade7315d..0000000000000
--- a/clang-tools-extra/clang-tidy/cert/PostfixOperatorCheck.cpp
+++ /dev/null
@@ -1,80 +0,0 @@
-//===--- PostfixOperatorCheck.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 "PostfixOperatorCheck.h"
-#include "clang/AST/ASTContext.h"
-#include "clang/ASTMatchers/ASTMatchFinder.h"
-#include "clang/Lex/Lexer.h"
-
-using namespace clang::ast_matchers;
-
-namespace clang::tidy::cert {
-
-void PostfixOperatorCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(functionDecl(hasAnyOverloadedOperatorName("++", "--"),
-                                  unless(isInstantiated()))
-                         .bind("decl"),
-                     this);
-}
-
-void PostfixOperatorCheck::check(const MatchFinder::MatchResult &Result) {
-  const auto *FuncDecl = Result.Nodes.getNodeAs<FunctionDecl>("decl");
-
-  bool HasThis = false;
-  if (const auto *MethodDecl = dyn_cast<CXXMethodDecl>(FuncDecl))
-    HasThis = MethodDecl->isInstance();
-
-  // Check if the operator is a postfix one.
-  if (FuncDecl->getNumParams() != (HasThis ? 1 : 2))
-    return;
-
-  SourceRange ReturnRange = FuncDecl->getReturnTypeSourceRange();
-  SourceLocation Location = ReturnRange.getBegin();
-  if (!Location.isValid())
-    return;
-
-  QualType ReturnType = FuncDecl->getReturnType();
-
-  // Warn when the operators return a reference.
-  if (const auto *RefType = ReturnType->getAs<ReferenceType>()) {
-    auto Diag = diag(Location, "overloaded %0 returns a reference instead of a "
-                               "constant object type")
-                << FuncDecl;
-
-    if (Location.isMacroID() || ReturnType->getAs<TypedefType>() ||
-        RefType->getPointeeTypeAsWritten()->getAs<TypedefType>())
-      return;
-
-    QualType ReplaceType =
-        ReturnType.getNonReferenceType().getLocalUnqualifiedType();
-    // The getReturnTypeSourceRange omits the qualifiers. We do not want to
-    // duplicate the const.
-    if (!ReturnType->getPointeeType().isConstQualified())
-      ReplaceType.addConst();
-
-    Diag << FixItHint::CreateReplacement(
-        ReturnRange,
-        ReplaceType.getAsString(Result.Context->getPrintingPolicy()) + " ");
-
-    return;
-  }
-
-  if (ReturnType.isConstQualified() || ReturnType->isBuiltinType() ||
-      ReturnType->isPointerType())
-    return;
-
-  auto Diag =
-      diag(Location, "overloaded %0 returns a non-constant object instead of a "
-                     "constant object type")
-      << FuncDecl;
-
-  if (!Location.isMacroID() && !ReturnType->getAs<TypedefType>())
-    Diag << FixItHint::CreateInsertion(Location, "const ");
-}
-
-} // namespace clang::tidy::cert
diff --git a/clang-tools-extra/clang-tidy/cert/PostfixOperatorCheck.h b/clang-tools-extra/clang-tidy/cert/PostfixOperatorCheck.h
deleted file mode 100644
index 782b9a99ba413..0000000000000
--- a/clang-tools-extra/clang-tidy/cert/PostfixOperatorCheck.h
+++ /dev/null
@@ -1,34 +0,0 @@
-//===--- PostfixOperatorCheck.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_CERT_POSTFIX_OPERATOR_H
-#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_POSTFIX_OPERATOR_H
-
-#include "../ClangTidyCheck.h"
-
-namespace clang::tidy::cert {
-
-/// Checks if the overloaded postfix ++ and -- operator return a constant
-/// object.
-///
-/// For the user-facing documentation see:
-/// https://clang.llvm.org/extra/clang-tidy/checks/cert/dcl21-cpp.html
-class PostfixOperatorCheck : public ClangTidyCheck {
-public:
-  PostfixOperatorCheck(StringRef Name, ClangTidyContext *Context)
-      : ClangTidyCheck(Name, Context) {}
-  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
-    return LangOpts.CPlusPlus;
-  }
-  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
-  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
-};
-
-} // namespace clang::tidy::cert
-
-#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_POSTFIX_OPERATOR_H
diff --git a/clang-tools-extra/clangd/TidyFastChecks.inc b/clang-tools-extra/clangd/TidyFastChecks.inc
index 9596ba4d725b6..9050ce16127ff 100644
--- a/clang-tools-extra/clangd/TidyFastChecks.inc
+++ b/clang-tools-extra/clangd/TidyFastChecks.inc
@@ -116,7 +116,6 @@ FAST(cert-con36-c, 2.0)
 FAST(cert-con54-cpp, 3.0)
 FAST(cert-dcl03-c, 2.0)
 FAST(cert-dcl16-c, -1.0)
-FAST(cert-dcl21-cpp, 0.0)
 FAST(cert-dcl37-c, 3.0)
 FAST(cert-dcl50-cpp, -1.0)
 FAST(cert-dcl51-cpp, 1.0)
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index fd2cba4e4f463..ad99fe9d08f9c 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -109,6 +109,9 @@ Changes in existing checks
 Removed checks
 ^^^^^^^^^^^^^^
 
+- Removed `cert-dcl21-cpp`, which was deprecated since :program:`clang-tidy` 17,
+  since the rule DCL21-CPP has been removed from the CERT guidelines.
+
 Improvements to include-fixer
 -----------------------------
 
diff --git a/clang-tools-extra/docs/clang-tidy/checks/cert/dcl21-cpp.rst b/clang-tools-extra/docs/clang-tidy/checks/cert/dcl21-cpp.rst
deleted file mode 100644
index 9845c2776e11d..0000000000000
--- a/clang-tools-extra/docs/clang-tidy/checks/cert/dcl21-cpp.rst
+++ /dev/null
@@ -1,28 +0,0 @@
-.. title:: clang-tidy - cert-dcl21-cpp
-
-cert-dcl21-cpp
-==============
-
-.. note::
-  This check is deprecated since it's no longer part of the CERT standard.
-  It will be removed in :program:`clang-tidy` version 19.
-
-This check flags postfix ``operator++`` and ``operator--`` declarations
-if the return type is not a const object. This also warns if the return type
-is a reference type.
-
-The object returned by a postfix increment or decrement operator is supposed
-to be a snapshot of the object's value prior to modification. With such an
-implementation, any modifications made to the resulting object from calling
-operator++(int) would be modifying a temporary object. Thus, such an
-implementation of a postfix increment or decrement operator should instead
-return a const object, prohibiting accidental mutation of a temporary object.
-Similarly, it is unexpected for the postfix operator to return a reference to
-its previous state, and any subsequent modifications would be operating on a
-stale object.
-
-This check corresponds to the CERT C++ Coding Standard recommendation
-DCL21-CPP. Overloaded postfix increment and decrement operators should return a
-const object. However, all of the CERT recommendations have been removed from
-public view, and so their justification for the behavior of this check requires
-an account on their wiki to view.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index f773e80b562e4..f40192ed9dea2 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -155,7 +155,6 @@ Clang-Tidy Checks
    :doc:`bugprone-unused-return-value <bugprone/unused-return-value>`,
    :doc:`bugprone-use-after-move <bugprone/use-after-move>`,
    :doc:`bugprone-virtual-near-miss <bugprone/virtual-near-miss>`, "Yes"
-   :doc:`cert-dcl21-cpp <cert/dcl21-cpp>`, "Yes"
    :doc:`cert-dcl50-cpp <cert/dcl50-cpp>`,
    :doc:`cert-dcl58-cpp <cert/dcl58-cpp>`,
    :doc:`cert-env33-c <cert/env33-c>`,
diff --git a/clang-tools-extra/test/clang-tidy/checkers/cert/dcl21-cpp.cpp b/clang-tools-extra/test/clang-tidy/checkers/cert/dcl21-cpp.cpp
deleted file mode 100644
index c975f70fbb02c..0000000000000
--- a/clang-tools-extra/test/clang-tidy/checkers/cert/dcl21-cpp.cpp
+++ /dev/null
@@ -1,134 +0,0 @@
-// RUN: %check_clang_tidy %s cert-dcl21-cpp %t
-
-class A {};
-
-A operator++(A &, int);
-// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: overloaded 'operator++' returns a non-constant object instead of a constant object type [cert-dcl21-cpp]
-// CHECK-FIXES: {{^}}const A operator++(A &, int);
-
-A operator--(A &, int);
-// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: overloaded 'operator--' returns a no
-// CHECK-FIXES: {{^}}const A operator--(A &, int);
-
-class B {};
-
-B &operator++(B &);
-const B operator++(B &, int);
-
-B &operator--(B &);
-const B operator--(B &, int);
-
-
-class D {
-D &operator++();
-const D operator++(int);
-
-D &operator--();
-const D operator--(int);
-};
-
-class C {
-C operator++(int);
-// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: overloaded 'operator++' returns a no
-// CHECK-FIXES: {{^}}const C operator++(int);
-
-C operator--(int);
-// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: overloaded 'operator--' returns a no
-// CHECK-FIXES: {{^}}const C operator--(int);
-};
-
-class E {};
-
-E &operator++(E &, int);
-// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: overloaded 'operator++' returns a reference instead of a constant object type [cert-dcl21-cpp]
-// CHECK-FIXES: {{^}}const E operator++(E &, int);
-
-E &operator--(E &, int);
-// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: overloaded 'operator--' returns a re
-// CHECK-FIXES: {{^}}const E operator--(E &, int);
-
-class G {
-G &operator++(int);
-// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: overloaded 'operator++' returns a re
-// CHECK-FIXES: {{^}}const G operator++(int);
-
-G &operator--(int);
-// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: overloaded 'operator--' returns a re
-// CHECK-FIXES: {{^}}const G operator--(int);
-};
-
-class F {};
-
-const F &operator++(F &, int);
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: overloaded 'operator++' returns a re
-// CHECK-FIXES: {{^}}const F operator++(F &, int);
-
-const F &operator--(F &, int);
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: overloaded 'operator--' returns a re
-// CHECK-FIXES: {{^}}const F operator--(F &, int);
-
-class H {
-const H &operator++(int);
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: overloaded 'operator++' returns a re
-// CHECK-FIXES: {{^}}const H operator++(int);
-
-const H &operator--(int);
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: overloaded 'operator--' returns a re
-// CHECK-FIXES: {{^}}const H operator--(int);
-};
-
-
-#define FROM_MACRO P&
-class P {
-const FROM_MACRO operator++(int);
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: overloaded 'operator++' returns a re
-// CHECK-FIXES: {{^}}const FROM_MACRO operator++(int);
-};
-
-
-template<typename T>
-class Q {
-const Q &operator++(int);
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: overloaded 'operator++' returns a re
-// CHECK-FIXES: {{^}}const Q<T> operator++(int);
-
-const Q &operator--(int);
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: overloaded 'operator--' returns a re
-// CHECK-FIXES: {{^}}const Q<T> operator--(int);
-};
-
-void foobar() {
-  Q<int> a;
-  Q<float> b;
-  (void)a;
-  (void)b;
-}
-
-struct S {};
-typedef S& SRef;
-
-SRef operator++(SRef, int);
-// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: overloaded 'operator++' returns a re
-// CHECK-FIXES: {{^}}SRef operator++(SRef, int);
-
-struct T {
-  typedef T& TRef;
-  
-  TRef operator++(int);
-// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: overloaded 'operator++' returns a re
-// CHECK-FIXES: {{^}}  TRef operator++(int);
-};
-
-struct U {
-  typedef const U& ConstURef;
-  
-  ConstURef& operator++(int);
-// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: overloaded 'operator++' returns a re
-// CHECK-FIXES: {{^}}  ConstURef& operator++(int);
-};
-
-struct V {
-  V *operator++(int);
-  V *const operator--(int);
-};
-
diff --git a/clang/docs/tools/clang-formatted-files.txt b/clang/docs/tools/clang-formatted-files.txt
index 18512b1a7bf6b..40ab76fa26a9e 100644
--- a/clang/docs/tools/clang-formatted-files.txt
+++ b/clang/docs/tools/clang-formatted-files.txt
@@ -985,8 +985,6 @@ clang-tools-extra/clang-tidy/cert/MutatingCopyCheck.cpp
 clang-tools-extra/clang-tidy/cert/MutatingCopyCheck.h
 clang-tools-extra/clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.cpp
 clang-tools-extra/clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.h
-clang-tools-extra/clang-tidy/cert/PostfixOperatorCheck.cpp
-clang-tools-extra/clang-tidy/cert/PostfixOperatorCheck.h
 clang-tools-extra/clang-tidy/cert/ProperlySeededRandomGeneratorCheck.cpp
 clang-tools-extra/clang-tidy/cert/ProperlySeededRandomGeneratorCheck.h
 clang-tools-extra/clang-tidy/cert/SetLongJmpCheck.cpp
diff --git a/llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/cert/BUILD.gn b/llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/cert/BUILD.gn
index 86190d4262b1c..788f8499fe072 100644
--- a/llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/cert/BUILD.gn
+++ b/llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/cert/BUILD.gn
@@ -23,7 +23,6 @@ static_library("cert") {
     "LimitedRandomnessCheck.cpp",
     "MutatingCopyCheck.cpp",
     "NonTrivialTypesLibcMemoryCallsCheck.cpp",
-    "PostfixOperatorCheck.cpp",
     "ProperlySeededRandomGeneratorCheck.cpp",
     "SetLongJmpCheck.cpp",
     "StaticObjectExceptionCheck.cpp",

Copy link

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff cf2533e75ec4360da460bb577e0a4e64f2d8997f fdb65ef4805152ad143a120e343d696f8b08d3db -- clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp clang-tools-extra/clangd/TidyFastChecks.inc
View the diff from clang-format here.
diff --git a/clang-tools-extra/clangd/TidyFastChecks.inc b/clang-tools-extra/clangd/TidyFastChecks.inc
index 9050ce1612..b1c82a2e29 100644
--- a/clang-tools-extra/clangd/TidyFastChecks.inc
+++ b/clang-tools-extra/clangd/TidyFastChecks.inc
@@ -115,7 +115,7 @@ FAST(bugprone-virtual-near-miss, 0.0)
 FAST(cert-con36-c, 2.0)
 FAST(cert-con54-cpp, 3.0)
 FAST(cert-dcl03-c, 2.0)
-FAST(cert-dcl16-c, -1.0)
+FAST(cert - dcl16 - c, -1.0)
 FAST(cert-dcl37-c, 3.0)
 FAST(cert-dcl50-cpp, -1.0)
 FAST(cert-dcl51-cpp, 1.0)

@carlosgalvezp
Copy link
Contributor Author

The code formatting job seems to produce incorrect changes.

Copy link
Contributor

@EugeneZelenko EugeneZelenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks OK for me, but will be good idea if @PiotrZSL will also take a look.

Copy link
Member

@PiotrZSL PiotrZSL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@carlosgalvezp carlosgalvezp merged commit 4cb13f2 into llvm:main Jan 31, 2024
7 of 9 checks passed
@carlosgalvezp carlosgalvezp deleted the cert_dcl21_remove branch January 31, 2024 19:35
@carlosgalvezp carlosgalvezp restored the cert_dcl21_remove branch February 1, 2024 19:08
@carlosgalvezp carlosgalvezp deleted the cert_dcl21_remove branch February 1, 2024 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang Clang issues not falling into any other category clang-tidy clang-tools-extra clangd
Projects
None yet
Development

Successfully merging this pull request may close these issues.

clang-tidy cert-dcl21-cpp is confusing (wrong?)
4 participants