Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index
This revision was automatically updated to reflect the committed changes.
Closed by commit rL281206: [clang-tidy] readability-misplaced-array-index: add
new check that warns when… (authored by danielmarjamaki).
Changed prior to commit:
https://reviews.llvm.org/D21134?vs=69558=70994#toc
Repository:
rL LLVM
https://reviews.llvm.org/D21134
Files:
clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/readability/MisplacedArrayIndexCheck.cpp
clang-tools-extra/trunk/clang-tidy/readability/MisplacedArrayIndexCheck.h
clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/readability-misplaced-array-index.rst
clang-tools-extra/trunk/test/clang-tidy/readability-misplaced-array-index.cpp
Index: clang-tools-extra/trunk/test/clang-tidy/readability-misplaced-array-index.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/readability-misplaced-array-index.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/readability-misplaced-array-index.cpp
@@ -0,0 +1,34 @@
+// RUN: %check_clang_tidy %s readability-misplaced-array-index %t
+
+#define ABC "abc"
+
+struct XY { int *X; int *Y; };
+
+void dostuff(int);
+
+void unusualSyntax(int *P1, struct XY *P2) {
+ 10[P1] = 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: confusing array subscript expression, usually the index is inside the []
+ // CHECK-FIXES: P1[10] = 0;
+
+ 10[P2->X] = 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: confusing array subscript expression
+ // CHECK-FIXES: P2->X[10] = 0;
+
+ dostuff(1["abc"]);
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: confusing array subscript expression
+ // CHECK-FIXES: dostuff("abc"[1]);
+
+ dostuff(1[ABC]);
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: confusing array subscript expression
+ // CHECK-FIXES: dostuff(ABC[1]);
+
+ dostuff(0[0 + ABC]);
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: confusing array subscript expression
+ // CHECK-FIXES: dostuff(0[0 + ABC]);
+ // No fixit. Probably the code should be ABC[0]
+}
+
+void normalSyntax(int *X) {
+ X[10] = 0;
+}
Index: clang-tools-extra/trunk/clang-tidy/readability/MisplacedArrayIndexCheck.h
===
--- clang-tools-extra/trunk/clang-tidy/readability/MisplacedArrayIndexCheck.h
+++ clang-tools-extra/trunk/clang-tidy/readability/MisplacedArrayIndexCheck.h
@@ -0,0 +1,36 @@
+//===--- MisplacedArrayIndexCheck.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_MISPLACED_ARRAY_INDEX_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_MISPLACED_ARRAY_INDEX_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// Warn about unusual array index syntax (`index[array]` instead of
+/// `array[index]`).
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability-misplaced-array-index.html
+class MisplacedArrayIndexCheck : public ClangTidyCheck {
+public:
+ MisplacedArrayIndexCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult ) override;
+};
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_MISPLACED_ARRAY_INDEX_H
Index: clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp
===
--- clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -19,6 +19,7 @@
#include "IdentifierNamingCheck.h"
#include "ImplicitBoolCastCheck.h"
#include "InconsistentDeclarationParameterNameCheck.h"
+#include "MisplacedArrayIndexCheck.h"
#include "NamedParameterCheck.h"
#include "NonConstParameterCheck.h"
#include "RedundantControlFlowCheck.h"
@@ -54,6 +55,8 @@
"readability-implicit-bool-cast");
CheckFactories.registerCheck(
"readability-inconsistent-declaration-parameter-name");
+CheckFactories.registerCheck(
+"readability-misplaced-array-index");
CheckFactories.registerCheck(
"readability-static-definition-in-anonymous-namespace");
CheckFactories.registerCheck(
Index: clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt
Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index
danielmarjamaki marked 4 inline comments as done. danielmarjamaki added a comment. https://reviews.llvm.org/D21134 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index
danielmarjamaki updated this revision to Diff 69558.
danielmarjamaki marked 2 inline comments as done.
danielmarjamaki added a comment.
fix review comments
https://reviews.llvm.org/D21134
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/MisplacedArrayIndexCheck.cpp
clang-tidy/readability/MisplacedArrayIndexCheck.h
clang-tidy/readability/ReadabilityTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/readability-misplaced-array-index.rst
test/clang-tidy/readability-misplaced-array-index.cpp
Index: test/clang-tidy/readability-misplaced-array-index.cpp
===
--- test/clang-tidy/readability-misplaced-array-index.cpp
+++ test/clang-tidy/readability-misplaced-array-index.cpp
@@ -0,0 +1,34 @@
+// RUN: %check_clang_tidy %s readability-misplaced-array-index %t
+
+#define ABC "abc"
+
+struct XY { int *X; int *Y; };
+
+void dostuff(int);
+
+void unusualSyntax(int *P1, struct XY *P2) {
+ 10[P1] = 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: confusing array subscript expression, usually the index is inside the []
+ // CHECK-FIXES: P1[10] = 0;
+
+ 10[P2->X] = 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: confusing array subscript expression
+ // CHECK-FIXES: P2->X[10] = 0;
+
+ dostuff(1["abc"]);
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: confusing array subscript expression
+ // CHECK-FIXES: dostuff("abc"[1]);
+
+ dostuff(1[ABC]);
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: confusing array subscript expression
+ // CHECK-FIXES: dostuff(ABC[1]);
+
+ dostuff(0[0 + ABC]);
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: confusing array subscript expression
+ // CHECK-FIXES: dostuff(0[0 + ABC]);
+ // No fixit. Probably the code should be ABC[0]
+}
+
+void normalSyntax(int *X) {
+ X[10] = 0;
+}
Index: docs/clang-tidy/checks/readability-misplaced-array-index.rst
===
--- docs/clang-tidy/checks/readability-misplaced-array-index.rst
+++ docs/clang-tidy/checks/readability-misplaced-array-index.rst
@@ -0,0 +1,27 @@
+.. title:: clang-tidy - readability-misplaced-array-index
+
+readability-misplaced-array-index
+=
+
+This check warns for unusual array index syntax.
+
+The following code has unusual array index syntax:
+
+.. code-block:: c++
+
+ void f(int *X, int Y) {
+Y[X] = 0;
+ }
+
+becomes
+
+.. code-block:: c++
+
+ void f(int *X, int Y) {
+X[Y] = 0;
+ }
+
+The check warns about such unusual syntax for readability reasons:
+ * There are programmers that are not familiar with this unusual syntax.
+ * It is possible that variables are mixed up.
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -127,6 +127,7 @@
readability-identifier-naming
readability-implicit-bool-cast
readability-inconsistent-declaration-parameter-name
+ readability-misplaced-array-index
readability-named-parameter
readability-non-const-parameter
readability-redundant-control-flow
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -85,6 +85,11 @@
Warns about the performance overhead arising from concatenating strings using
the ``operator+``, instead of ``operator+=``.
+- New `readability-misplaced-array-index
+ `_ check
+
+ Warns when there is array index before the [] instead of inside it.
+
- New `readability-non-const-parameter
`_ check
Index: clang-tidy/readability/ReadabilityTidyModule.cpp
===
--- clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -19,6 +19,7 @@
#include "IdentifierNamingCheck.h"
#include "ImplicitBoolCastCheck.h"
#include "InconsistentDeclarationParameterNameCheck.h"
+#include "MisplacedArrayIndexCheck.h"
#include "NamedParameterCheck.h"
#include "NonConstParameterCheck.h"
#include "RedundantControlFlowCheck.h"
@@ -54,6 +55,8 @@
"readability-implicit-bool-cast");
CheckFactories.registerCheck(
"readability-inconsistent-declaration-parameter-name");
+CheckFactories.registerCheck(
+"readability-misplaced-array-index");
CheckFactories.registerCheck(
"readability-static-definition-in-anonymous-namespace");
CheckFactories.registerCheck(
Index: clang-tidy/readability/MisplacedArrayIndexCheck.h
===
--- clang-tidy/readability/MisplacedArrayIndexCheck.h
+++
Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index
alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.
A few nits.
Comment at: clang-tidy/readability/MisplacedArrayIndexCheck.h:19
@@ +18,3 @@
+
+/// Warn about unusual array index syntax (index[array] instead of
+/// array[index]).
Please enclose inline code snippets in backquotes.
Comment at: docs/clang-tidy/checks/readability-misplaced-array-index.rst:10
@@ +9,3 @@
+
+.. code:: c++
+
This should be `.. code-block:: c++`.
Comment at: docs/clang-tidy/checks/readability-misplaced-array-index.rst:13
@@ +12,3 @@
+ void f(int *x, int y) {
+y[x] = 0;
+ }
danielmarjamaki wrote:
> ok thanks same mistake I've done before. Should I start using uppercase
> variable names from now on?
Interesting question. Probably, better to do so.
https://reviews.llvm.org/D21134
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index
danielmarjamaki marked 6 inline comments as done.
Comment at: clang-tidy/readability/MisplacedArrayIndexCheck.cpp:57
@@ +56,2 @@
+} // namespace tidy
+} // namespace clang
I removed hasMacroId() and use fixit::getText(). The replacements look good now.
Comment at: docs/clang-tidy/checks/readability-misplaced-array-index.rst:13
@@ +12,3 @@
+ void f(int *x, int y) {
+y[x] = 0;
+ }
ok thanks same mistake I've done before. Should I start using uppercase
variable names from now on?
https://reviews.llvm.org/D21134
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index
danielmarjamaki updated this revision to Diff 69113.
danielmarjamaki added a comment.
fixed review comments
https://reviews.llvm.org/D21134
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/MisplacedArrayIndexCheck.cpp
clang-tidy/readability/MisplacedArrayIndexCheck.h
clang-tidy/readability/ReadabilityTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/readability-misplaced-array-index.rst
test/clang-tidy/readability-misplaced-array-index.cpp
Index: test/clang-tidy/readability-misplaced-array-index.cpp
===
--- test/clang-tidy/readability-misplaced-array-index.cpp
+++ test/clang-tidy/readability-misplaced-array-index.cpp
@@ -0,0 +1,36 @@
+// RUN: %check_clang_tidy %s readability-misplaced-array-index %t
+
+#define ABC "abc"
+
+struct XY { int *x; int *y; };
+
+void dostuff(int);
+
+void unusualSyntax(int *x, struct XY *xy)
+{
+ 10[x] = 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: confusing array subscript expression, usually the index is inside the []
+ // CHECK-FIXES: x[10] = 0;
+
+ 10[xy->x] = 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: confusing array subscript expression
+ // CHECK-FIXES: xy->x[10] = 0;
+
+ dostuff(1["abc"]);
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: confusing array subscript expression
+ // CHECK-FIXES: dostuff("abc"[1]);
+
+ dostuff(1[ABC]);
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: confusing array subscript expression
+ // CHECK-FIXES: dostuff(ABC[1]);
+
+ dostuff(0[0 + ABC]);
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: confusing array subscript expression
+ // CHECK-FIXES: dostuff(0[0 + ABC]);
+ // No fixit. Probably the code should be ABC[0]
+}
+
+void normalSyntax(int *x)
+{
+ x[10] = 0;
+}
Index: docs/clang-tidy/checks/readability-misplaced-array-index.rst
===
--- docs/clang-tidy/checks/readability-misplaced-array-index.rst
+++ docs/clang-tidy/checks/readability-misplaced-array-index.rst
@@ -0,0 +1,27 @@
+.. title:: clang-tidy - readability-misplaced-array-index
+
+readability-misplaced-array-index
+=
+
+This check warns for unusual array index syntax.
+
+The following code has unusual array index syntax:
+
+.. code:: c++
+
+ void f(int *x, int y) {
+y[x] = 0;
+ }
+
+becomes
+
+.. code:: c++
+
+ void f(int *x, int y) {
+x[y] = 0;
+ }
+
+The check warns about such unusual syntax for readability reasons:
+ * There are programmers that are not familiar with this unusual syntax.
+ * It is possible that variables are mixed up.
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -127,6 +127,7 @@
readability-identifier-naming
readability-implicit-bool-cast
readability-inconsistent-declaration-parameter-name
+ readability-misplaced-array-index
readability-named-parameter
readability-non-const-parameter
readability-redundant-control-flow
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -85,6 +85,11 @@
Warns about the performance overhead arising from concatenating strings using
the ``operator+``, instead of ``operator+=``.
+- New `readability-misplaced-array-index
+ `_ check
+
+ Warns when there is array index before the [] instead of inside it.
+
- New `readability-non-const-parameter
`_ check
Index: clang-tidy/readability/ReadabilityTidyModule.cpp
===
--- clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -19,6 +19,7 @@
#include "IdentifierNamingCheck.h"
#include "ImplicitBoolCastCheck.h"
#include "InconsistentDeclarationParameterNameCheck.h"
+#include "MisplacedArrayIndexCheck.h"
#include "NamedParameterCheck.h"
#include "NonConstParameterCheck.h"
#include "RedundantControlFlowCheck.h"
@@ -54,6 +55,8 @@
"readability-implicit-bool-cast");
CheckFactories.registerCheck(
"readability-inconsistent-declaration-parameter-name");
+CheckFactories.registerCheck(
+"readability-misplaced-array-index");
CheckFactories.registerCheck(
"readability-static-definition-in-anonymous-namespace");
CheckFactories.registerCheck(
Index: clang-tidy/readability/MisplacedArrayIndexCheck.h
===
--- clang-tidy/readability/MisplacedArrayIndexCheck.h
+++ clang-tidy/readability/MisplacedArrayIndexCheck.h
@@ -0,0 +1,36 @@
+//===---
Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index
alexfh requested changes to this revision.
This revision now requires changes to proceed.
Comment at: clang-tidy/readability/MisplacedArrayIndexCheck.cpp:56
@@ +55,3 @@
+ if (hasMacroID(ArraySubscriptE) ||
+
!Result.SourceManager->isWrittenInSameFile(ArraySubscriptE->getLocStart(),
+ ArraySubscriptE->getLocEnd()))
Both ranges seem to be using `ArraySubscriptE->getLHS()->getSourceRange()`. I
guess, the second one should refer to
`ArraySubscriptE->getRHS()->getSourceRange()` instead?
Comment at: docs/clang-tidy/checks/readability-misplaced-array-index.rst:12
@@ +11,3 @@
+
+ void f(int *x, int y)
+ {
Please format examples according to LLVM style (specifically, leave braces at
the end of the previous line.
Comment at: test/clang-tidy/readability-misplaced-array-index.cpp:16
@@ +15,3 @@
+ 10[xy->x] = 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: confusing array subscript
expression, usually the index is inside the []
+ // CHECK-FIXES: xy->x[10] = 0;
Please truncate all CHECK-MESSAGES except for the first one after "confusing
array subscript expression".
Comment at: test/clang-tidy/readability-misplaced-array-index.cpp:25
@@ +24,3 @@
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: confusing array subscript
expression, usually the index is inside the []
+ // No fixit, we don't want to replace with "abc"[1]
+
Please add a CHECK-FIXES to verify the original code is left intact. Same for
other places where no fixits are expected.
https://reviews.llvm.org/D21134
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index
danielmarjamaki marked an inline comment as done. danielmarjamaki added a comment. https://reviews.llvm.org/D21134 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index
danielmarjamaki updated this revision to Diff 67338.
danielmarjamaki added a comment.
ran clang-format
https://reviews.llvm.org/D21134
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/MisplacedArrayIndexCheck.cpp
clang-tidy/readability/MisplacedArrayIndexCheck.h
clang-tidy/readability/ReadabilityTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/readability-misplaced-array-index.rst
test/clang-tidy/readability-misplaced-array-index.cpp
Index: test/clang-tidy/readability-misplaced-array-index.cpp
===
--- test/clang-tidy/readability-misplaced-array-index.cpp
+++ test/clang-tidy/readability-misplaced-array-index.cpp
@@ -0,0 +1,35 @@
+// RUN: %check_clang_tidy %s readability-misplaced-array-index %t
+
+#define ABC "abc"
+
+struct XY { int *x; int *y; };
+
+void dostuff(int);
+
+void unusualSyntax(int *x, struct XY *xy)
+{
+ 10[x] = 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: confusing array subscript expression, usually the index is inside the []
+ // CHECK-FIXES: x[10] = 0;
+
+ 10[xy->x] = 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: confusing array subscript expression, usually the index is inside the []
+ // CHECK-FIXES: xy->x[10] = 0;
+
+ dostuff(1["abc"]);
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: confusing array subscript expression, usually the index is inside the []
+ // CHECK-FIXES: dostuff("abc"[1]);
+
+ dostuff(1[ABC]);
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: confusing array subscript expression, usually the index is inside the []
+ // No fixit, we don't want to replace with "abc"[1]
+
+ dostuff(0[0 + ABC]);
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: confusing array subscript expression, usually the index is inside the []
+ // No fixit. Probably the code should be ABC[0]
+}
+
+void normalSyntax(int *x)
+{
+ x[10] = 0;
+}
Index: docs/clang-tidy/checks/readability-misplaced-array-index.rst
===
--- docs/clang-tidy/checks/readability-misplaced-array-index.rst
+++ docs/clang-tidy/checks/readability-misplaced-array-index.rst
@@ -0,0 +1,29 @@
+.. title:: clang-tidy - readability-misplaced-array-index
+
+readability-misplaced-array-index
+=
+
+This check warns for unusual array index syntax.
+
+The following code has unusual array index syntax:
+
+.. code:: c++
+
+ void f(int *x, int y)
+ {
+y[x] = 0;
+ }
+
+becomes
+
+.. code:: c++
+
+ void f(int *x, int y)
+ {
+x[y] = 0;
+ }
+
+The check warns about such unusual syntax for readability reasons:
+ * There are programmers that are not familiar with this unusual syntax.
+ * It is possible that variables are mixed up.
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -126,6 +126,7 @@
readability-identifier-naming
readability-implicit-bool-cast
readability-inconsistent-declaration-parameter-name
+ readability-misplaced-array-index
readability-named-parameter
readability-redundant-control-flow
readability-redundant-smartptr-get
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -75,6 +75,10 @@
This check warns about the performance overhead arising from concatenating
strings using the ``operator+``, instead of ``operator+=``.
+- New `readability-misplaced-array-index
+ `_ check
+
+ Warns when there is array index before the [] instead of inside it.
Improvements to include-fixer
-
Index: clang-tidy/readability/ReadabilityTidyModule.cpp
===
--- clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -19,6 +19,7 @@
#include "IdentifierNamingCheck.h"
#include "ImplicitBoolCastCheck.h"
#include "InconsistentDeclarationParameterNameCheck.h"
+#include "MisplacedArrayIndexCheck.h"
#include "NamedParameterCheck.h"
#include "RedundantControlFlowCheck.h"
#include "RedundantSmartptrGetCheck.h"
@@ -53,6 +54,8 @@
"readability-implicit-bool-cast");
CheckFactories.registerCheck(
"readability-inconsistent-declaration-parameter-name");
+CheckFactories.registerCheck(
+"readability-misplaced-array-index");
CheckFactories.registerCheck(
"readability-static-definition-in-anonymous-namespace");
CheckFactories.registerCheck(
Index: clang-tidy/readability/MisplacedArrayIndexCheck.h
===
--- clang-tidy/readability/MisplacedArrayIndexCheck.h
+++
Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index
danielmarjamaki updated this revision to Diff 67337.
danielmarjamaki added a comment.
More generic diagnostic. Diagnose all integerType[pointerType] expressions.
https://reviews.llvm.org/D21134
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/MisplacedArrayIndexCheck.cpp
clang-tidy/readability/MisplacedArrayIndexCheck.h
clang-tidy/readability/ReadabilityTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/readability-misplaced-array-index.rst
test/clang-tidy/readability-misplaced-array-index.cpp
Index: test/clang-tidy/readability-misplaced-array-index.cpp
===
--- test/clang-tidy/readability-misplaced-array-index.cpp
+++ test/clang-tidy/readability-misplaced-array-index.cpp
@@ -0,0 +1,35 @@
+// RUN: %check_clang_tidy %s readability-misplaced-array-index %t
+
+#define ABC "abc"
+
+struct XY { int *x; int *y; };
+
+void dostuff(int);
+
+void unusualSyntax(int *x, struct XY *xy)
+{
+ 10[x] = 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: confusing array subscript expression, usually the index is inside the []
+ // CHECK-FIXES: x[10] = 0;
+
+ 10[xy->x] = 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: confusing array subscript expression, usually the index is inside the []
+ // CHECK-FIXES: xy->x[10] = 0;
+
+ dostuff(1["abc"]);
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: confusing array subscript expression, usually the index is inside the []
+ // CHECK-FIXES: dostuff("abc"[1]);
+
+ dostuff(1[ABC]);
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: confusing array subscript expression, usually the index is inside the []
+ // No fixit, we don't want to replace with "abc"[1]
+
+ dostuff(0[0 + ABC]);
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: confusing array subscript expression, usually the index is inside the []
+ // No fixit. Probably the code should be ABC[0]
+}
+
+void normalSyntax(int *x)
+{
+ x[10] = 0;
+}
Index: docs/clang-tidy/checks/readability-misplaced-array-index.rst
===
--- docs/clang-tidy/checks/readability-misplaced-array-index.rst
+++ docs/clang-tidy/checks/readability-misplaced-array-index.rst
@@ -0,0 +1,29 @@
+.. title:: clang-tidy - readability-misplaced-array-index
+
+readability-misplaced-array-index
+=
+
+This check warns for unusual array index syntax.
+
+The following code has unusual array index syntax:
+
+.. code:: c++
+
+ void f(int *x, int y)
+ {
+y[x] = 0;
+ }
+
+becomes
+
+.. code:: c++
+
+ void f(int *x, int y)
+ {
+x[y] = 0;
+ }
+
+The check warns about such unusual syntax for readability reasons:
+ * There are programmers that are not familiar with this unusual syntax.
+ * It is possible that variables are mixed up.
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -126,6 +126,7 @@
readability-identifier-naming
readability-implicit-bool-cast
readability-inconsistent-declaration-parameter-name
+ readability-misplaced-array-index
readability-named-parameter
readability-redundant-control-flow
readability-redundant-smartptr-get
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -75,6 +75,10 @@
This check warns about the performance overhead arising from concatenating
strings using the ``operator+``, instead of ``operator+=``.
+- New `readability-misplaced-array-index
+ `_ check
+
+ Warns when there is array index before the [] instead of inside it.
Improvements to include-fixer
-
Index: clang-tidy/readability/ReadabilityTidyModule.cpp
===
--- clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -19,6 +19,7 @@
#include "IdentifierNamingCheck.h"
#include "ImplicitBoolCastCheck.h"
#include "InconsistentDeclarationParameterNameCheck.h"
+#include "MisplacedArrayIndexCheck.h"
#include "NamedParameterCheck.h"
#include "RedundantControlFlowCheck.h"
#include "RedundantSmartptrGetCheck.h"
@@ -53,6 +54,8 @@
"readability-implicit-bool-cast");
CheckFactories.registerCheck(
"readability-inconsistent-declaration-parameter-name");
+CheckFactories.registerCheck(
+"readability-misplaced-array-index");
CheckFactories.registerCheck(
"readability-static-definition-in-anonymous-namespace");
CheckFactories.registerCheck(
Index: clang-tidy/readability/MisplacedArrayIndexCheck.h
===
---
Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index
jroelofs added a comment. In https://reviews.llvm.org/D21134#509522, @danielmarjamaki wrote: > In https://reviews.llvm.org/D21134#508524, @jroelofs wrote: > > > I think the replacement is wrong for something like: > > > > int *arr; int offs1, offs2; > > offs1[arr + offs2] = 42; > > > > > > which I think would give: > > > > int *arr; int offs1, offs2; > > arr + offs2[offs1] = 42; > > > > > > If the precedence of the "indexing" argument is less than that of the > > subscript operator, then you need to insert parens: > > > > int *arr; int offs1, offs2; > > (arr + offs2)[offs1] = 42; > > > > > I don't think so. The matcher says: > > hasRHS(ignoringParenImpCasts( > anyOf(stringLiteral(), declRefExpr(hasType(pointsTo(qualType(, > memberExpr(hasType(pointsTo(qualType())) > > > I think it's hard to know what the replaced code should be. In some cases it > might be better with: > > arr[offs1 + offs2] = 42; > Oh, yeah, you're right. https://reviews.llvm.org/D21134 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index
danielmarjamaki marked 2 inline comments as done. Comment at: test/clang-tidy/readability-misplaced-array-index.cpp:31 @@ +30,3 @@ + x[10] = 0; + dostuff(0[0 + ABC]); +} aaron.ballman wrote: > Why is this considered to be "normal syntax" and not worth getting a > diagnostic? hmm.. I agree that it would be good to have a diagnostic for this code also. I currently only diagnose when subscript is stringLiteral, declRefExpr or memberExpr. These are the cases when the code can be easily fixed by just replacing the expressions. I will look into writing a diagnostic for this also. https://reviews.llvm.org/D21134 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index
aaron.ballman added inline comments. Comment at: test/clang-tidy/readability-misplaced-array-index.cpp:31 @@ +30,3 @@ + x[10] = 0; + dostuff(0[0 + ABC]); +} Why is this considered to be "normal syntax" and not worth getting a diagnostic? https://reviews.llvm.org/D21134 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index
danielmarjamaki marked 4 inline comments as done.
danielmarjamaki added a comment.
as far as I see the only unsolved review comment now is about macros. if it can
be handled better.
Comment at: clang-tidy/readability/MisplacedArrayIndexCheck.cpp:28-29
@@ +27,4 @@
+ anyOf(stringLiteral(), declRefExpr(hasType(pointerType())),
+memberExpr(hasType(pointerType()))
+ .bind("expr"),
+ this);
Yes thanks!
Comment at: clang-tidy/readability/MisplacedArrayIndexCheck.cpp:53
@@ +52,3 @@
+ if (hasMacroID(ArraySubscriptE) ||
+
!Result.SourceManager->isWrittenInSameFile(ArraySubscriptE->getLocStart(),
+ ArraySubscriptE->getLocEnd()))
you can do lots of weird things with macros. so I wanted to just diagnose and
then have no fixits that would be wrong etc. I removed the comment since I
think the code is pretty clear.
Comment at: clang-tidy/readability/MisplacedArrayIndexCheck.cpp:55
@@ +54,3 @@
+ ArraySubscriptE->getLocEnd()))
+return;
+
> What exactly is the recursive hasMacroID function trying to prevent? Do you
> have a test that breaks if you just check that the start location of the
> expression is not a macro?
I am worried about macros anywhere. I did not want to apply fixes for this
code: 0[ABC] but maybe using the Lexer would make that safe.
> In most cases, it's enough to check that the whole range is on the same macro
> expansion level using Lexer::makeFileCharRange in order to prevent most
> problems with macros, while allowing fixes in safe cases, e.g. replacements
> in parameters of EXPECT_* macros from gtest.
I was very unsuccessful with that. I must do something wrong...
```
CharSourceRange LRange = Lexer::makeFileCharRange(
CharSourceRange::getCharRange(
ArraySubscriptE->getLHS()->getSourceRange()),
Result.Context->getSourceManager(), Result.Context->getLangOpts());
CharSourceRange RRange = Lexer::makeFileCharRange(
CharSourceRange::getCharRange(
ArraySubscriptE->getLHS()->getSourceRange()),
Result.Context->getSourceManager(), Result.Context->getLangOpts());
StringRef LText = Lexer::getSourceText(LRange, *Result.SourceManager,
Result.Context->getLangOpts());
StringRef RText = Lexer::getSourceText(RRange, *Result.SourceManager,
Result.Context->getLangOpts());
Diag <<
FixItHint::CreateReplacement(ArraySubscriptE->getLHS()->getSourceRange(),
RText);
Diag <<
FixItHint::CreateReplacement(ArraySubscriptE->getRHS()->getSourceRange(),
LText);
```
No fixits worked with that code, with or without macros.
Do you see what I did wrong?
https://reviews.llvm.org/D21134
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index
danielmarjamaki updated this revision to Diff 67322.
danielmarjamaki added a comment.
take care of review comments
https://reviews.llvm.org/D21134
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/MisplacedArrayIndexCheck.cpp
clang-tidy/readability/MisplacedArrayIndexCheck.h
clang-tidy/readability/ReadabilityTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/readability-misplaced-array-index.rst
test/clang-tidy/readability-misplaced-array-index.cpp
Index: test/clang-tidy/readability-misplaced-array-index.cpp
===
--- test/clang-tidy/readability-misplaced-array-index.cpp
+++ test/clang-tidy/readability-misplaced-array-index.cpp
@@ -0,0 +1,32 @@
+// RUN: %check_clang_tidy %s readability-misplaced-array-index %t
+
+#define ABC "abc"
+
+struct XY { int *x; int *y; };
+
+void dostuff(int);
+
+void unusualSyntax(int *x, struct XY *xy)
+{
+ 10[x] = 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: confusing array subscript expression, usually the index is inside the []
+ // CHECK-FIXES: x[10] = 0;
+
+ 10[xy->x] = 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: confusing array subscript expression, usually the index is inside the []
+ // CHECK-FIXES: xy->x[10] = 0;
+
+ dostuff(1["abc"]);
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: confusing array subscript expression, usually the index is inside the []
+ // CHECK-FIXES: dostuff("abc"[1]);
+
+ dostuff(1[ABC]);
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: confusing array subscript expression, usually the index is inside the []
+ // No fixit, we don't want to replace with "abc"[1]
+}
+
+void normalSyntax(int *x)
+{
+ x[10] = 0;
+ dostuff(0[0 + ABC]);
+}
Index: docs/clang-tidy/checks/readability-misplaced-array-index.rst
===
--- docs/clang-tidy/checks/readability-misplaced-array-index.rst
+++ docs/clang-tidy/checks/readability-misplaced-array-index.rst
@@ -0,0 +1,29 @@
+.. title:: clang-tidy - readability-misplaced-array-index
+
+readability-misplaced-array-index
+=
+
+This check warns for unusual array index syntax.
+
+The following code has unusual array index syntax:
+
+.. code:: c++
+
+ void f(int *x, int y)
+ {
+y[x] = 0;
+ }
+
+becomes
+
+.. code:: c++
+
+ void f(int *x, int y)
+ {
+x[y] = 0;
+ }
+
+The check warns about such unusual syntax for readability reasons:
+ * There are programmers that are not familiar with this unusual syntax.
+ * It is possible that variables are mixed up.
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -126,6 +126,7 @@
readability-identifier-naming
readability-implicit-bool-cast
readability-inconsistent-declaration-parameter-name
+ readability-misplaced-array-index
readability-named-parameter
readability-redundant-control-flow
readability-redundant-smartptr-get
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -75,6 +75,10 @@
This check warns about the performance overhead arising from concatenating
strings using the ``operator+``, instead of ``operator+=``.
+- New `readability-misplaced-array-index
+ `_ check
+
+ Warns when there is array index before the [] instead of inside it.
Improvements to include-fixer
-
Index: clang-tidy/readability/ReadabilityTidyModule.cpp
===
--- clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -19,6 +19,7 @@
#include "IdentifierNamingCheck.h"
#include "ImplicitBoolCastCheck.h"
#include "InconsistentDeclarationParameterNameCheck.h"
+#include "MisplacedArrayIndexCheck.h"
#include "NamedParameterCheck.h"
#include "RedundantControlFlowCheck.h"
#include "RedundantSmartptrGetCheck.h"
@@ -53,6 +54,8 @@
"readability-implicit-bool-cast");
CheckFactories.registerCheck(
"readability-inconsistent-declaration-parameter-name");
+CheckFactories.registerCheck(
+"readability-misplaced-array-index");
CheckFactories.registerCheck(
"readability-static-definition-in-anonymous-namespace");
CheckFactories.registerCheck(
Index: clang-tidy/readability/MisplacedArrayIndexCheck.h
===
--- clang-tidy/readability/MisplacedArrayIndexCheck.h
+++ clang-tidy/readability/MisplacedArrayIndexCheck.h
@@ -0,0 +1,36 @@
+//===--- MisplacedArrayIndexCheck.h - clang-tidy-*- C++ -*-===//
+//
+// The LLVM Compiler
Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index
danielmarjamaki added a comment. In https://reviews.llvm.org/D21134#508524, @jroelofs wrote: > I think the replacement is wrong for something like: > > int *arr; int offs1, offs2; > offs1[arr + offs2] = 42; > > > which I think would give: > > int *arr; int offs1, offs2; > arr + offs2[offs1] = 42; > > > If the precedence of the "indexing" argument is less than that of the > subscript operator, then you need to insert parens: > > int *arr; int offs1, offs2; > (arr + offs2)[offs1] = 42; > I don't think so. The matcher says: hasRHS(ignoringParenImpCasts( anyOf(stringLiteral(), declRefExpr(hasType(pointsTo(qualType(, memberExpr(hasType(pointsTo(qualType())) I think it's hard to know what the replaced code should be. In some cases it might be better with: arr[offs1 + offs2] = 42; https://reviews.llvm.org/D21134 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index
danielmarjamaki marked an inline comment as done.
danielmarjamaki added a comment.
In https://reviews.llvm.org/D21134#508511, @aaron.ballman wrote:
> Is there a reason we don't want this check to be a part of the clang
> frontend, rather than as a clang-tidy check?
I discussed this with frontend and clang-tidy people in the llvm meeting last
year and we came to the conclusion it was better in clang-tidy.
I don't remember the exact reasons.
But I think that this code is just "weird". It could be a bug but I write the
warning mostly for readability reasons.
Comment at: clang-tidy/readability/MisplacedArrayIndexCheck.cpp:43
@@ +42,3 @@
+
+static StringRef getAsString(const MatchFinder::MatchResult ,
+ const Expr *E) {
danielmarjamaki wrote:
> alexfh wrote:
> > Looks like this repeats getText from clang/Tooling/FixIt.h.
> Yes indeed..
this is fixed. As you suggested I used createReplacement() in the fixit. It's
much nicer!
https://reviews.llvm.org/D21134
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index
aaron.ballman added inline comments. Comment at: clang-tidy/readability/MisplacedArrayIndexCheck.cpp:48 @@ +47,3 @@ + + auto D = + diag(ArraySubscriptE->getLocStart(), alexfh wrote: > aaron.ballman wrote: > > Should not use `auto` here because the type is not spelled out in the > > initialization. > While in general I agree with this recommendation, `auto Diag = diag(...);` > has almost become an idiom in this code. I'd not worry about obscurity of > `auto` in this context, especially, if the variable is renamed to `Diag`, > which will make its meaning and possible ways of using it clearer. I'm okay with that approach. https://reviews.llvm.org/D21134 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index
alexfh added inline comments. Comment at: clang-tidy/readability/MisplacedArrayIndexCheck.cpp:48 @@ +47,3 @@ + + auto D = + diag(ArraySubscriptE->getLocStart(), aaron.ballman wrote: > Should not use `auto` here because the type is not spelled out in the > initialization. While in general I agree with this recommendation, `auto Diag = diag(...);` has almost become an idiom in this code. I'd not worry about obscurity of `auto` in this context, especially, if the variable is renamed to `Diag`, which will make its meaning and possible ways of using it clearer. https://reviews.llvm.org/D21134 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index
alexfh added a comment. In https://reviews.llvm.org/D21134#508524, @jroelofs wrote: > I think the replacement is wrong for something like: > > int *arr; int offs1, offs2; > offs1[arr + offs2] = 42; > > > which I think would give: > > int *arr; int offs1, offs2; > arr + offs2[offs1] = 42; > > > If the precedence of the "indexing" argument is less than that of the > subscript operator, then you need to insert parens: > > int *arr; int offs1, offs2; > (arr + offs2)[offs1] = 42; > Indeed. It might be relevant to future (current ones seem to be fine) uses of `tooling::fixit::createReplacement` as well, so should probably be taken care of in its implementation. Maybe an additional hint from the calling code is needed (`bool InsertParensIfNeeded = false`). https://reviews.llvm.org/D21134 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index
aaron.ballman added a subscriber: aaron.ballman.
aaron.ballman added a reviewer: aaron.ballman.
aaron.ballman added a comment.
Is there a reason we don't want this check to be a part of the clang frontend,
rather than as a clang-tidy check?
Comment at: clang-tidy/readability/MisplacedArrayIndexCheck.cpp:27-28
@@ +26,4 @@
+ hasRHS(ignoringParenImpCasts(
+ anyOf(stringLiteral(),
declRefExpr(hasType(pointsTo(qualType(,
+memberExpr(hasType(pointsTo(qualType(
+ .bind("expr"),
Can this use `hasType(pointerType())` instead of
`hasType(pointsTo(qualType()))` ?
Comment at: clang-tidy/readability/MisplacedArrayIndexCheck.cpp:48
@@ +47,3 @@
+
+ auto D =
+ diag(ArraySubscriptE->getLocStart(),
Should not use `auto` here because the type is not spelled out in the
initialization.
Comment at: clang-tidy/readability/MisplacedArrayIndexCheck.cpp:50
@@ +49,3 @@
+ diag(ArraySubscriptE->getLocStart(),
+ "unusual array index syntax, usually the index is inside the []");
+
alexfh wrote:
> I'd say "confusing" instead of "unusual". This would also help avoiding the
> repetition ("unusual ..., usually ..."):
>
> "confusing array index syntax; usually, the index is inside the brackets"
Instead of "array index syntax", perhaps "array subscript expression"?
Comment at: clang-tidy/readability/MisplacedArrayIndexCheck.cpp:52
@@ +51,3 @@
+
+ // Don't even try to resolve macro or include contraptions. Not worth
emitting
+ // a fixit for.
What does "contraptions" mean in this context?
https://reviews.llvm.org/D21134
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index
alexfh added a comment.
Thanks, that's better. Still a couple of comments.
Comment at: clang-tidy/readability/MisplacedArrayIndexCheck.cpp:50
@@ +49,3 @@
+ diag(ArraySubscriptE->getLocStart(),
+ "unusual array index syntax, usually the index is inside the []");
+
I'd say "confusing" instead of "unusual". This would also help avoiding the
repetition ("unusual ..., usually ..."):
"confusing array index syntax; usually, the index is inside the brackets"
Comment at: clang-tidy/readability/MisplacedArrayIndexCheck.cpp:54
@@ +53,3 @@
+ // a fixit for.
+ if (hasMacroID(ArraySubscriptE) ||
+
!Result.SourceManager->isWrittenInSameFile(ArraySubscriptE->getLocStart(),
What exactly is the recursive `hasMacroID` function trying to prevent? Do you
have a test that breaks if you just check that the start location of the
expression is not a macro?
In most cases, it's enough to check that the whole range is on the same macro
expansion level using `Lexer::makeFileCharRange` in order to prevent most
problems with macros, while allowing fixes in safe cases, e.g. replacements in
parameters of `EXPECT_*` macros from gtest.
https://reviews.llvm.org/D21134
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index
danielmarjamaki removed rL LLVM as the repository for this revision.
danielmarjamaki updated this revision to Diff 67145.
danielmarjamaki added a comment.
Cleanup my previous commit. Ran clang-format.
https://reviews.llvm.org/D21134
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/MisplacedArrayIndexCheck.cpp
clang-tidy/readability/MisplacedArrayIndexCheck.h
clang-tidy/readability/ReadabilityTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/readability-misplaced-array-index.rst
test/clang-tidy/readability-misplaced-array-index.cpp
Index: test/clang-tidy/readability-misplaced-array-index.cpp
===
--- test/clang-tidy/readability-misplaced-array-index.cpp
+++ test/clang-tidy/readability-misplaced-array-index.cpp
@@ -0,0 +1,32 @@
+// RUN: %check_clang_tidy %s readability-misplaced-array-index %t
+
+#define ABC "abc"
+
+struct XY { int *x; int *y; };
+
+void dostuff(int);
+
+void unusualSyntax(int *x, struct XY *xy)
+{
+ 10[x] = 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: unusual array index syntax, usually the index is inside the []
+ // CHECK-FIXES: x[10] = 0;
+
+ 10[xy->x] = 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: unusual array index syntax, usually the index is inside the []
+ // CHECK-FIXES: xy->x[10] = 0;
+
+ dostuff(1["abc"]);
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: unusual array index syntax, usually the index is inside the []
+ // CHECK-FIXES: dostuff("abc"[1]);
+
+ dostuff(1[ABC]);
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: unusual array index syntax, usually the index is inside the []
+ // No fixit, we don't want to replace with "abc"[1]
+}
+
+void normalSyntax(int *x)
+{
+ x[10] = 0;
+ dostuff(0[0 + ABC]);
+}
Index: docs/clang-tidy/checks/readability-misplaced-array-index.rst
===
--- docs/clang-tidy/checks/readability-misplaced-array-index.rst
+++ docs/clang-tidy/checks/readability-misplaced-array-index.rst
@@ -0,0 +1,29 @@
+.. title:: clang-tidy - readability-misplaced-array-index
+
+readability-misplaced-array-index
+=
+
+This check warns for unusual array index syntax.
+
+The following code has unusual array index syntax:
+
+.. code:: c++
+
+ void f(int *x, int y)
+ {
+y[x] = 0;
+ }
+
+becomes
+
+.. code:: c++
+
+ void f(int *x, int y)
+ {
+x[y] = 0;
+ }
+
+The check warns about such unusual syntax for readability reasons:
+ * There are programmers that are not familiar with this unusual syntax.
+ * It is possible that variables are mixed up.
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -126,6 +126,7 @@
readability-identifier-naming
readability-implicit-bool-cast
readability-inconsistent-declaration-parameter-name
+ readability-misplaced-array-index
readability-named-parameter
readability-redundant-control-flow
readability-redundant-smartptr-get
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -75,6 +75,10 @@
This check warns about the performance overhead arising from concatenating
strings using the ``operator+``, instead of ``operator+=``.
+- New `readability-misplaced-array-index
+ `_ check
+
+ Warns when there is array index before the [] instead of inside it.
Improvements to include-fixer
-
Index: clang-tidy/readability/ReadabilityTidyModule.cpp
===
--- clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -19,6 +19,7 @@
#include "IdentifierNamingCheck.h"
#include "ImplicitBoolCastCheck.h"
#include "InconsistentDeclarationParameterNameCheck.h"
+#include "MisplacedArrayIndexCheck.h"
#include "NamedParameterCheck.h"
#include "RedundantControlFlowCheck.h"
#include "RedundantSmartptrGetCheck.h"
@@ -53,6 +54,8 @@
"readability-implicit-bool-cast");
CheckFactories.registerCheck(
"readability-inconsistent-declaration-parameter-name");
+CheckFactories.registerCheck(
+"readability-misplaced-array-index");
CheckFactories.registerCheck(
"readability-static-definition-in-anonymous-namespace");
CheckFactories.registerCheck(
Index: clang-tidy/readability/MisplacedArrayIndexCheck.h
===
--- clang-tidy/readability/MisplacedArrayIndexCheck.h
+++ clang-tidy/readability/MisplacedArrayIndexCheck.h
@@ -0,0 +1,36 @@
+//===--- MisplacedArrayIndexCheck.h - clang-tidy-*- C++ -*-===//
+//
Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index
danielmarjamaki set the repository for this revision to rL LLVM.
danielmarjamaki updated this revision to Diff 67144.
danielmarjamaki added a comment.
Fixed review comments.
Repository:
rL LLVM
https://reviews.llvm.org/D21134
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/MisplacedArrayIndexCheck.cpp
clang-tidy/readability/MisplacedArrayIndexCheck.h
clang-tidy/readability/ReadabilityTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/readability-misplaced-array-index.rst
test/clang-tidy/readability-misplaced-array-index.cpp
Index: test/clang-tidy/readability-misplaced-array-index.cpp
===
--- test/clang-tidy/readability-misplaced-array-index.cpp
+++ test/clang-tidy/readability-misplaced-array-index.cpp
@@ -0,0 +1,32 @@
+// RUN: %check_clang_tidy %s readability-misplaced-array-index %t
+
+#define ABC "abc"
+
+struct XY { int *x; int *y; };
+
+void dostuff(int);
+
+void unusualSyntax(int *x, struct XY *xy)
+{
+ 10[x] = 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: unusual array index syntax, usually the index is inside the []
+ // CHECK-FIXES: x[10] = 0;
+
+ 10[xy->x] = 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: unusual array index syntax, usually the index is inside the []
+ // CHECK-FIXES: xy->x[10] = 0;
+
+ dostuff(1["abc"]);
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: unusual array index syntax, usually the index is inside the []
+ // CHECK-FIXES: dostuff("abc"[1]);
+
+ dostuff(1[ABC]);
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: unusual array index syntax, usually the index is inside the []
+ // No fixit, we don't want to replace with "abc"[1]
+}
+
+void normalSyntax(int *x)
+{
+ x[10] = 0;
+ dostuff(0[0 + ABC]);
+}
Index: docs/clang-tidy/checks/readability-misplaced-array-index.rst
===
--- docs/clang-tidy/checks/readability-misplaced-array-index.rst
+++ docs/clang-tidy/checks/readability-misplaced-array-index.rst
@@ -0,0 +1,29 @@
+.. title:: clang-tidy - readability-misplaced-array-index
+
+readability-misplaced-array-index
+=
+
+This check warns for unusual array index syntax.
+
+The following code has unusual array index syntax:
+
+.. code:: c++
+
+ void f(int *x, int y)
+ {
+y[x] = 0;
+ }
+
+becomes
+
+.. code:: c++
+
+ void f(int *x, int y)
+ {
+x[y] = 0;
+ }
+
+The check warns about such unusual syntax for readability reasons:
+ * There are programmers that are not familiar with this unusual syntax.
+ * It is possible that variables are mixed up.
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -126,6 +126,7 @@
readability-identifier-naming
readability-implicit-bool-cast
readability-inconsistent-declaration-parameter-name
+ readability-misplaced-array-index
readability-named-parameter
readability-redundant-control-flow
readability-redundant-smartptr-get
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -75,6 +75,10 @@
This check warns about the performance overhead arising from concatenating
strings using the ``operator+``, instead of ``operator+=``.
+- New `readability-misplaced-array-index
+ `_ check
+
+ Warns when there is array index before the [] instead of inside it.
Improvements to include-fixer
-
Index: clang-tidy/readability/ReadabilityTidyModule.cpp
===
--- clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -19,6 +19,7 @@
#include "IdentifierNamingCheck.h"
#include "ImplicitBoolCastCheck.h"
#include "InconsistentDeclarationParameterNameCheck.h"
+#include "MisplacedArrayIndexCheck.h"
#include "NamedParameterCheck.h"
#include "RedundantControlFlowCheck.h"
#include "RedundantSmartptrGetCheck.h"
@@ -53,6 +54,8 @@
"readability-implicit-bool-cast");
CheckFactories.registerCheck(
"readability-inconsistent-declaration-parameter-name");
+CheckFactories.registerCheck(
+"readability-misplaced-array-index");
CheckFactories.registerCheck(
"readability-static-definition-in-anonymous-namespace");
CheckFactories.registerCheck(
Index: clang-tidy/readability/MisplacedArrayIndexCheck.h
===
--- clang-tidy/readability/MisplacedArrayIndexCheck.h
+++ clang-tidy/readability/MisplacedArrayIndexCheck.h
@@ -0,0 +1,36 @@
+//===--- MisplacedArrayIndexCheck.h - clang-tidy-*- C++ -*-===//
+//
+//
Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index
danielmarjamaki added inline comments.
Comment at: clang-tidy/readability/MisplacedArrayIndexCheck.cpp:43
@@ +42,3 @@
+
+static StringRef getAsString(const MatchFinder::MatchResult ,
+ const Expr *E) {
alexfh wrote:
> Looks like this repeats getText from clang/Tooling/FixIt.h.
Yes indeed..
https://reviews.llvm.org/D21134
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index
alexfh added a comment.
The check seems reasonable, I'm surprised there's no warning in Clang that
catches index[array] syntax ;)
A few comments re: implementation.
Comment at: clang-tidy/readability/MisplacedArrayIndexCheck.cpp:43
@@ +42,3 @@
+
+static StringRef getAsString(const MatchFinder::MatchResult ,
+ const Expr *E) {
Looks like this repeats getText from clang/Tooling/FixIt.h.
Comment at: clang-tidy/readability/MisplacedArrayIndexCheck.cpp:73
@@ +72,3 @@
+
+ D <<
FixItHint::CreateReplacement(ArraySubscriptE->getLHS()->getSourceRange(),
+RHSString);
Looks like you could use createReplacement and/or getText from
clang/Tooling/FixIt.h.
http://reviews.llvm.org/D21134
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index
danielmarjamaki updated this revision to Diff 60804.
danielmarjamaki added a comment.
fixed typo in releasenotes
http://reviews.llvm.org/D21134
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/MisplacedArrayIndexCheck.cpp
clang-tidy/readability/MisplacedArrayIndexCheck.h
clang-tidy/readability/ReadabilityTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/readability-misplaced-array-index.rst
test/clang-tidy/readability-misplaced-array-index.cpp
Index: test/clang-tidy/readability-misplaced-array-index.cpp
===
--- test/clang-tidy/readability-misplaced-array-index.cpp
+++ test/clang-tidy/readability-misplaced-array-index.cpp
@@ -0,0 +1,32 @@
+// RUN: %check_clang_tidy %s readability-misplaced-array-index %t
+
+#define ABC "abc"
+
+struct XY { int *x; int *y; };
+
+void dostuff(int);
+
+void unusualSyntax(int *x, struct XY *xy)
+{
+ 10[x] = 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: unusual array index syntax, usually the index is inside the []
+ // CHECK-FIXES: x[10] = 0;
+
+ 10[xy->x] = 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: unusual array index syntax, usually the index is inside the []
+ // CHECK-FIXES: xy->x[10] = 0;
+
+ dostuff(1["abc"]);
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: unusual array index syntax, usually the index is inside the []
+ // CHECK-FIXES: dostuff("abc"[1]);
+
+ dostuff(1[ABC]);
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: unusual array index syntax, usually the index is inside the []
+ // No fixit, we don't want to replace with "abc"[1]
+}
+
+void normalSyntax(int *x)
+{
+ x[10] = 0;
+ dostuff(0[0 + ABC]);
+}
Index: docs/clang-tidy/checks/readability-misplaced-array-index.rst
===
--- docs/clang-tidy/checks/readability-misplaced-array-index.rst
+++ docs/clang-tidy/checks/readability-misplaced-array-index.rst
@@ -0,0 +1,29 @@
+.. title:: clang-tidy - readability-misplaced-array-index
+
+readability-misplaced-array-index
+=
+
+This check warns for unusual array index syntax.
+
+The following code has unusual array index syntax:
+
+.. code:: c++
+
+ void f(int *x, int y)
+ {
+y[x] = 0;
+ }
+
+becomes
+
+.. code:: c++
+
+ void f(int *x, int y)
+ {
+x[y] = 0;
+ }
+
+The check warns about such unusual syntax for readability reasons:
+ * There are programmers that are not familiar with this unusual syntax.
+ * It is possible that variables are mixed up.
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -120,6 +120,7 @@
readability-identifier-naming
readability-implicit-bool-cast
readability-inconsistent-declaration-parameter-name
+ readability-misplaced-array-index
readability-named-parameter
readability-redundant-control-flow
readability-redundant-smartptr-get
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -249,6 +249,11 @@
Warns about defaulted constructors and assignment operators that are actually
deleted.
+- New `readability-misplaced-array-index
+ `_ check
+
+ Warns when there is array index before the [] instead of inside it.
+
- New `readability-redundant-control-flow
`_ check
Index: clang-tidy/readability/ReadabilityTidyModule.cpp
===
--- clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -19,6 +19,7 @@
#include "IdentifierNamingCheck.h"
#include "ImplicitBoolCastCheck.h"
#include "InconsistentDeclarationParameterNameCheck.h"
+#include "MisplacedArrayIndexCheck.h"
#include "NamedParameterCheck.h"
#include "RedundantControlFlowCheck.h"
#include "RedundantSmartptrGetCheck.h"
@@ -53,6 +54,8 @@
"readability-implicit-bool-cast");
CheckFactories.registerCheck(
"readability-inconsistent-declaration-parameter-name");
+CheckFactories.registerCheck(
+"readability-misplaced-array-index");
CheckFactories.registerCheck(
"readability-static-definition-in-anonymous-namespace");
CheckFactories.registerCheck(
Index: clang-tidy/readability/MisplacedArrayIndexCheck.h
===
--- clang-tidy/readability/MisplacedArrayIndexCheck.h
+++ clang-tidy/readability/MisplacedArrayIndexCheck.h
@@ -0,0 +1,36 @@
+//===--- MisplacedArrayIndexCheck.h - clang-tidy-*- C++ -*-===//
+//
+// The LLVM Compiler
Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index
danielmarjamaki removed rL LLVM as the repository for this revision.
danielmarjamaki updated this revision to Diff 60803.
danielmarjamaki added a comment.
updated releasedocs
deeper lookup if macro is used
only warn when the replacement can be done safely. the expressions "x[y+z]" and
y+z[x]" are not the same.
http://reviews.llvm.org/D21134
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/MisplacedArrayIndexCheck.cpp
clang-tidy/readability/MisplacedArrayIndexCheck.h
clang-tidy/readability/ReadabilityTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/readability-misplaced-array-index.rst
test/clang-tidy/readability-misplaced-array-index.cpp
Index: test/clang-tidy/readability-misplaced-array-index.cpp
===
--- test/clang-tidy/readability-misplaced-array-index.cpp
+++ test/clang-tidy/readability-misplaced-array-index.cpp
@@ -0,0 +1,32 @@
+// RUN: %check_clang_tidy %s readability-misplaced-array-index %t
+
+#define ABC "abc"
+
+struct XY { int *x; int *y; };
+
+void dostuff(int);
+
+void unusualSyntax(int *x, struct XY *xy)
+{
+ 10[x] = 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: unusual array index syntax, usually the index is inside the []
+ // CHECK-FIXES: x[10] = 0;
+
+ 10[xy->x] = 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: unusual array index syntax, usually the index is inside the []
+ // CHECK-FIXES: xy->x[10] = 0;
+
+ dostuff(1["abc"]);
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: unusual array index syntax, usually the index is inside the []
+ // CHECK-FIXES: dostuff("abc"[1]);
+
+ dostuff(1[ABC]);
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: unusual array index syntax, usually the index is inside the []
+ // No fixit, we don't want to replace with "abc"[1]
+}
+
+void normalSyntax(int *x)
+{
+ x[10] = 0;
+ dostuff(0[0 + ABC]);
+}
Index: docs/clang-tidy/checks/readability-misplaced-array-index.rst
===
--- docs/clang-tidy/checks/readability-misplaced-array-index.rst
+++ docs/clang-tidy/checks/readability-misplaced-array-index.rst
@@ -0,0 +1,29 @@
+.. title:: clang-tidy - readability-misplaced-array-index
+
+readability-misplaced-array-index
+=
+
+This check warns for unusual array index syntax.
+
+The following code has unusual array index syntax:
+
+.. code:: c++
+
+ void f(int *x, int y)
+ {
+y[x] = 0;
+ }
+
+becomes
+
+.. code:: c++
+
+ void f(int *x, int y)
+ {
+x[y] = 0;
+ }
+
+The check warns about such unusual syntax for readability reasons:
+ * There are programmers that are not familiar with this unusual syntax.
+ * It is possible that variables are mixed up.
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -120,6 +120,7 @@
readability-identifier-naming
readability-implicit-bool-cast
readability-inconsistent-declaration-parameter-name
+ readability-misplaced-array-index
readability-named-parameter
readability-redundant-control-flow
readability-redundant-smartptr-get
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -249,6 +249,11 @@
Warns about defaulted constructors and assignment operators that are actually
deleted.
+- New `misplaced-array-index
+ `_ check
+
+ Warns when there is array index before the [] instead of inside it.
+
- New `readability-redundant-control-flow
`_ check
Index: clang-tidy/readability/ReadabilityTidyModule.cpp
===
--- clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -19,6 +19,7 @@
#include "IdentifierNamingCheck.h"
#include "ImplicitBoolCastCheck.h"
#include "InconsistentDeclarationParameterNameCheck.h"
+#include "MisplacedArrayIndexCheck.h"
#include "NamedParameterCheck.h"
#include "RedundantControlFlowCheck.h"
#include "RedundantSmartptrGetCheck.h"
@@ -53,6 +54,8 @@
"readability-implicit-bool-cast");
CheckFactories.registerCheck(
"readability-inconsistent-declaration-parameter-name");
+CheckFactories.registerCheck(
+"readability-misplaced-array-index");
CheckFactories.registerCheck(
"readability-static-definition-in-anonymous-namespace");
CheckFactories.registerCheck(
Index: clang-tidy/readability/MisplacedArrayIndexCheck.h
===
--- clang-tidy/readability/MisplacedArrayIndexCheck.h
+++
Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index
Eugene.Zelenko added a subscriber: Eugene.Zelenko. Eugene.Zelenko added a comment. Please mention this check in docs/ReleaseNotes.rst (in alphabetical order). Repository: rL LLVM http://reviews.llvm.org/D21134 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D21134: clang-tidy: new check readability-misplaced-array-index
danielmarjamaki created this revision.
danielmarjamaki added a reviewer: alexfh.
danielmarjamaki added a subscriber: cfe-commits.
danielmarjamaki set the repository for this revision to rL LLVM.
this is a new check for clang-tidy that diagnoses when it see unusual array
index syntax.
there is nothing wrong about such syntax so it's not a misc check. It's just a
readability check.
Repository:
rL LLVM
http://reviews.llvm.org/D21134
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/MisplacedArrayIndexCheck.cpp
clang-tidy/readability/MisplacedArrayIndexCheck.h
clang-tidy/readability/ReadabilityTidyModule.cpp
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/readability-misplaced-array-index.rst
test/clang-tidy/readability-misplaced-array-index.cpp
Index: test/clang-tidy/readability-misplaced-array-index.cpp
===
--- test/clang-tidy/readability-misplaced-array-index.cpp
+++ test/clang-tidy/readability-misplaced-array-index.cpp
@@ -0,0 +1,19 @@
+// RUN: %check_clang_tidy %s readability-misplaced-array-index %t
+
+#define ABC "abc"
+
+int unusualSyntax(int *x)
+{
+ 10[x] = 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: unusual array index syntax, usually the index is inside the []
+ // CHECK-FIXES: x[10] = 0;
+
+ return 1[ABC];
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: unusual array index syntax, usually the index is inside the []
+ // No fixit, we don't want to replace with "abc"[1]
+}
+
+int normalSyntax(int *x)
+{
+ return x[10];
+}
Index: docs/clang-tidy/checks/readability-misplaced-array-index.rst
===
--- docs/clang-tidy/checks/readability-misplaced-array-index.rst
+++ docs/clang-tidy/checks/readability-misplaced-array-index.rst
@@ -0,0 +1,29 @@
+.. title:: clang-tidy - readability-misplaced-array-index
+
+readability-misplaced-array-index
+=
+
+This check warns for unusual array index syntax.
+
+The following code has unusual array index syntax:
+
+.. code:: c++
+
+ void f(int *x, int y)
+ {
+y[x] = 0;
+ }
+
+becomes
+
+.. code:: c++
+
+ void f(int *x, int y)
+ {
+x[y] = 0;
+ }
+
+The check warns about such unusual syntax for readability reasons:
+ * There are programmers that are not familiar with this unusual syntax.
+ * It is possible that variables are mixed up.
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -120,6 +120,7 @@
readability-identifier-naming
readability-implicit-bool-cast
readability-inconsistent-declaration-parameter-name
+ readability-misplaced-array-index
readability-named-parameter
readability-redundant-control-flow
readability-redundant-smartptr-get
Index: clang-tidy/readability/ReadabilityTidyModule.cpp
===
--- clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -19,6 +19,7 @@
#include "IdentifierNamingCheck.h"
#include "ImplicitBoolCastCheck.h"
#include "InconsistentDeclarationParameterNameCheck.h"
+#include "MisplacedArrayIndexCheck.h"
#include "NamedParameterCheck.h"
#include "RedundantControlFlowCheck.h"
#include "RedundantSmartptrGetCheck.h"
@@ -53,6 +54,8 @@
"readability-implicit-bool-cast");
CheckFactories.registerCheck(
"readability-inconsistent-declaration-parameter-name");
+CheckFactories.registerCheck(
+"readability-misplaced-array-index");
CheckFactories.registerCheck(
"readability-static-definition-in-anonymous-namespace");
CheckFactories.registerCheck(
Index: clang-tidy/readability/MisplacedArrayIndexCheck.h
===
--- clang-tidy/readability/MisplacedArrayIndexCheck.h
+++ clang-tidy/readability/MisplacedArrayIndexCheck.h
@@ -0,0 +1,36 @@
+//===--- MisplacedArrayIndexCheck.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_MISPLACED_ARRAY_INDEX_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_MISPLACED_ARRAY_INDEX_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// Warn about unusual array index syntax (index[array] instead of
+/// array[index]).
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability-misplaced-array-index.html
+class MisplacedArrayIndexCheck : public ClangTidyCheck {
+public:
+
