JonasToth updated this revision to Diff 166041.
JonasToth added a comment.
- Merge branch 'master' into experiment_isolate_decl
- further work on isolate decl, fix tokenizing bug with templates
- include big test-suite and make them run
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51949
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/IsolateDeclCheck.cpp
clang-tidy/readability/IsolateDeclCheck.h
clang-tidy/readability/ReadabilityTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/readability-isolate-decl.rst
test/clang-tidy/readability-isolate-decl-cxx17.cpp
test/clang-tidy/readability-isolate-decl.cpp
Index: test/clang-tidy/readability-isolate-decl.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/readability-isolate-decl.cpp
@@ -0,0 +1,365 @@
+// RUN: %check_clang_tidy %s readability-isolate-decl %t
+
+void f() {
+ int i;
+}
+
+void f2() {
+ int i, j, *k, lala = 42;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 4 variables
+ // CHECK-FIXES: int i;
+ // CHECK-FIXES: {{^ }}int j;
+ // CHECK-FIXES: {{^ }}int *k;
+ // CHECK-FIXES: {{^ }}int lala = 42;
+
+ int normal, weird = /* comment */ 42;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+ // CHECK-FIXES: int normal;
+ // CHECK-FIXES: {{^ }}int weird = /* comment */ 42;
+ //
+ int /* here is a comment */ v1,
+ // another comment
+ v2 = 42 // Ok, more comments
+ ;
+ // CHECK-MESSAGES: [[@LINE-4]]:3: warning: this statement declares 2 variables
+ // CHECK-FIXES: int /* here is a comment */ v1;
+ // CHECK-FIXES: {{^ }}int /* here is a comment */ // another comment
+ // CHECK-FIXES: {{^ }}v2 = 42 // Ok, more comments
+ // CHECK-FIXES: {{^ }};
+}
+
+void f3() {
+ int i, *pointer1;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+ // CHECK-FIXES: int i;
+ // CHECK-FIXES: {{^ }}int *pointer1;
+ //
+ int *pointer2 = nullptr, *pointer3 = &i;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+ // CHECK-FIXES: int *pointer2 = nullptr;
+ // CHECK-FIXES: {{^ }}int *pointer3 = &i;
+}
+
+void f4() {
+ double d = 42. /* foo */, z = 43., /* hi */ y, c /* */ /* */, l = 2.;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 5 variables
+ // CHECK-FIXES: double d = 42. /* foo */;
+ // CHECK-FIXES: {{^ }}double z = 43.;
+ // CHECK-FIXES: {{^ }}double /* hi */ y;
+ // CHECK-FIXES: {{^ }}double c /* */ /* */;
+ // CHECK-FIXES: {{^ }}double l = 2.;
+}
+
+struct SomeClass {
+ SomeClass() = default;
+ SomeClass(int value);
+};
+void f5() {
+ SomeClass v1, v2(42), v3{42}, v4(42.5);
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 4 variables
+ // CHECK-FIXES: SomeClass v1;
+ // CHECK-FIXES: {{^ }}SomeClass v2(42);
+ // CHECK-FIXES: {{^ }}SomeClass v3{42};
+ // CHECK-FIXES: {{^ }}SomeClass v4(42.5);
+
+ SomeClass v5 = 42, *p1 = nullptr;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+ // CHECK-FIXES: SomeClass v5 = 42;
+ // CHECK-FIXES: {{^ }}SomeClass *p1 = nullptr;
+}
+
+void f6() {
+ int array1[] = {1, 2, 3, 4}, array2[] = {1, 2, 3}, value1, value2 = 42;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 4 variables
+ // CHECK-FIXES: int array1[] = {1, 2, 3, 4};
+ // CHECK-FIXES: {{^ }}int array2[] = {1, 2, 3};
+ // CHECK-FIXES: {{^ }}int value1;
+ // CHECK-FIXES: {{^ }}int value2 = 42;
+}
+
+template <typename T>
+struct TemplatedType {
+ TemplatedType() = default;
+ TemplatedType(T value);
+};
+
+void f7() {
+ TemplatedType<int> TT1(42), TT2{42}, TT3;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 3 variables
+ // CHECK-FIXES: TemplatedType<int> TT1(42);
+ // CHECK-FIXES: {{^ }}TemplatedType<int> TT2{42};
+ // CHECK-FIXES: {{^ }}TemplatedType<int> TT3;
+ //
+ TemplatedType<int *> *TT4(nullptr), TT5, **TT6 = nullptr, *const *const TT7{nullptr};
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 4 variables
+ // CHECK FIXES: TemplatedType<int *> *TT4(nullptr);
+ // CHECK FIXES: {{^ }}TemplatedType<int *> TT5;
+ // CHECK FIXES: {{^ }}TemplatedType<int *> **TT6 = nullptr;
+ // CHECK FIXES: {{^ }}TemplatedType<int *> *const *const TT7{nullptr};
+ //
+ TemplatedType<int &> **TT8(nullptr), TT9;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+}
+
+void forbidden_transformations() {
+ for (int i = 0, j = 42; i < j; ++i)
+ ;
+}
+
+#define NULL 0
+#define MY_NICE_TYPE int **
+#define VAR_NAME(name) name##__LINE__
+#define A_BUNCH_OF_VARIABLES int m1 = 42, m2 = 43, m3 = 44;
+
+void macros() {
+ int *p1 = NULL, *p2 = NULL;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+ // CHECK-FIXES: int *p1 = NULL;
+ // CHECK-FIXES: {{^ }}int *p2 = NULL;
+
+ // Macros are involved, so there will be no transformation
+ MY_NICE_TYPE p3, v1, v2;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 3 variables
+
+ int VAR_NAME(v3),
+ VAR_NAME(v4),
+ VAR_NAME(v5);
+ // CHECK-MESSAGES: [[@LINE-3]]:3: warning: this statement declares 3 variables
+
+ A_BUNCH_OF_VARIABLES
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 3 variables
+}
+
+void dontTouchParameter(int param1, int param2) {}
+
+struct StructOne {
+ StructOne() {}
+ StructOne(int b) {}
+
+ int member1, member2;
+ // TODO Transform FieldDecl's as well
+};
+
+using PointerType = int;
+
+struct {
+ int i;
+} AS1, AS2;
+struct TemT {
+ template <typename T>
+ T *getAs() {
+ return nullptr;
+ }
+} TT1, TT2;
+
+void complex() {
+ typedef int *IntPtr;
+ typedef int ArrayType[2];
+ typedef int FunType(void);
+
+ IntPtr intptr1, intptr2 = nullptr, intptr3;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 3 variables
+ // TODO SourceText is invalid error
+ // CHECK FIXES: IntPtr intptr1;
+ // CHECK FIXES: {{^ }}IntPtr intptr2 = nullptr;
+ // CHECK FIXES: {{^ }}IntPtr intptr3;
+
+ ArrayType arraytype1, arraytype2 = {1}, arraytype3;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 3 variables
+ // CHECK-FIXES: ArrayType arraytype1;
+ // CHECK-FIXES: {{^ }}ArrayType arraytype2 = {1};
+ // CHECK-FIXES: {{^ }}ArrayType arraytype3;
+
+ // Don't touch function declarations.
+ FunType funtype1, funtype2, functype3;
+
+ for (int index1 = 0, index2 = 0;;) {
+ int localFor1 = 1, localFor2 = 2;
+ // CHECK-MESSAGES: [[@LINE-1]]:5: warning: this statement declares 2 variables
+ // CHECK-FIXES: int localFor1 = 1;
+ // CHECK-FIXES: {{^ }}int localFor2 = 2;
+ }
+
+ StructOne s1, s2(23), s3, s4(3), *sptr = new StructOne(2);
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 5 variables
+ // CHECK-FIXES: StructOne s1;
+ // CHECK-FIXES: {{^ }}StructOne s2(23);
+ // CHECK-FIXES: {{^ }}StructOne s3;
+ // CHECK-FIXES: {{^ }}StructOne s4(3);
+ // CHECK-FIXES: {{^ }}StructOne *sptr = new StructOne(2);
+
+ struct StructOne cs1, cs2(42);
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+ // CHECK-FIXES: struct StructOne cs1;
+ // CHECK-FIXES: {{^ }}struct StructOne cs2(42);
+
+ int *ptrArray[3], dummy, **ptrArray2[5], twoDim[2][3], *twoDimPtr[2][3];
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 5 variables
+ // CHECK-FIXES: int *ptrArray[3];
+ // CHECK-FIXES: {{^ }}int dummy;
+ // CHECK-FIXES: {{^ }}int **ptrArray2[5];
+ // CHECK-FIXES: {{^ }}int twoDim[2][3];
+ // CHECK-FIXES: {{^ }}int *twoDimPtr[2][3];
+
+ {
+ void f1(int), g1(int, float);
+ }
+
+ {
+ void gg(int, float);
+
+ void (*f2)(int), (*g2)(int, float) = gg;
+ // CHECK-MESSAGES: [[@LINE-1]]:5: warning: this statement declares 2 variables
+ // CHECK-FIXES: void (*f2)(int);
+ // CHECK-FIXES: {{^ }}void (*g2)(int, float) = gg;
+
+ void /*(*/ (/*(*/ *f3)(int), (*g3)(int, float);
+ // CHECK-MESSAGES: [[@LINE-1]]:5: warning: this statement declares 2 variables
+ // CHECK FIXES: void /*(*/ (/*(*/ *f3)(int);
+ // CHECK FIXES: {{^ }}void /*(*/ (*g3)(int, float);
+ }
+
+ struct S {
+ int a;
+ const int b;
+ void f() {}
+ };
+
+ int S::*p = &S::a, S::*const q = &S::a;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+ // CHECK FIXES: int S::*p = &S::a;
+ // CHECK FIXES: {{^ }}int S::*const q = &S::a;
+
+ int /* :: */ S::*pp2 = &S::a, var1 = 0;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+ // CHECK FIXES: int /* :: */ S::*pp2 = &S::a;
+ // CHECK FIXES: {{^ }}int var1 = 0;
+
+ const int S::*r = &S::b, S::*t;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+ // CHECK FIXES: const int S::*r = &S::b;
+ // CHECK FIXES: {{^ }}const int S::*t;
+
+ {
+ int S::*mdpa1[2] = {&S::a, &S::a}, var1 = 0;
+ // CHECK-MESSAGES: [[@LINE-1]]:5: warning: this statement declares 2 variables
+ // CHECK FIXES: int S::*mdpa1[2] = {&S::a, &S::a};
+ // CHECK FIXES: {{^ }}int var1 = 0;
+
+ int S ::**mdpa2[2] = {&p, &pp2}, var2 = 0;
+ // CHECK-MESSAGES: [[@LINE-1]]:5: warning: this statement declares 2 variables
+ // CHECK FIXES: int S ::**mdpa2[2] = {&p, &pp2};
+ // CHECK FIXES: {{^ }}int var2 = 0;
+
+ void (S::*mdfp1)() = &S::f, (S::*mdfp2)() = &S::f;
+ // CHECK-MESSAGES: [[@LINE-1]]:5: warning: this statement declares 2 variables
+ // CHECK FIXES: void (S::*mdfp1)() = &S::f;
+ // CHECK FIXES: {{^ }}void (S::*mdfp2)() = &S::f;
+
+ void (S::*mdfpa1[2])() = {&S::f, &S::f}, (S::*mdfpa2)() = &S::f;
+ // CHECK-MESSAGES: [[@LINE-1]]:5: warning: this statement declares 2 variables
+ // CHECK FIXES: void (S::*mdfpa1[2])() = {&S::f, &S::f};
+ // CHECK FIXES: {{^ }}void (S::*mdfpa2)() = &S::f;
+
+ void (S::* * mdfpa3[2])() = {&mdfpa1[0], &mdfpa1[1]}, (S::*mdfpa4)() = &S::f;
+ // CHECK-MESSAGES: [[@LINE-1]]:5: warning: this statement declares 2 variables
+ // CHECK FIXES: void (S::* * mdfpa3[2])() = {&mdfpa1[0], &mdfpa1[1]};
+ // CHECK FIXES: {{^ }}void (S::*mdfpa4)() = &S::f;
+ }
+
+ typedef const int S::*MemPtr;
+ MemPtr aaa = &S::a, bbb = &S::b;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+ // CHECK FIXES: MemPtr aaa = &S::a;
+ // CHECK FIXES: {{^ }}MemPtr bbb = &S::b;
+
+ class CS {
+ public:
+ int a;
+ const int b;
+ };
+ int const CS ::*pp = &CS::a, CS::*const qq = &CS::a;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+ // CHECK FIXES: int const CS ::*pp = &CS::a;
+ // CHECK FIXES: {{^ }}int const CS::*const qq = &CS::a;
+
+ // clang-format off
+ auto returner = []() { return int(32); };
+ int intfunction = returner(), intarray[] =
+ {
+ 1,
+ 2,
+ 3,
+ 4
+ }, bb = 4;
+ // CHECK-MESSAGES: [[@LINE-7]]:3: warning: this statement declares 3 variables
+ // CHECK-FIXES: int intfunction = returner();
+ // CHECK-FIXES: {{^ }}int intarray[] =
+ // CHECK-FIXES: {{^ }}{
+ // CHECK-FIXES: {{^ }}1,
+ // CHECK-FIXES: {{^ }}2,
+ // CHECK-FIXES: {{^ }}3,
+ // CHECK-FIXES: {{^ }}4
+ // CHECK-FIXES: {{^ }}};
+ // CHECK-FIXES: {{^ }}int bb = 4;
+ // clang-format on
+
+ TemT *T1 = &TT1, *T2 = &TT2;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+ // CHECK-FIXES: TemT *T1 = &TT1;
+ // CHECK-FIXES: {{^ }}TemT *T2 = &TT2;
+
+ const PointerType *PT1 = T1->getAs<PointerType>(),
+ *PT2 = T2->getAs<PointerType>();
+ // CHECK-MESSAGES: [[@LINE-2]]:3: warning: this statement declares 2 variables
+ // CHECK-FIXES: const PointerType *PT1 = T1->getAs<PointerType>();
+ // CHECK-FIXES: {{^ }}const PointerType *PT2 = T2->getAs<PointerType>();
+
+ // bool defPre = false,
+ // #ifdef IS_ENABLED
+ // defTest = false;
+ // #else
+ // defTest = true;
+ // #endif
+ // CHECK MESSAGES: [[@LINE-6]]:3: warning: this statement declares 2 variables
+ // CHECK FIXES: bool defPre = false;
+ // CHECK FIXES: {{^ }}bool
+ // CHECK FIXES: {{^#}}ifdef IS_ENABLED
+ // CHECK FIXES: {{^ }}defTest = false;
+ // CHECK FIXES: {{^#}}else
+ // CHECK FIXES: {{^ }}defTest = true;
+
+ const int *p1 = nullptr;
+ const int *p2 = nullptr;
+
+ const int *&pref1 = p1, *&pref2 = p2;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+ // CHECK-FIXES: const int *&pref1 = p1;
+ // CHECK-FIXES: {{^ }}const int *&pref2 = p2;
+
+ // clang-format off
+ const char *literal1 = "clang" "test"\
+ "one",
+ *literal2 = "empty", literal3[] = "three";
+ // CHECK-MESSAGES: [[@LINE-3]]:3: warning: this statement declares 3 variables
+ // CHECK-FIXES: const char *literal1 = "clang" "test"\
+ // CHECK-FIXES: {{^ }}"one";
+ // CHECK-FIXES: {{^ }}const char *literal2 = "empty";
+ // CHECK-FIXES: {{^ }}const char literal3[] = "three";
+ // clang-format on
+}
+
+typedef int *tptr, tbt;
+typedef int (&tfp)(int, long), tarr[10];
+typedef int tarr2[10], tct;
+
+template <typename A, typename B>
+void should_not_be_touched(A, B);
+
+int variable, function(void);
+
+int call_func_with_sideeffect();
+void bad_if_decl() {
+ if (true)
+ int i, j, k = call_func_with_sideeffect();
+}
Index: test/clang-tidy/readability-isolate-decl-cxx17.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/readability-isolate-decl-cxx17.cpp
@@ -0,0 +1,105 @@
+// RUN: %check_clang_tidy %s readability-isolate-decl %t -- -- -std=c++17
+
+template <typename T1, typename T2>
+struct pair {
+ T1 first;
+ T2 second;
+ pair(T1 v1, T2 v2) : first(v1), second(v2) {}
+
+ template <int N>
+ decltype(auto) get() const {
+ if constexpr (N == 0)
+ return first;
+ else if constexpr (N == 1)
+ return second;
+ }
+};
+
+void forbidden_transformations() {
+ if (int i = 42, j = i; i == j)
+ ;
+
+ auto [i, j] = pair<int, int>(42, 42);
+}
+
+struct SomeClass {
+ SomeClass() = default;
+ SomeClass(int value);
+};
+
+namespace std {
+template <typename T>
+class initializer_list {};
+
+template <typename T>
+class vector {
+public:
+ vector() = default;
+ vector(initializer_list<T> init) {}
+};
+
+class string {
+public:
+ string() = default;
+ string(const char *) {}
+};
+
+namespace string_literals {
+string operator""s(const char *, decltype(sizeof(int))) {
+ return string();
+}
+} // namespace string_literals
+} // namespace std
+
+namespace Types {
+typedef int MyType;
+} // namespace Types
+
+int touch1, touch2;
+// TODO This is not a 'declStmt', why?
+// CHECK MESSAGES: [[@LINE-1]]:1: warning: this statement declares 2 variables
+// CHECK FIXES: {{^}}int touch1;
+// CHECK FIXES: {{^}}int touch2;
+
+void modern() {
+ auto autoInt1 = 3, autoInt2 = 4;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+ // CHECK-FIXES: auto autoInt1 = 3;
+ // CHECK-FIXES: {{^ }}auto autoInt2 = 4;
+
+ decltype(int()) declnottouch = 4;
+ decltype(int()) declint1 = 5, declint2 = 3;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+ // CHECK-FIXES: decltype(int()) declint1 = 5;
+ // CHECK-FIXES: {{^ }}decltype(int()) declint2 = 3;
+
+ std::vector<int> vectorA = {1, 2}, vectorB = {1, 2, 3}, vectorC({1, 1, 1});
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 3 variables
+ // CHECK-FIXES: std::vector<int> vectorA = {1, 2};
+ // CHECK-FIXES: {{^ }}std::vector<int> vectorB = {1, 2, 3};
+ // CHECK-FIXES: {{^ }}std::vector<int> vectorC({1, 1, 1});
+
+ using uType = int;
+ uType utype1, utype2;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+ // CHECK-FIXES: uType utype1;
+ // CHECK-FIXES: {{^ }}uType utype2;
+
+ Types::MyType mytype1, mytype2, mytype3 = 3;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 3 variables
+ // CHECK-FIXES: Types::MyType mytype1;
+ // CHECK-FIXES: {{^ }}Types::MyType mytype2;
+ // CHECK-FIXES: {{^ }}Types::MyType mytype3 = 3;
+
+ {
+ using namespace std::string_literals;
+
+ std::vector<std::string> s{"foo"s, "bar"s}, t{"foo"s}, u, a({"hey", "you"}), bb = {"h", "a"};
+ // CHECK-MESSAGES: [[@LINE-1]]:5: warning: this statement declares 5 variables
+ // CHECK-FIXES: std::vector<std::string> s{"foo"s, "bar"s};
+ // CHECK-FIXES: {{^ }}std::vector<std::string> t{"foo"s};
+ // CHECK-FIXES: {{^ }}std::vector<std::string> u;
+ // CHECK-FIXES: {{^ }}std::vector<std::string> a({"hey", "you"});
+ // CHECK-FIXES: {{^ }}std::vector<std::string> bb = {"h", "a"};
+ }
+}
Index: docs/clang-tidy/checks/readability-isolate-decl.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/readability-isolate-decl.rst
@@ -0,0 +1,6 @@
+.. title:: clang-tidy - readability-isolate-decl
+
+readability-isolate-decl
+========================
+
+FIXME: Describe what patterns does the check detect and why. Give examples.
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -225,6 +225,7 @@
readability-identifier-naming
readability-implicit-bool-conversion
readability-inconsistent-declaration-parameter-name
+ readability-isolate-decl
readability-magic-numbers
readability-misleading-indentation
readability-misplaced-array-index
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -93,6 +93,11 @@
Flags uses of ``absl::StrCat()`` to append to a ``std::string``. Suggests
``absl::StrAppend()`` should be used instead.
+- New :doc:`readability-isolate-decl
+ <clang-tidy/checks/readability-isolate-decl>` check.
+
+ FIXME: add release notes.
+
- New :doc:`readability-magic-numbers
<clang-tidy/checks/readability-magic-numbers>` check.
Index: clang-tidy/readability/ReadabilityTidyModule.cpp
===================================================================
--- clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -20,6 +20,7 @@
#include "IdentifierNamingCheck.h"
#include "ImplicitBoolConversionCheck.h"
#include "InconsistentDeclarationParameterNameCheck.h"
+#include "IsolateDeclCheck.h"
#include "MagicNumbersCheck.h"
#include "MisleadingIndentationCheck.h"
#include "MisplacedArrayIndexCheck.h"
@@ -66,6 +67,8 @@
"readability-implicit-bool-conversion");
CheckFactories.registerCheck<InconsistentDeclarationParameterNameCheck>(
"readability-inconsistent-declaration-parameter-name");
+ CheckFactories.registerCheck<IsolateDeclCheck>(
+ "readability-isolate-decl");
CheckFactories.registerCheck<MagicNumbersCheck>(
"readability-magic-numbers");
CheckFactories.registerCheck<MisleadingIndentationCheck>(
Index: clang-tidy/readability/IsolateDeclCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/readability/IsolateDeclCheck.h
@@ -0,0 +1,35 @@
+//===--- IsolateDeclCheck.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_ISOLATEDECLCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_ISOLATEDECLCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// FIXME: Write a short description.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability-isolate-decl.html
+class IsolateDeclCheck : public ClangTidyCheck {
+public:
+ IsolateDeclCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ 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_ISOLATEDECLCHECK_H
Index: clang-tidy/readability/IsolateDeclCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/readability/IsolateDeclCheck.cpp
@@ -0,0 +1,375 @@
+//===--- IsolateDeclCheck.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 "IsolateDeclCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+#include "llvm/Support/Error.h"
+
+#include <iostream>
+#include <numeric>
+
+using namespace clang::ast_matchers;
+
+#define PRINT_DEBUG 1
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+namespace {
+AST_MATCHER(DeclStmt, isSingleDecl) { return Node.isSingleDecl(); }
+AST_MATCHER(DeclStmt, isVariablesOnly) {
+ return std::all_of(Node.decl_begin(), Node.decl_end(),
+ [](Decl *D) { return isa<VarDecl>(D); });
+}
+
+AST_MATCHER_P(IfStmt, hasInit, ast_matchers::internal::Matcher<Stmt>,
+ InnerMatcher) {
+ const Stmt *const InitStmt = Node.getInit();
+ if (InitStmt)
+ return InnerMatcher.matches(*InitStmt, Finder, Builder);
+ return false;
+}
+} // namespace
+
+void IsolateDeclCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ stmt(allOf(
+ declStmt(allOf(isVariablesOnly(), unless(isSingleDecl()),
+ hasParent(compoundStmt())))
+ .bind("decl_stmt"),
+ unless(
+ hasAncestor(forStmt(hasLoopInit(equalsBoundNode("decl_stmt"))))),
+ unless(hasAncestor(ifStmt(hasInit(equalsBoundNode("decl_stmt"))))))),
+ this);
+}
+
+template <typename TokenKind, typename... TokenKinds>
+static SourceLocation findNextAnyTokenKind(SourceLocation Start,
+ const SourceManager &SM,
+ const LangOptions &LangOpts,
+ TokenKind TK, TokenKinds... TKs) {
+ while (true) {
+ Optional<Token> CurrentToken = Lexer::findNextToken(Start, SM, LangOpts);
+
+ if (!CurrentToken.hasValue())
+ return SourceLocation();
+
+ Token PotentialMatch = *CurrentToken;
+ if (PotentialMatch.isOneOf(TK, TKs...))
+ return PotentialMatch.getLocation();
+
+ Start =
+ Lexer::getLocForEndOfToken(Start, 0, SM, LangOpts).getLocWithOffset(1);
+ }
+}
+
+static SourceLocation findNextTerminator(SourceLocation Start,
+ const SourceManager &SM,
+ const LangOptions &LangOpts) {
+ return findNextAnyTokenKind(Start, SM, LangOpts, tok::comma, tok::semi);
+}
+
+SourceLocation findPreviousTokenStart(SourceLocation Start,
+ const SourceManager &SM,
+ const LangOptions &LangOpts) {
+ if (Start.isInvalid() || Start.isMacroID())
+ return SourceLocation();
+
+ SourceLocation BeforeStart = Start.getLocWithOffset(-1);
+ if (BeforeStart.isInvalid() || BeforeStart.isMacroID())
+ return SourceLocation();
+
+ return Lexer::GetBeginningOfToken(BeforeStart, SM, LangOpts);
+}
+
+static SourceLocation findPreviousTokenKind(SourceLocation Start,
+ const SourceManager &SM,
+ const LangOptions &LangOpts,
+ tok::TokenKind TK) {
+ while (true) {
+ SourceLocation L = findPreviousTokenStart(Start, SM, LangOpts);
+ if (L.isInvalid() || L.isMacroID())
+ return SourceLocation();
+
+ Token T;
+ bool FailedRetrievingToken = Lexer::getRawToken(L, T, SM, LangOpts);
+
+ if (FailedRetrievingToken)
+ return SourceLocation();
+
+ if (T.is(TK))
+ return T.getLocation();
+
+ Start = L;
+ }
+}
+template <typename TokenKind, typename... TokenKinds>
+static SourceLocation
+findPreviousAnyTokenKind(SourceLocation Start, const SourceManager &SM,
+ const LangOptions &LangOpts, TokenKind TK,
+ TokenKinds... TKs) {
+ while (true) {
+ SourceLocation L = findPreviousTokenStart(Start, SM, LangOpts);
+ if (L.isInvalid() || L.isMacroID())
+ return SourceLocation();
+
+ Token T;
+ bool FailedRetrievingToken = Lexer::getRawToken(L, T, SM, LangOpts);
+
+ if (FailedRetrievingToken)
+ return SourceLocation();
+
+ if (T.isOneOf(TK, TKs...))
+ return T.getLocation();
+
+ Start = L;
+ }
+}
+
+static SourceLocation findStartOfIndirection(SourceLocation Start,
+ int Indirections,
+ const SourceManager &SM,
+ const LangOptions &LangOpts) {
+ assert(Indirections >= 0 && "Indirections must be non-negative");
+ if (Indirections == 0)
+ return Start;
+
+ for (int i = Indirections; i > 0; --i) {
+ Start = findPreviousAnyTokenKind(Start, SM, LangOpts, tok::star, tok::amp);
+ if (Start.isInvalid() || Start.isMacroID())
+ return SourceLocation();
+ }
+ return Start;
+}
+
+static bool isMacroID(SourceRange R) {
+ return R.getBegin().isMacroID() || R.getEnd().isMacroID();
+}
+
+static int countIndirections(const Type *T, int Indirections = 0) {
+ assert(T && "Require non-null");
+ if (T->isPointerType() || T->isReferenceType())
+ return countIndirections(T->getPointeeType().getTypePtr(), ++Indirections);
+
+ if (T->isArrayType()) {
+ const auto *AT = dyn_cast<ArrayType>(T->getCanonicalTypeInternal());
+ // Note: Do not increment the 'Indirections' because it is not yet clear
+ // if there is an indirection added in the source code of the array
+ // declaration.
+ return countIndirections(AT->getElementType().getTypePtr(), Indirections);
+ }
+ return Indirections;
+}
+
+/// This function assumes, that \p DS only contains VarDecl elements.
+static Optional<std::vector<SourceRange>>
+DeclSlice(const DeclStmt *DS, const SourceManager &SM,
+ const LangOptions &LangOpts) {
+ std::size_t DeclCount = std::distance(DS->decl_begin(), DS->decl_end());
+ if (DeclCount < 2) {
+#if PRINT_DEBUG
+ std::cerr << "Not enough Declarations (" << DeclCount
+ << ") in statement to create isolated declarations\n";
+#endif
+ return None;
+ }
+
+ // The initial type of the declaration and each declaration has it's slice.
+ std::vector<SourceRange> Slices;
+ Slices.reserve(DeclCount + 1);
+
+#if PRINT_DEBUG
+ std::cerr << "Number of Slices to create: " << Slices.capacity() << "\n";
+#endif
+
+ // Special case for the type where the SourceRange is computed
+ // differently.
+ VarDecl *FirstDecl = dyn_cast<VarDecl>(*DS->decl_begin());
+ assert(FirstDecl && "Expect only VarDecls");
+
+ if (FirstDecl->getType()->isMemberPointerType() ||
+ FirstDecl->getType()->isMemberDataPointerType() ||
+ FirstDecl->getType()->isMemberFunctionPointerType())
+ return None;
+
+ SourceLocation Start = findStartOfIndirection(
+ FirstDecl->getLocation(),
+ countIndirections(FirstDecl->getType().getTypePtr()), SM, LangOpts);
+
+ // Fix function-pointer declarations that have a '(' in front of the
+ // pointer.
+ if (FirstDecl->getType()->isFunctionPointerType())
+ Start = findPreviousTokenKind(Start, SM, LangOpts, tok::l_paren);
+
+#if PRINT_DEBUG
+ std::cerr << "Start of Indirection: " << Start.printToString(SM) << "\n";
+#endif
+
+ SourceRange DeclRange(DS->getBeginLoc(), Start);
+ if (DeclRange.isInvalid()) {
+#if PRINT_DEBUG
+ std::cerr << "DeclRange is invalid\n";
+#endif
+ return None;
+ }
+ if (isMacroID(DeclRange)) {
+#if PRINT_DEBUG
+ std::cerr << "DeclRange is in macro\n";
+#endif
+ return None;
+ }
+
+ Slices.emplace_back(DeclRange);
+
+ SourceLocation DeclBegin = Start;
+ for (auto It = DS->decl_begin(), End = DS->decl_end(); It != End; ++It) {
+ VarDecl *CurrentDecl = dyn_cast<VarDecl>(*It);
+ assert(CurrentDecl && "Expect only VarDecls here");
+
+ if (CurrentDecl->getType()->isMemberPointerType() ||
+ CurrentDecl->getType()->isMemberDataPointerType() ||
+ CurrentDecl->getType()->isMemberFunctionPointerType())
+ return None;
+
+ SourceLocation DeclEnd;
+ if (CurrentDecl->hasInit())
+ DeclEnd =
+ findNextTerminator(CurrentDecl->getInit()->getEndLoc(), SM, LangOpts);
+ else
+ DeclEnd = findNextTerminator(CurrentDecl->getEndLoc(), SM, LangOpts);
+
+#if PRINT_DEBUG
+ std::cerr << "Start of Slice: " << DeclBegin.printToString(SM) << "\n";
+ std::cerr << "End of Slice: " << DeclEnd.printToString(SM) << "\n";
+#endif
+
+ SourceRange VarNameRange(DeclBegin, DeclEnd);
+ if (VarNameRange.isInvalid()) {
+#if PRINT_DEBUG
+ std::cerr << "VarNameRange is invalid\n";
+#endif
+ return None;
+ }
+ if (isMacroID(VarNameRange)) {
+#if PRINT_DEBUG
+ std::cerr << "VarNameRange is in macro\n";
+#endif
+ return None;
+ }
+
+ Slices.emplace_back(VarNameRange);
+ DeclBegin = DeclEnd.getLocWithOffset(1);
+ }
+
+ return Slices;
+}
+
+static Optional<std::vector<StringRef>>
+SourceFromRanges(const std::vector<SourceRange> &Ranges,
+ const SourceManager &SM, const LangOptions &LangOpts) {
+ std::vector<StringRef> Snippets;
+ Snippets.reserve(Ranges.size());
+
+ for (const auto &Range : Ranges) {
+ CharSourceRange CR = Lexer::getAsCharRange(
+ CharSourceRange::getCharRange(Range.getBegin(), Range.getEnd()), SM,
+ LangOpts);
+
+ if (CR.isInvalid()) {
+#if PRINT_DEBUG
+ std::cerr << "CharSourceRange is invalid\n";
+#endif
+ return None;
+ }
+
+ bool InvalidText = false;
+ StringRef Snippet = Lexer::getSourceText(CR, SM, LangOpts, &InvalidText);
+
+ if (InvalidText) {
+#if PRINT_DEBUG
+ std::cerr << "SourcetText in invalid\n";
+#endif
+ return None;
+ }
+
+ Snippets.emplace_back(Snippet);
+ }
+
+ return Snippets;
+}
+
+/// Excepts a vector {TypeSnippet, Firstdecl, SecondDecl, ...}.
+static std::vector<std::string>
+CreateIsolatedDecls(const std::vector<StringRef> &Snippets) {
+ // The first section is the type snippet, which does not make a decl itself.
+ assert(Snippets.size() >= 2 && "No enough snippets to create isolated decls");
+ std::vector<std::string> Decls(Snippets.size() - 1);
+
+ for (std::size_t i = 1, End = Snippets.size(); i < End; ++i)
+ Decls[i - 1] = Twine(Snippets[0].str()).concat(Snippets[i].ltrim()).str();
+
+ return Decls;
+}
+
+static std::string &TrimRight(std::string &str) {
+ auto it1 = std::find_if(str.rbegin(), str.rend(), [](char ch) {
+ return !std::isspace<char>(ch, std::locale::classic());
+ });
+ str.erase(it1.base(), str.end());
+ return str;
+}
+
+static std::string Concat(const std::vector<std::string> &Decls,
+ StringRef Indent) {
+ std::string R;
+ for (const StringRef &D : Decls)
+ R += Twine(D).concat(";\n").concat(Indent).str();
+ return TrimRight(R);
+}
+
+void IsolateDeclCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *WholeDecl = Result.Nodes.getNodeAs<DeclStmt>("decl_stmt");
+
+ auto Diag =
+ diag(WholeDecl->getBeginLoc(), "this statement declares %0 variables")
+ << static_cast<unsigned int>(
+ std::distance(WholeDecl->decl_begin(), WholeDecl->decl_end()));
+
+ Optional<std::vector<SourceRange>> PotentialSlices =
+ DeclSlice(WholeDecl, *Result.SourceManager, getLangOpts());
+ if (!PotentialSlices)
+ return;
+
+ const std::vector<SourceRange> &Slices = *PotentialSlices;
+
+ Optional<std::vector<StringRef>> PotentialSnippets =
+ SourceFromRanges(Slices, *Result.SourceManager, getLangOpts());
+
+ if (!PotentialSnippets)
+ return;
+
+ std::vector<StringRef> Snippets = *PotentialSnippets;
+
+ std::vector<std::string> NewDecls = CreateIsolatedDecls(Snippets);
+ std::string Replacement =
+ Concat(NewDecls, Lexer::getIndentationForLine(WholeDecl->getBeginLoc(),
+ *Result.SourceManager));
+#if PRINT_DEBUG
+ std::cerr << Replacement << "\n";
+#endif
+
+ Diag << FixItHint::CreateReplacement(WholeDecl->getSourceRange(),
+ Replacement);
+}
+} // namespace readability
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/readability/CMakeLists.txt
===================================================================
--- clang-tidy/readability/CMakeLists.txt
+++ clang-tidy/readability/CMakeLists.txt
@@ -11,6 +11,7 @@
IdentifierNamingCheck.cpp
ImplicitBoolConversionCheck.cpp
InconsistentDeclarationParameterNameCheck.cpp
+ IsolateDeclCheck.cpp
MagicNumbersCheck.cpp
MisleadingIndentationCheck.cpp
MisplacedArrayIndexCheck.cpp
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
