ymandel created this revision.
ymandel added reviewers: aaron.ballman, JonasToth, ioeric.
Herald added subscribers: cfe-commits, xazax.hun, mgorny.
Adds a new check readability-const-value-return, which checks for functions with
a ``const``-qualified return type and recommends removal of the `const` keyword.
Such use of ``const`` is superfluous, and prevents valuable compiler
optimizations.
Based in part on the (abandoned) review https://reviews.llvm.org/D33531.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53025
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/ConstValueReturnCheck.cpp
clang-tidy/readability/ConstValueReturnCheck.h
clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tidy/utils/LexerUtils.cpp
clang-tidy/utils/LexerUtils.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/readability-const-value-return.rst
test/clang-tidy/Inputs/readability-const-value-return/
test/clang-tidy/Inputs/readability-const-value-return/macro-def.h
test/clang-tidy/readability-const-value-return.cpp
Index: test/clang-tidy/readability-const-value-return.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/readability-const-value-return.cpp
@@ -0,0 +1,139 @@
+// RUN: %check_clang_tidy %s readability-const-value-return %t -- -- \
+// RUN: -I %S/Inputs/readability-const-value-return
+
+#include "macro-def.h"
+
+// p# = positive test
+// n# = negative test
+
+const int p1() {
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: avoid marking return types as ``const`` for values, as it often disables optimizations and causes unnecessary copies
+// CHECK-FIXES: int p1() {
+ return 1;
+}
+
+const int p15();
+// CHECK-FIXES: int p15();
+
+template <typename A> class Klazz {};
+class Clazz {
+ public:
+ Clazz *const p2() {
+ // CHECK-MESSAGES: [[@LINE-1]]:10: warning: avoid marking return types as ``
+ // CHECK-FIXES: Clazz *p2() {
+ return this;
+ }
+
+ Clazz *const p3();
+ // CHECK-FIXES: Clazz *p3();
+
+ const int p4() const {
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: avoid marking return types as ``c
+ // CHECK-FIXES: int p4() const {
+ return 4;
+ }
+
+ const Klazz<const int>* const p5() const;
+ // CHECK-FIXES: const Klazz<const int>* p5() const;
+
+ const Clazz operator++(int x) { // p12
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: avoid marking return types as ``con
+ // CHECK-FIXES: Clazz operator++(int x) {
+ }
+
+ struct Strukt {
+ int i;
+ };
+
+ const Strukt p6() {}
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: avoid marking return types as ``con
+ // CHECK-FIXES: Strukt p6() {}
+
+ const Strukt* const p7();
+ // CHECK-FIXES: const Strukt* p7();
+
+ static const int p8() {}
+ // CHECK-MESSAGES: [[@LINE-1]]:10: warning: avoid marking return types as ``co
+ // CHECK-FIXES: static int p8() {}
+
+ static const Strukt p9() {}
+ // CHECK-MESSAGES: [[@LINE-1]]:10: warning: avoid marking return types as ``co
+ // CHECK-FIXES: static Strukt p9() {}
+
+ int n0() const { return 0; }
+ const Klazz<const int>& n11(const Klazz<const int>) const;
+};
+
+Clazz *const Clazz::p3() {
+ // CHECK-MESSAGES: [[@LINE-1]]:8: warning: avoid marking return types as ``con
+ // CHECK-FIXES: Clazz *Clazz::p3() {
+ return this;
+}
+
+const Klazz<const int>* const Clazz::p5() const {}
+// CHECK-MESSAGES: [[@LINE-1]]:25: warning: avoid marking return types as ``cons
+// CHECK-FIXES: const Klazz<const int>* Clazz::p5() const {}
+
+const Clazz::Strukt* const Clazz::p7() {}
+// CHECK-MESSAGES: [[@LINE-1]]:22: warning: avoid marking return types as ``cons
+// CHECK-FIXES: const Clazz::Strukt* Clazz::p7() {}
+
+Clazz *const p10();
+// CHECK-FIXES: Clazz *p10();
+
+Clazz *const p10() {
+ // CHECK-MESSAGES: [[@LINE-1]]:8: warning: avoid marking return types as ``con
+ // CHECK-FIXES: Clazz *p10() {
+ return new Clazz();
+}
+
+const Clazz bar;
+const Clazz *const p11() {
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: avoid marking return types as ``co
+ // CHECK-FIXES: const Clazz *p11() {
+ return &bar;
+}
+
+const Klazz<const int> p12() {}
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: avoid marking return types as ``const
+// CHECK-FIXES: Klazz<const int> p12() {}
+
+const Klazz<const int>* const p13() {}
+// CHECK-MESSAGES: [[@LINE-1]]:25: warning: avoid marking return types as ``cons
+// CHECK-FIXES: const Klazz<const int>* p13() {}
+
+const auto p14() {
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: avoid marking return types as ``cons
+// CHECK-FIXES: auto p14() {
+ const int i = 0;
+ return i;
+}
+
+//another decl at top.
+const int p15();
+// CHECK-FIXES: int p15();
+const int p15() {
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: avoid marking return types as ``const
+// CHECK-FIXES: int p15() {
+ return 0;
+}
+
+const int n1 = 1;
+const Clazz n2 = Clazz();
+const Clazz* n3 = new Clazz();
+Clazz *const n4 = new Clazz();
+const Clazz *const n5 = new Clazz();
+constexpr int n6 = 6;
+constexpr int n7() { return 8; }
+const int eight = 8;
+constexpr const int* n8() { return &eight; }
+Klazz<const int> n9();
+const Klazz<const int>* n10();
+const Klazz<const int>& Clazz::n11(const Klazz<const int>) const {}
+const int n14();
+
+#define CONSTINT const int
+CONSTINT n12() {}
+
+// DEFFF in header.
+DEFFF n13() {}
Index: test/clang-tidy/Inputs/readability-const-value-return/macro-def.h
===================================================================
--- /dev/null
+++ test/clang-tidy/Inputs/readability-const-value-return/macro-def.h
@@ -0,0 +1,6 @@
+#ifndef MACRO_DEF_H
+#define MACRO_DEF_H
+
+#define DEFFF const int
+
+#endif // MACRO_DEF_H
Index: docs/clang-tidy/checks/readability-const-value-return.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/readability-const-value-return.rst
@@ -0,0 +1,25 @@
+.. title:: clang-tidy - readability-const-value-return
+
+readability-const-value-return
+===================================
+
+Checks for functions with a ``const``-qualified return type and recommends
+removal of the `const` keyword. Such use of ``const`` is superfluous, and
+prevents valuable compiler optimizations.
+
+Examples:
+
+.. code-block:: c++
+
+ const int foo();
+ const Clazz foo();
+ Clazz *const foo();
+
+Note that this does not apply to returning pointers or references to const
+values. These are fine:
+
+.. code-block:: c++
+
+ const int* foo();
+ const int& foo();
+ const Clazz* foo();
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -9,8 +9,8 @@
abseil-no-internal-dependencies
abseil-no-namespace
abseil-redundant-strcat-calls
- abseil-string-find-startswith
abseil-str-cat-append
+ abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
android-cloexec-creat
@@ -219,6 +219,7 @@
portability-simd-intrinsics
readability-avoid-const-params-in-decls
readability-braces-around-statements
+ readability-const-value-return
readability-container-size-empty
readability-delete-null-pointer
readability-deleted-default
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -57,6 +57,13 @@
Improvements to clang-tidy
--------------------------
+- New :doc:`readability-const-value-return
+ <clang-tidy/checks/readability-const-value-return>` check.
+
+ Checks for functions with a ``const``-qualified return type and recommends
+ removal of the `const` keyword. Such use of ``const`` is superfluous, and
+ prevents valuable compiler optimizations.
+
- New :doc:`abseil-duration-division
<clang-tidy/checks/abseil-duration-division>` check.
Index: clang-tidy/utils/LexerUtils.h
===================================================================
--- clang-tidy/utils/LexerUtils.h
+++ clang-tidy/utils/LexerUtils.h
@@ -22,6 +22,11 @@
Token getPreviousToken(SourceLocation Location, const SourceManager &SM,
const LangOptions &LangOpts, bool SkipComments = true);
+// Gets the start locations of all ``const`` tokens in the Range.
+std::vector<SourceLocation> getConstTokLocations(
+ CharSourceRange Range, const clang::ASTContext &Context,
+ const SourceManager &SM);
+
} // namespace lexer
} // namespace utils
} // namespace tidy
Index: clang-tidy/utils/LexerUtils.cpp
===================================================================
--- clang-tidy/utils/LexerUtils.cpp
+++ clang-tidy/utils/LexerUtils.cpp
@@ -31,6 +31,31 @@
return Token;
}
+std::vector<SourceLocation> getConstTokLocations(
+ CharSourceRange Range, const clang::ASTContext &Context,
+ const SourceManager &SM) {
+ std::pair<FileID, unsigned> LocInfo =
+ SM.getDecomposedLoc(Range.getBegin());
+ StringRef File = SM.getBufferData(LocInfo.first);
+ const char *TokenBegin = File.data() + LocInfo.second;
+ Lexer RawLexer(SM.getLocForStartOfFile(LocInfo.first), Context.getLangOpts(),
+ File.begin(), TokenBegin, File.end());
+ Token Tok;
+ std::vector<SourceLocation> Locations;
+ while (!RawLexer.LexFromRawLexer(Tok) &&
+ !SM.isBeforeInTranslationUnit(Range.getEnd(), Tok.getLocation())) {
+ if (Tok.is(tok::raw_identifier)) {
+ IdentifierInfo &Info = Context.Idents.get(
+ StringRef(SM.getCharacterData(Tok.getLocation()), Tok.getLength()));
+ Tok.setIdentifierInfo(&Info);
+ Tok.setKind(Info.getTokenID());
+ }
+ if (Tok.is(tok::kw_const))
+ Locations.push_back(Tok.getLocation());
+ }
+ return Locations;
+}
+
} // namespace lexer
} // namespace utils
} // namespace tidy
Index: clang-tidy/readability/ReadabilityTidyModule.cpp
===================================================================
--- clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -12,6 +12,7 @@
#include "../ClangTidyModuleRegistry.h"
#include "AvoidConstParamsInDecls.h"
#include "BracesAroundStatementsCheck.h"
+#include "ConstValueReturnCheck.h"
#include "ContainerSizeEmptyCheck.h"
#include "DeleteNullPointerCheck.h"
#include "DeletedDefaultCheck.h"
@@ -50,6 +51,8 @@
"readability-avoid-const-params-in-decls");
CheckFactories.registerCheck<BracesAroundStatementsCheck>(
"readability-braces-around-statements");
+ CheckFactories.registerCheck<ConstValueReturnCheck>(
+ "readability-const-value-return");
CheckFactories.registerCheck<ContainerSizeEmptyCheck>(
"readability-container-size-empty");
CheckFactories.registerCheck<DeleteNullPointerCheck>(
Index: clang-tidy/readability/ConstValueReturnCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/readability/ConstValueReturnCheck.h
@@ -0,0 +1,35 @@
+//===--- ConstValueReturnCheck.h - clang-tidy -------------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_CONSTVALUERETURNCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_CONSTVALUERETURNCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// Suggests removal of the `const` qualifier from any function that returns a
+/// const type.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability-const-value-return.html
+class ConstValueReturnCheck : public ClangTidyCheck {
+ public:
+ using ClangTidyCheck::ClangTidyCheck;
+ void registerMatchers(ast_matchers::MatchFinder* finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult& result) override;
+};
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_CONSTVALUERETURNCHECK_H
Index: clang-tidy/readability/ConstValueReturnCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/readability/ConstValueReturnCheck.cpp
@@ -0,0 +1,86 @@
+//===--- ConstValueReturnCheck.cpp - clang-tidy ---------------------------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "ConstValueReturnCheck.h"
+#include "../utils/LexerUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+#include "llvm/ADT/Optional.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+namespace {
+
+// Finds the location of the relevant "const" token in `Def`.
+llvm::Optional<SourceLocation> findConstToRemove(
+ const FunctionDecl *Def, const MatchFinder::MatchResult &Result) {
+ // Since either of the locs can be in a macro, use makeFileCharRange to be
+ // sure that we have a consistent char range, located entirely in the source
+ // file.
+ CharSourceRange FileRange = Lexer::makeFileCharRange(
+ CharSourceRange::getCharRange(Def->getBeginLoc(),
+ Def->getNameInfo().getLoc()),
+ *Result.SourceManager, Result.Context->getLangOpts());
+
+ if (FileRange.isInvalid())
+ return {};
+
+ auto Locs = utils::lexer::getConstTokLocations(FileRange, *Result.Context,
+ *Result.SourceManager);
+ if (Locs.empty())
+ return {};
+
+ // If the return type is a pointer, then remove `const` after asterisk, which
+ // will be the last const before function name; else, get first.
+ return Def->getReturnType()->isPointerType() ? Locs.back() : Locs.front();
+}
+
+} // namespace
+
+void ConstValueReturnCheck::registerMatchers(MatchFinder *Finder) {
+ // Find all the functions for which the return types are const qualified.
+ Finder->addMatcher(
+ functionDecl(returns(isConstQualified()), isDefinition()).bind("func"),
+ this);
+}
+
+void ConstValueReturnCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *Def = Result.Nodes.getNodeAs<FunctionDecl>("func");
+ if (!Def->getReturnType().isLocalConstQualified()) {
+ return;
+ }
+
+ // Fix the definition.
+ llvm::Optional<SourceLocation> Loc = findConstToRemove(Def, Result);
+ if (!Loc) return;
+ DiagnosticBuilder Diagnostics = diag(*Loc,
+ "avoid marking return types as ``const`` for values, as it often "
+ "disables optimizations and causes unnecessary copies")
+ << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(*Loc, *Loc));
+
+ // Fix any visible declarations, but don't warn with a separate message --
+ // associate all fixes with the definition.
+ for (auto *Decl = Def->getPreviousDecl(); Decl != nullptr;
+ Decl = Decl->getPreviousDecl()) {
+ if (const llvm::Optional<SourceLocation> PrevLoc =
+ findConstToRemove(Decl, Result)) {
+ Diagnostics << FixItHint::CreateRemoval(
+ CharSourceRange::getTokenRange(*PrevLoc, *PrevLoc));
+ }
+ }
+}
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/readability/CMakeLists.txt
===================================================================
--- clang-tidy/readability/CMakeLists.txt
+++ clang-tidy/readability/CMakeLists.txt
@@ -3,6 +3,7 @@
add_clang_library(clangTidyReadabilityModule
AvoidConstParamsInDecls.cpp
BracesAroundStatementsCheck.cpp
+ ConstValueReturnCheck.cpp
ContainerSizeEmptyCheck.cpp
DeleteNullPointerCheck.cpp
DeletedDefaultCheck.cpp
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
