mgehre updated this revision to Diff 39953.
mgehre added a comment.
simplify code
http://reviews.llvm.org/D13746
Files:
clang-tidy/cppcoreguidelines/CMakeLists.txt
clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.h
docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index.cpp
Index: test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index.cpp
@@ -0,0 +1,69 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-pro-bounds-constant-array-index %t -- -config='{CheckOptions: [{key: cppcoreguidelines-pro-bounds-constant-array-index.GslHeader, value: "dir1/gslheader.h"}]}' -- -std=c++11
+#include <array>
+// CHECK-FIXES: #include "dir1/gslheader.h"
+
+namespace gsl {
+ template<class T, size_t N>
+ T& at( T(&a)[N], size_t index );
+
+ template<class T, size_t N>
+ T& at( std::array<T, N> &a, size_t index );
+}
+
+constexpr int const_index(int base) {
+ return base + 3;
+}
+
+void f(std::array<int, 10> a, int pos) {
+ a [ pos / 2 /*comment*/] = 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use array subscript when the index is not a compile-time constant; use gsl::at() instead [cppcoreguidelines-pro-bounds-constant-array-index]
+ // CHECK-FIXES: gsl::at(a, pos / 2 /*comment*/) = 1;
+ int j = a[pos - 1];
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not use array subscript when the index is not a compile-time constant; use gsl::at() instead
+ // CHECK-FIXES: int j = gsl::at(a, pos - 1);
+
+ a.at(pos-1) = 2; // OK, at() instead of []
+ gsl::at(a, pos-1) = 2; // OK, gsl::at() instead of []
+
+ a[-1] = 3;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: array index -1 is before the beginning of the array [cppcoreguidelines-pro-bounds-constant-array-index]
+ a[10] = 4;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: array index 10 is past the end of the array (which contains 10 elements) [cppcoreguidelines-pro-bounds-constant-array-index]
+
+ a[const_index(7)] = 3;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: array index 10 is past the end of the array (which contains 10 elements)
+
+ a[0] = 3; // OK, constant index and inside bounds
+ a[1] = 3; // OK, constant index and inside bounds
+ a[9] = 3; // OK, constant index and inside bounds
+ a[const_index(6)] = 3; // OK, constant index and inside bounds
+}
+
+void g() {
+ int a[10];
+ for (int i = 0; i < 10; ++i) {
+ a[i] = i;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use array subscript when the index is not a compile-time constant; use gsl::at() instead
+ // CHECK-FIXES: gsl::at(a, i) = i;
+ gsl::at(a, i) = i; // OK, gsl::at() instead of []
+ }
+
+ a[-1] = 3; // flagged by clang-diagnostic-array-bounds
+ a[10] = 4; // flagged by clang-diagnostic-array-bounds
+ a[const_index(7)] = 3; // flagged by clang-diagnostic-array-bounds
+
+ a[0] = 3; // OK, constant index and inside bounds
+ a[1] = 3; // OK, constant index and inside bounds
+ a[9] = 3; // OK, constant index and inside bounds
+ a[const_index(6)] = 3; // OK, constant index and inside bounds
+}
+
+struct S {
+ int& operator[](int i);
+};
+
+void customOperator() {
+ S s;
+ int i = 0;
+ s[i] = 3; // OK, custom operator
+}
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -5,6 +5,7 @@
cert-setlongjmp
cert-variadic-function-def
cppcoreguidelines-pro-bounds-array-to-pointer-decay
+ cppcoreguidelines-pro-bounds-constant-array-index
cppcoreguidelines-pro-bounds-pointer-arithmetic
cppcoreguidelines-pro-type-const-cast
cppcoreguidelines-pro-type-reinterpret-cast
Index: docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst
@@ -0,0 +1,13 @@
+cppcoreguidelines-pro-bounds-constant-array-index
+=================================================
+
+This check flags all array subscriptions on static arrays and std::arrays that either have a non-compile-time constant index or are out of bounds (for std::array).
+For out-of-bounds checking of static arrays, see the clang-diagnostic-array-bounds check.
+
+Dynamic accesses into arrays are difficult for both tools and humans to validate as safe. gsl::span is a bounds-checked, safe type for accessing arrays of data. gsl::at() is another alternative that ensures single accesses are bounds-checked. If iterators are needed to access an array, use the iterators from an gsl::span constructed over the array.
+
+The check can generated fixes after the option cppcoreguidelines-pro-bounds-constant-array-index.GslHeader has been set to the name of the
+include file that contains gsl::at(), e.g. "gsl/gsl.h".
+
+This rule is part of the "Bounds safety" profile of the C++ Core Guidelines, see
+https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#-bounds2-only-index-into-arrays-using-constant-expressions.
Index: clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.h
@@ -0,0 +1,40 @@
+//===--- ProBoundsConstantArrayIndexCheck.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_CPPCOREGUIDELINES_PRO_BOUNDS_CONSTANT_ARRAY_INDEX_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PRO_BOUNDS_CONSTANT_ARRAY_INDEX_H
+
+#include "../ClangTidy.h"
+#include "../utils/IncludeInserter.h"
+
+namespace clang {
+namespace tidy {
+
+/// This checks that all array subscriptions on static arrays and std::arrays
+/// have a constant index and are within bounds
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.html
+class ProBoundsConstantArrayIndexCheck : public ClangTidyCheck {
+ std::string GslHeader;
+ const IncludeSorter::IncludeStyle IncludeStyle;
+ std::unique_ptr<IncludeInserter> Inserter;
+
+public:
+ ProBoundsConstantArrayIndexCheck(StringRef Name, ClangTidyContext *Context);
+ void registerPPCallbacks(CompilerInstance &Compiler) override;
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PRO_BOUNDS_CONSTANT_ARRAY_INDEX_H
Index: clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
@@ -0,0 +1,131 @@
+//===--- ProBoundsConstantArrayIndexCheck.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 "ProBoundsConstantArrayIndexCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Preprocessor.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+
+ProBoundsConstantArrayIndexCheck::ProBoundsConstantArrayIndexCheck(
+ StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context), GslHeader(Options.get("GslHeader", "")),
+ IncludeStyle(IncludeSorter::parseIncludeStyle(
+ Options.get("IncludeStyle", "llvm"))) {}
+
+void ProBoundsConstantArrayIndexCheck::storeOptions(
+ ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "GslHeader", GslHeader);
+}
+
+void ProBoundsConstantArrayIndexCheck::registerPPCallbacks(
+ CompilerInstance &Compiler) {
+ if (!getLangOpts().CPlusPlus)
+ return;
+
+ Inserter.reset(new IncludeInserter(Compiler.getSourceManager(),
+ Compiler.getLangOpts(), IncludeStyle));
+ Compiler.getPreprocessor().addPPCallbacks(Inserter->CreatePPCallbacks());
+}
+
+void ProBoundsConstantArrayIndexCheck::registerMatchers(MatchFinder *Finder) {
+ if (!getLangOpts().CPlusPlus)
+ return;
+
+ Finder->addMatcher(arraySubscriptExpr(hasBase(ignoringImpCasts(hasType(
+ constantArrayType().bind("type")))),
+ hasIndex(expr().bind("index")))
+ .bind("expr"),
+ this);
+
+ Finder->addMatcher(
+ cxxOperatorCallExpr(
+ hasOverloadedOperatorName("[]"),
+ hasArgument(
+ 0, hasType(cxxRecordDecl(hasName("::std::array")).bind("type"))),
+ hasArgument(1, expr().bind("index")))
+ .bind("expr"),
+ this);
+}
+
+void ProBoundsConstantArrayIndexCheck::check(
+ const MatchFinder::MatchResult &Result) {
+ const auto *Matched = Result.Nodes.getNodeAs<Expr>("expr");
+ const auto *IndexExpr = Result.Nodes.getNodeAs<Expr>("index");
+ llvm::APSInt Index;
+ if (!IndexExpr->isIntegerConstantExpr(Index, *Result.Context, nullptr,
+ /*isEvaluated=*/true)) {
+ SourceRange BaseRange;
+ if (const auto *ArraySubscriptE = dyn_cast<ArraySubscriptExpr>(Matched))
+ BaseRange = ArraySubscriptE->getBase()->getSourceRange();
+ else
+ BaseRange =
+ dyn_cast<CXXOperatorCallExpr>(Matched)->getArg(0)->getSourceRange();
+ SourceRange IndexRange = IndexExpr->getSourceRange();
+
+ auto Diag = diag(Matched->getExprLoc(),
+ "do not use array subscript when the index is "
+ "not a compile-time constant; use gsl::at() "
+ "instead");
+ if (!GslHeader.empty()) {
+ Diag << FixItHint::CreateInsertion(BaseRange.getBegin(), "gsl::at(")
+ << FixItHint::CreateReplacement(
+ SourceRange(BaseRange.getEnd().getLocWithOffset(1),
+ IndexRange.getBegin().getLocWithOffset(-1)),
+ ", ")
+ << FixItHint::CreateReplacement(Matched->getLocEnd(), ")");
+
+ auto Insertion = Inserter->CreateIncludeInsertion(
+ Result.SourceManager->getMainFileID(), GslHeader,
+ /*IsAngled=*/false);
+ if (Insertion.hasValue())
+ Diag << Insertion.getValue();
+ }
+ return;
+ }
+
+ const auto *StdArrayDecl =
+ Result.Nodes.getNodeAs<ClassTemplateSpecializationDecl>("type");
+
+ // For static arrays, this is handled in clang-diagnostic-array-bounds.
+ if (!StdArrayDecl)
+ return;
+
+ if (Index.isSigned() && Index.isNegative()) {
+ diag(Matched->getExprLoc(),
+ "array index %0 is before the beginning of the array")
+ << Index.toString(10);
+ return;
+ }
+
+ const auto &TemplateArgs = StdArrayDecl->getTemplateArgs();
+ if (TemplateArgs.size() < 2)
+ return;
+ // First template arg of std::array is the type, second arg is the size.
+ const auto &SizeArg = TemplateArgs[1];
+ if (SizeArg.getKind() != TemplateArgument::Integral)
+ return;
+ llvm::APInt ArraySize = SizeArg.getAsIntegral();
+
+ // Get uint64_t values, because different bitwidths would lead to an assertion
+ // in APInt::uge.
+ if (Index.getZExtValue() >= ArraySize.getZExtValue()) {
+ diag(Matched->getExprLoc(), "array index %0 is past the end of the array "
+ "(which contains %1 elements)")
+ << Index.toString(10) << ArraySize.toString(10, false);
+ }
+}
+
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
===================================================================
--- clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
+++ clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
@@ -12,6 +12,7 @@
#include "../ClangTidyModuleRegistry.h"
#include "../misc/AssignOperatorSignatureCheck.h"
#include "ProBoundsArrayToPointerDecayCheck.h"
+#include "ProBoundsConstantArrayIndexCheck.h"
#include "ProBoundsPointerArithmeticCheck.h"
#include "ProTypeConstCastCheck.h"
#include "ProTypeReinterpretCastCheck.h"
@@ -29,6 +30,8 @@
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
CheckFactories.registerCheck<ProBoundsArrayToPointerDecayCheck>(
"cppcoreguidelines-pro-bounds-array-to-pointer-decay");
+ CheckFactories.registerCheck<ProBoundsConstantArrayIndexCheck>(
+ "cppcoreguidelines-pro-bounds-constant-array-index");
CheckFactories.registerCheck<ProBoundsPointerArithmeticCheck>(
"cppcoreguidelines-pro-bounds-pointer-arithmetic");
CheckFactories.registerCheck<ProTypeConstCastCheck>(
Index: clang-tidy/cppcoreguidelines/CMakeLists.txt
===================================================================
--- clang-tidy/cppcoreguidelines/CMakeLists.txt
+++ clang-tidy/cppcoreguidelines/CMakeLists.txt
@@ -3,6 +3,7 @@
add_clang_library(clangTidyCppCoreGuidelinesModule
CppCoreGuidelinesTidyModule.cpp
ProBoundsArrayToPointerDecayCheck.cpp
+ ProBoundsConstantArrayIndexCheck.cpp
ProBoundsPointerArithmeticCheck.cpp
ProTypeConstCastCheck.cpp
ProTypeReinterpretCastCheck.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
