Izaron updated this revision to Diff 467590.
Izaron added a comment.

Change `IgnoreVoidReturnType` to `RewriteVoidReturn` type.
Check the cheaper condition first.

A big thank to @bernhardmgruber for nice comments!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135822/new/

https://reviews.llvm.org/D135822

Files:
  clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/modernize/use-trailing-return-type.rst
  
clang-tools-extra/test/clang-tidy/checkers/modernize/use-trailing-return-type-cxx20.cpp
  
clang-tools-extra/test/clang-tidy/checkers/modernize/use-trailing-return-type-void.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize/use-trailing-return-type-void.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/use-trailing-return-type-void.cpp
@@ -0,0 +1,25 @@
+// RUN: %check_clang_tidy %s modernize-use-trailing-return-type %t -- \
+// RUN:   -config="{CheckOptions: [{key: modernize-use-trailing-return-type.RewriteVoidReturnType, value: true}]}"
+
+void c();
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}auto c() -> void;{{$}}
+void c(int arg);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}auto c(int arg) -> void;{{$}}
+void c(int arg) { return; }
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}auto c(int arg) -> void { return; }{{$}}
+
+//
+// Samples which do not trigger the check
+//
+
+auto f() -> void;
+auto f(int arg) -> void;
+auto f(int arg) -> void { return; }
+
+class C {
+    C() {}
+    ~C() {}
+};
Index: clang-tools-extra/test/clang-tidy/checkers/modernize/use-trailing-return-type-cxx20.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/modernize/use-trailing-return-type-cxx20.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/use-trailing-return-type-cxx20.cpp
@@ -67,6 +67,7 @@
   static strong_ordering const greater;
 
   constexpr strong_ordering(value_type v) : val(v) {}
+  constexpr ~strong_ordering() = default;
   template <typename T>
   requires(T{0}) friend constexpr auto
   operator==(strong_ordering v, T u) noexcept -> bool {
Index: clang-tools-extra/docs/clang-tidy/checks/modernize/use-trailing-return-type.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/modernize/use-trailing-return-type.rst
+++ clang-tools-extra/docs/clang-tidy/checks/modernize/use-trailing-return-type.rst
@@ -66,3 +66,11 @@
 This code fails to compile because the S in the context of f refers to the equally named function parameter.
 Similarly, the S in the context of m refers to the equally named class member.
 The check can currently only detect and avoid a clash with a function parameter name.
+
+Options
+-------
+
+.. option:: RewriteVoidReturnType
+
+   When `true`, the check will rewrite function signature for functions
+   with void return type. Default is `false`.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -175,6 +175,12 @@
   The check now skips concept definitions since redundant expressions still make sense
   inside them.
 
+- Improved :doc:`modernize-use-trailing-return-type <clang-tidy/checks/modernize/use-trailing-return-type>`
+  check.
+
+  The check now has a new option `RewriteVoidReturnType` which indicates if the
+  check should rewrite function signature for functions with void return type.
+
 Removed checks
 ^^^^^^^^^^^^^^
 
Index: clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.h
+++ clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.h
@@ -28,11 +28,11 @@
 /// http://clang.llvm.org/extra/clang-tidy/checks/modernize/use-trailing-type-return.html
 class UseTrailingReturnTypeCheck : public ClangTidyCheck {
 public:
-  UseTrailingReturnTypeCheck(StringRef Name, ClangTidyContext *Context)
-      : ClangTidyCheck(Name, Context) {}
+  UseTrailingReturnTypeCheck(StringRef Name, ClangTidyContext *Context);
   bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
     return LangOpts.CPlusPlus11;
   }
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
                            Preprocessor *ModuleExpanderPP) override;
@@ -40,6 +40,7 @@
 
 private:
   Preprocessor *PP = nullptr;
+  const bool RewriteVoidReturnType;
 
   SourceLocation findTrailingReturnTypeSourceLocation(
       const FunctionDecl &F, const FunctionTypeLoc &FTL, const ASTContext &Ctx,
Index: clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp
@@ -125,6 +125,17 @@
   return Loc;
 }
 
+UseTrailingReturnTypeCheck::UseTrailingReturnTypeCheck(
+    StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      RewriteVoidReturnType(
+          Options.getLocalOrGlobal("RewriteVoidReturnType", false)) {}
+
+void UseTrailingReturnTypeCheck::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "RewriteVoidReturnType", RewriteVoidReturnType);
+}
+
 SourceLocation UseTrailingReturnTypeCheck::findTrailingReturnTypeSourceLocation(
     const FunctionDecl &F, const FunctionTypeLoc &FTL, const ASTContext &Ctx,
     const SourceManager &SM, const LangOptions &LangOpts) {
@@ -384,9 +395,9 @@
 }
 
 void UseTrailingReturnTypeCheck::registerMatchers(MatchFinder *Finder) {
-  auto F = functionDecl(
-               unless(anyOf(hasTrailingReturn(), returns(voidType()),
-                            cxxConversionDecl(), cxxMethodDecl(isImplicit()))))
+  auto F = functionDecl(unless(anyOf(hasTrailingReturn(), cxxConstructorDecl(),
+                                     cxxDestructorDecl(), cxxConversionDecl(),
+                                     cxxMethodDecl(isImplicit()))))
                .bind("Func");
 
   Finder->addMatcher(F, this);
@@ -406,6 +417,9 @@
   const auto *Fr = Result.Nodes.getNodeAs<FriendDecl>("Friend");
   assert(F && "Matcher is expected to find only FunctionDecls");
 
+  if (!RewriteVoidReturnType && F->getReturnType()->isVoidType())
+    return;
+
   // Three-way comparison operator<=> is syntactic sugar and generates implicit
   // nodes for all other operators.
   if (F->getLocation().isInvalid() || F->isImplicit())
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D135822: [... Evgeny Shulgin via Phabricator via cfe-commits
    • [PATCH] D1358... Evgeny Shulgin via Phabricator via cfe-commits
    • [PATCH] D1358... Bernhard Manfred Gruber via Phabricator via cfe-commits
    • [PATCH] D1358... Evgeny Shulgin via Phabricator via cfe-commits

Reply via email to