[clang-tools-extra] Enforce SL.con.3: Add check to replace operator[] with at() [Cont.] (PR #95220)
https://github.com/PBHDK updated https://github.com/llvm/llvm-project/pull/95220
From 37292995de0c5aa87408586749795a97468d4725 Mon Sep 17 00:00:00 2001
From: Sebastian Wolf
Date: Wed, 17 Apr 2024 16:16:35 +0200
Subject: [PATCH 01/23] Enforce SL.con.3: Add check to replace operator[] with
at() on std containers
---
.../AvoidBoundsErrorsCheck.cpp| 81 +++
.../AvoidBoundsErrorsCheck.h | 32
.../cppcoreguidelines/CMakeLists.txt | 1 +
.../CppCoreGuidelinesTidyModule.cpp | 3 +
clang-tools-extra/docs/ReleaseNotes.rst | 5 ++
.../cppcoreguidelines/avoid-bounds-errors.rst | 20 +
.../docs/clang-tidy/checks/list.rst | 1 +
.../cppcoreguidelines/avoid-bounds-errors.cpp | 66 +++
8 files changed, 209 insertions(+)
create mode 100644
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
create mode 100644
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
create mode 100644
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-bounds-errors.rst
create mode 100644
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-bounds-errors.cpp
diff --git
a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
new file mode 100644
index 0..524c21b5bdb81
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
@@ -0,0 +1,81 @@
+//===--- AvoidBoundsErrorsCheck.cpp - clang-tidy
--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "AvoidBoundsErrorsCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+#include
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::cppcoreguidelines {
+
+bool isApplicable(const QualType &Type) {
+ const auto TypeStr = Type.getAsString();
+ bool Result = false;
+ // Only check for containers in the std namespace
+ if (TypeStr.find("std::vector") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::array") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::deque") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::map") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::unordered_map") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::flat_map") != std::string::npos) {
+Result = true;
+ }
+ // TODO Add std::span with C++26
+ return Result;
+}
+
+void AvoidBoundsErrorsCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ callExpr(callee(cxxMethodDecl(hasName("operator[]")).bind("f")))
+ .bind("x"),
+ this);
+}
+
+void AvoidBoundsErrorsCheck::check(const MatchFinder::MatchResult &Result) {
+ const ASTContext &Context = *Result.Context;
+ const SourceManager &Source = Context.getSourceManager();
+ const auto *MatchedExpr = Result.Nodes.getNodeAs("x");
+ const auto *MatchedFunction = Result.Nodes.getNodeAs("f");
+ const auto Type = MatchedFunction->getThisType();
+ if (!isApplicable(Type)) {
+return;
+ }
+
+ // Get original code.
+ const SourceLocation b(MatchedExpr->getBeginLoc());
+ const SourceLocation e(MatchedExpr->getEndLoc());
+ const std::string OriginalCode =
+ Lexer::getSourceText(CharSourceRange::getTokenRange(b, e), Source,
+ getLangOpts())
+ .str();
+ const auto Range = SourceRange(b, e);
+
+ // Build replacement.
+ std::string NewCode = OriginalCode;
+ const auto BeginOpen = NewCode.find("[");
+ NewCode.replace(BeginOpen, 1, ".at(");
+ const auto BeginClose = NewCode.find("]");
+ NewCode.replace(BeginClose, 1, ")");
+
+ diag(MatchedExpr->getBeginLoc(), "Do not use operator[], use at() instead.")
+ << FixItHint::CreateReplacement(Range, NewCode);
+}
+
+} // namespace clang::tidy::cppcoreguidelines
diff --git
a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
new file mode 100644
index 0..f915729cd7bbe
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
@@ -0,0 +1,32 @@
+//===--- AvoidBoundsErrorsCheck.h - clang-tidy --*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef
LLV
[clang-tools-extra] Enforce SL.con.3: Add check to replace operator[] with at() [Cont.] (PR #95220)
https://github.com/PBHDK updated https://github.com/llvm/llvm-project/pull/95220
From 37292995de0c5aa87408586749795a97468d4725 Mon Sep 17 00:00:00 2001
From: Sebastian Wolf
Date: Wed, 17 Apr 2024 16:16:35 +0200
Subject: [PATCH 01/24] Enforce SL.con.3: Add check to replace operator[] with
at() on std containers
---
.../AvoidBoundsErrorsCheck.cpp| 81 +++
.../AvoidBoundsErrorsCheck.h | 32
.../cppcoreguidelines/CMakeLists.txt | 1 +
.../CppCoreGuidelinesTidyModule.cpp | 3 +
clang-tools-extra/docs/ReleaseNotes.rst | 5 ++
.../cppcoreguidelines/avoid-bounds-errors.rst | 20 +
.../docs/clang-tidy/checks/list.rst | 1 +
.../cppcoreguidelines/avoid-bounds-errors.cpp | 66 +++
8 files changed, 209 insertions(+)
create mode 100644
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
create mode 100644
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
create mode 100644
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-bounds-errors.rst
create mode 100644
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-bounds-errors.cpp
diff --git
a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
new file mode 100644
index 0..524c21b5bdb81
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
@@ -0,0 +1,81 @@
+//===--- AvoidBoundsErrorsCheck.cpp - clang-tidy
--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "AvoidBoundsErrorsCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+#include
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::cppcoreguidelines {
+
+bool isApplicable(const QualType &Type) {
+ const auto TypeStr = Type.getAsString();
+ bool Result = false;
+ // Only check for containers in the std namespace
+ if (TypeStr.find("std::vector") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::array") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::deque") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::map") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::unordered_map") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::flat_map") != std::string::npos) {
+Result = true;
+ }
+ // TODO Add std::span with C++26
+ return Result;
+}
+
+void AvoidBoundsErrorsCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ callExpr(callee(cxxMethodDecl(hasName("operator[]")).bind("f")))
+ .bind("x"),
+ this);
+}
+
+void AvoidBoundsErrorsCheck::check(const MatchFinder::MatchResult &Result) {
+ const ASTContext &Context = *Result.Context;
+ const SourceManager &Source = Context.getSourceManager();
+ const auto *MatchedExpr = Result.Nodes.getNodeAs("x");
+ const auto *MatchedFunction = Result.Nodes.getNodeAs("f");
+ const auto Type = MatchedFunction->getThisType();
+ if (!isApplicable(Type)) {
+return;
+ }
+
+ // Get original code.
+ const SourceLocation b(MatchedExpr->getBeginLoc());
+ const SourceLocation e(MatchedExpr->getEndLoc());
+ const std::string OriginalCode =
+ Lexer::getSourceText(CharSourceRange::getTokenRange(b, e), Source,
+ getLangOpts())
+ .str();
+ const auto Range = SourceRange(b, e);
+
+ // Build replacement.
+ std::string NewCode = OriginalCode;
+ const auto BeginOpen = NewCode.find("[");
+ NewCode.replace(BeginOpen, 1, ".at(");
+ const auto BeginClose = NewCode.find("]");
+ NewCode.replace(BeginClose, 1, ")");
+
+ diag(MatchedExpr->getBeginLoc(), "Do not use operator[], use at() instead.")
+ << FixItHint::CreateReplacement(Range, NewCode);
+}
+
+} // namespace clang::tidy::cppcoreguidelines
diff --git
a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
new file mode 100644
index 0..f915729cd7bbe
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
@@ -0,0 +1,32 @@
+//===--- AvoidBoundsErrorsCheck.h - clang-tidy --*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef
LLV
[clang-tools-extra] Enforce SL.con.3: Add check to replace operator[] with at() [Cont.] (PR #95220)
https://github.com/PBHDK updated https://github.com/llvm/llvm-project/pull/95220
From 37292995de0c5aa87408586749795a97468d4725 Mon Sep 17 00:00:00 2001
From: Sebastian Wolf
Date: Wed, 17 Apr 2024 16:16:35 +0200
Subject: [PATCH 01/25] Enforce SL.con.3: Add check to replace operator[] with
at() on std containers
---
.../AvoidBoundsErrorsCheck.cpp| 81 +++
.../AvoidBoundsErrorsCheck.h | 32
.../cppcoreguidelines/CMakeLists.txt | 1 +
.../CppCoreGuidelinesTidyModule.cpp | 3 +
clang-tools-extra/docs/ReleaseNotes.rst | 5 ++
.../cppcoreguidelines/avoid-bounds-errors.rst | 20 +
.../docs/clang-tidy/checks/list.rst | 1 +
.../cppcoreguidelines/avoid-bounds-errors.cpp | 66 +++
8 files changed, 209 insertions(+)
create mode 100644
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
create mode 100644
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
create mode 100644
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-bounds-errors.rst
create mode 100644
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-bounds-errors.cpp
diff --git
a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
new file mode 100644
index 0..524c21b5bdb81
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
@@ -0,0 +1,81 @@
+//===--- AvoidBoundsErrorsCheck.cpp - clang-tidy
--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "AvoidBoundsErrorsCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+#include
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::cppcoreguidelines {
+
+bool isApplicable(const QualType &Type) {
+ const auto TypeStr = Type.getAsString();
+ bool Result = false;
+ // Only check for containers in the std namespace
+ if (TypeStr.find("std::vector") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::array") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::deque") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::map") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::unordered_map") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::flat_map") != std::string::npos) {
+Result = true;
+ }
+ // TODO Add std::span with C++26
+ return Result;
+}
+
+void AvoidBoundsErrorsCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ callExpr(callee(cxxMethodDecl(hasName("operator[]")).bind("f")))
+ .bind("x"),
+ this);
+}
+
+void AvoidBoundsErrorsCheck::check(const MatchFinder::MatchResult &Result) {
+ const ASTContext &Context = *Result.Context;
+ const SourceManager &Source = Context.getSourceManager();
+ const auto *MatchedExpr = Result.Nodes.getNodeAs("x");
+ const auto *MatchedFunction = Result.Nodes.getNodeAs("f");
+ const auto Type = MatchedFunction->getThisType();
+ if (!isApplicable(Type)) {
+return;
+ }
+
+ // Get original code.
+ const SourceLocation b(MatchedExpr->getBeginLoc());
+ const SourceLocation e(MatchedExpr->getEndLoc());
+ const std::string OriginalCode =
+ Lexer::getSourceText(CharSourceRange::getTokenRange(b, e), Source,
+ getLangOpts())
+ .str();
+ const auto Range = SourceRange(b, e);
+
+ // Build replacement.
+ std::string NewCode = OriginalCode;
+ const auto BeginOpen = NewCode.find("[");
+ NewCode.replace(BeginOpen, 1, ".at(");
+ const auto BeginClose = NewCode.find("]");
+ NewCode.replace(BeginClose, 1, ")");
+
+ diag(MatchedExpr->getBeginLoc(), "Do not use operator[], use at() instead.")
+ << FixItHint::CreateReplacement(Range, NewCode);
+}
+
+} // namespace clang::tidy::cppcoreguidelines
diff --git
a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
new file mode 100644
index 0..f915729cd7bbe
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
@@ -0,0 +1,32 @@
+//===--- AvoidBoundsErrorsCheck.h - clang-tidy --*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef
LLV
[clang-tools-extra] Enforce SL.con.3: Add check to replace operator[] with at() [Cont.] (PR #95220)
@@ -0,0 +1,124 @@
+//===--- ProBoundsAvoidUncheckedContainerAccesses.cpp - clang-tidy
===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ProBoundsAvoidUncheckedContainerAccesses.h"
+#include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "llvm/ADT/StringRef.h"
+#include
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::cppcoreguidelines {
+
+static constexpr std::array SubscriptDefaultExclusions = {
+llvm::StringRef("::std::map"), llvm::StringRef("::std::unordered_map"),
+llvm::StringRef("::std::flat_map")};
+
+ProBoundsAvoidUncheckedContainerAccesses::
+ProBoundsAvoidUncheckedContainerAccesses(StringRef Name,
+ ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context) {
+
+ SubscriptExcludedClasses = clang::tidy::utils::options::parseStringList(
+ Options.get("ExcludeClasses", ""));
+ SubscriptExcludedClasses.insert(SubscriptExcludedClasses.end(),
+ SubscriptDefaultExclusions.begin(),
+ SubscriptDefaultExclusions.end());
+}
+
+void ProBoundsAvoidUncheckedContainerAccesses::storeOptions(
+ClangTidyOptions::OptionMap &Opts) {
+
+ if (SubscriptExcludedClasses.size() == SubscriptDefaultExclusions.size()) {
+Options.store(Opts, "ExcludeClasses", "");
+return;
+ }
+
+ // Sum up the sizes of the defaults ( + semicolons), so we can remove them
+ // from the saved options
+ size_t DefaultsStringLength = std::transform_reduce(
+ SubscriptDefaultExclusions.begin(), SubscriptDefaultExclusions.end(),
+ SubscriptDefaultExclusions.size(), std::plus<>(),
+ [](llvm::StringRef Name) { return Name.size(); });
+
+ std::string Serialized = clang::tidy::utils::options::serializeStringList(
+ SubscriptExcludedClasses);
+
+ Options.store(Opts, "ExcludeClasses",
+Serialized.substr(0, Serialized.size() -
DefaultsStringLength));
+}
+
+const CXXMethodDecl *findAlternative(const CXXRecordDecl *MatchedParent,
+ const CXXMethodDecl *MatchedOperator) {
PBHDK wrote:
Ah, fair point.
@HerrCai0907, would a simple "TODO: account for alternative being defined in
subclass" work? Or am I misunderstanding something?
https://github.com/llvm/llvm-project/pull/95220
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Enforce SL.con.3: Add check to replace operator[] with at() [Cont.] (PR #95220)
@@ -0,0 +1,124 @@
+//===--- ProBoundsAvoidUncheckedContainerAccesses.cpp - clang-tidy
===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ProBoundsAvoidUncheckedContainerAccesses.h"
+#include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "llvm/ADT/StringRef.h"
+#include
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::cppcoreguidelines {
+
+static constexpr std::array SubscriptDefaultExclusions = {
+llvm::StringRef("::std::map"), llvm::StringRef("::std::unordered_map"),
+llvm::StringRef("::std::flat_map")};
PBHDK wrote:
The "record have key_type, value_type, and .at method" suggestion would blindly
exclude any map-related at() method, even if its semantics are different,
correct?
Do you think this could cause any issues? Is it fair to assume that other
libraries will define it so that it needs to be excluded?
Worst case, we have false negatives. Or am I misunderstanding something?
https://github.com/llvm/llvm-project/pull/95220
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Enforce SL.con.3: Add check to replace operator[] with at() (PR #90043)
PBHDK wrote: > > > I am adding @leunam99 and @PBHDK to the PR, who will contribute in the > > > next few weeks. > > > > > > Since we cannot push to this PR, we will be submitting a new PR with > > @sebwolf-de's + our work, correct, @EugeneZelenko, @PiotrZSL, @HerrCai0907? > > Could you please resolve conversations for fixed comments? Once we're clear on all our questions (see comments above), we'll submit a new PR that'll resolve all comments. The code is ready, but we don't want to submit the PR as long as we still have open questions. The problem is that we can't mark them as resolved on GitHub ourselves, and Sebastian is out-of-office right now. I don't know if you can assign us to this PR somehow, but if not, someone else would have to mark them as resolved. https://github.com/llvm/llvm-project/pull/90043 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Enforce SL.con.3: Add check to replace operator[] with at() (PR #90043)
PBHDK wrote: > Check if you can commit to sebwolf-de:avoid-bounds-error-check Unfortunately, we can't. We do not have access to Sebastian's LLVM fork. Since Sebastian is out of office right now - this is one of the reasons we're involved - would you be ok with us submitting a new PR with our changes where we reference the current PR? https://github.com/llvm/llvm-project/pull/90043 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Enforce SL.con.3: Add check to replace operator[] with at() [Cont.] (PR #95220)
https://github.com/PBHDK created https://github.com/llvm/llvm-project/pull/95220
This PR is based on the PR #90043 by @sebwolf-de, who has given us (@leunam99
and myself) [permission to continue his
work](https://github.com/llvm/llvm-project/pull/90043#issuecomment-2137455627).
The original PR message reads:
> The string based test to find out whether the check is applicable on the
> class is not ideal, but I did not find a more elegant way, yet.
> If there is more clang-query magic available, that I'm not aware of, I'm
> happy to adapt that.
As part of the reviews for that PR, @sebwolf-de changed the following:
- Detect viable classes automatically instead of looking for fixed names
- Disable fix-it suggestions
This PR contains the same changes and, in addition, addresses further feedback
provided by the maintainers.
Changes in addition to the original PR:
- Exclude `std::map`, `std::flat_map`, and `std::unordered_set` from the
analysis by default, and add the ability for users to exclude additional
classes from the analysis
- Add the tests @PiotrZSL requested
[here](https://github.com/llvm/llvm-project/pull/90043#discussion_r1579858850)
- Rename the analysis from AvoidBoundsErrorsCheck to
PreferAtOverSubscriptOperatorCheck as requested by @PiotrZSL
[here](https://github.com/llvm/llvm-project/pull/90043#discussion_r1579853430)
- Add a more detailed description of what the analysis does as requested by
@PiotrZSL
[here](https://github.com/llvm/llvm-project/pull/90043#discussion_r1579847155)
We explicitly don't ignore unused code with `TK_IgnoreUnlessSpelledInSource`,
as requested by @PiotrZSL
[here](https://github.com/llvm/llvm-project/pull/90043#discussion_r1579844550),
because it caused the template-related tests to fail.
We are not sure what the best behaviour for templates is; should we:
- not warn if using `at()` will make a different instantiation not compile?
- warn at the place that requires the template instantiation?
- keep the warning and add the name of the class of the object / the template
parameters that lead to the message?
- not warn in templates at all because the code is implicit?
@carlosgalvezp and @HerrCai0907 discussed the possibility of disabling the
check when exceptions are disabled, but we were unsure whether they'd reached a
conclusion and whether it was still relevant when fix-it suggestions are
disabled.
What do you think?
From 37292995de0c5aa87408586749795a97468d4725 Mon Sep 17 00:00:00 2001
From: Sebastian Wolf
Date: Wed, 17 Apr 2024 16:16:35 +0200
Subject: [PATCH 1/8] Enforce SL.con.3: Add check to replace operator[] with
at() on std containers
---
.../AvoidBoundsErrorsCheck.cpp| 81 +++
.../AvoidBoundsErrorsCheck.h | 32
.../cppcoreguidelines/CMakeLists.txt | 1 +
.../CppCoreGuidelinesTidyModule.cpp | 3 +
clang-tools-extra/docs/ReleaseNotes.rst | 5 ++
.../cppcoreguidelines/avoid-bounds-errors.rst | 20 +
.../docs/clang-tidy/checks/list.rst | 1 +
.../cppcoreguidelines/avoid-bounds-errors.cpp | 66 +++
8 files changed, 209 insertions(+)
create mode 100644
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
create mode 100644
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
create mode 100644
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-bounds-errors.rst
create mode 100644
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-bounds-errors.cpp
diff --git
a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
new file mode 100644
index 0..524c21b5bdb81
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
@@ -0,0 +1,81 @@
+//===--- AvoidBoundsErrorsCheck.cpp - clang-tidy
--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "AvoidBoundsErrorsCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+#include
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::cppcoreguidelines {
+
+bool isApplicable(const QualType &Type) {
+ const auto TypeStr = Type.getAsString();
+ bool Result = false;
+ // Only check for containers in the std namespace
+ if (TypeStr.find("std::vector") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::array") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::deque") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::map") != std::string::npos) {
+Result = true;
+ }
[clang-tools-extra] Enforce SL.con.3: Add check to replace operator[] with at() (PR #90043)
PBHDK wrote: > Yes, you can just submit new PR, reference this one. Done! Here it is: https://github.com/llvm/llvm-project/pull/95220 https://github.com/llvm/llvm-project/pull/90043 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Enforce SL.con.3: Add check to replace operator[] with at() [Cont.] (PR #95220)
@@ -0,0 +1,31 @@ +.. title:: clang-tidy - cppcoreguidelines-prefer-at-over-subscript-operator + +cppcoreguidelines-prefer-at-over-subscript-operator += + +This check flags all uses of ``operator[]`` where an equivalent (same parameter and return types) ``at()`` method exists and suggest using that instead. PBHDK wrote: Which of the two statements do you prefer: this one or the one in the release notes? > Please synchronize with statement in Release Notes. Which of the two statements do you prefer: this one or the one in the release notes? https://github.com/llvm/llvm-project/pull/95220 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Enforce SL.con.3: Add check to replace operator[] with at() [Cont.] (PR #95220)
https://github.com/PBHDK edited https://github.com/llvm/llvm-project/pull/95220 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Enforce SL.con.3: Add check to replace operator[] with at() [Cont.] (PR #95220)
https://github.com/PBHDK edited https://github.com/llvm/llvm-project/pull/95220 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Enforce SL.con.3: Add check to replace operator[] with at() [Cont.] (PR #95220)
https://github.com/PBHDK updated https://github.com/llvm/llvm-project/pull/95220
From 37292995de0c5aa87408586749795a97468d4725 Mon Sep 17 00:00:00 2001
From: Sebastian Wolf
Date: Wed, 17 Apr 2024 16:16:35 +0200
Subject: [PATCH 01/11] Enforce SL.con.3: Add check to replace operator[] with
at() on std containers
---
.../AvoidBoundsErrorsCheck.cpp| 81 +++
.../AvoidBoundsErrorsCheck.h | 32
.../cppcoreguidelines/CMakeLists.txt | 1 +
.../CppCoreGuidelinesTidyModule.cpp | 3 +
clang-tools-extra/docs/ReleaseNotes.rst | 5 ++
.../cppcoreguidelines/avoid-bounds-errors.rst | 20 +
.../docs/clang-tidy/checks/list.rst | 1 +
.../cppcoreguidelines/avoid-bounds-errors.cpp | 66 +++
8 files changed, 209 insertions(+)
create mode 100644
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
create mode 100644
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
create mode 100644
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-bounds-errors.rst
create mode 100644
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-bounds-errors.cpp
diff --git
a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
new file mode 100644
index 0..524c21b5bdb81
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
@@ -0,0 +1,81 @@
+//===--- AvoidBoundsErrorsCheck.cpp - clang-tidy
--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "AvoidBoundsErrorsCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+#include
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::cppcoreguidelines {
+
+bool isApplicable(const QualType &Type) {
+ const auto TypeStr = Type.getAsString();
+ bool Result = false;
+ // Only check for containers in the std namespace
+ if (TypeStr.find("std::vector") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::array") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::deque") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::map") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::unordered_map") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::flat_map") != std::string::npos) {
+Result = true;
+ }
+ // TODO Add std::span with C++26
+ return Result;
+}
+
+void AvoidBoundsErrorsCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ callExpr(callee(cxxMethodDecl(hasName("operator[]")).bind("f")))
+ .bind("x"),
+ this);
+}
+
+void AvoidBoundsErrorsCheck::check(const MatchFinder::MatchResult &Result) {
+ const ASTContext &Context = *Result.Context;
+ const SourceManager &Source = Context.getSourceManager();
+ const auto *MatchedExpr = Result.Nodes.getNodeAs("x");
+ const auto *MatchedFunction = Result.Nodes.getNodeAs("f");
+ const auto Type = MatchedFunction->getThisType();
+ if (!isApplicable(Type)) {
+return;
+ }
+
+ // Get original code.
+ const SourceLocation b(MatchedExpr->getBeginLoc());
+ const SourceLocation e(MatchedExpr->getEndLoc());
+ const std::string OriginalCode =
+ Lexer::getSourceText(CharSourceRange::getTokenRange(b, e), Source,
+ getLangOpts())
+ .str();
+ const auto Range = SourceRange(b, e);
+
+ // Build replacement.
+ std::string NewCode = OriginalCode;
+ const auto BeginOpen = NewCode.find("[");
+ NewCode.replace(BeginOpen, 1, ".at(");
+ const auto BeginClose = NewCode.find("]");
+ NewCode.replace(BeginClose, 1, ")");
+
+ diag(MatchedExpr->getBeginLoc(), "Do not use operator[], use at() instead.")
+ << FixItHint::CreateReplacement(Range, NewCode);
+}
+
+} // namespace clang::tidy::cppcoreguidelines
diff --git
a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
new file mode 100644
index 0..f915729cd7bbe
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
@@ -0,0 +1,32 @@
+//===--- AvoidBoundsErrorsCheck.h - clang-tidy --*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef
LLV
[clang-tools-extra] Enforce SL.con.3: Add check to replace operator[] with at() [Cont.] (PR #95220)
https://github.com/PBHDK updated https://github.com/llvm/llvm-project/pull/95220
From 37292995de0c5aa87408586749795a97468d4725 Mon Sep 17 00:00:00 2001
From: Sebastian Wolf
Date: Wed, 17 Apr 2024 16:16:35 +0200
Subject: [PATCH 01/12] Enforce SL.con.3: Add check to replace operator[] with
at() on std containers
---
.../AvoidBoundsErrorsCheck.cpp| 81 +++
.../AvoidBoundsErrorsCheck.h | 32
.../cppcoreguidelines/CMakeLists.txt | 1 +
.../CppCoreGuidelinesTidyModule.cpp | 3 +
clang-tools-extra/docs/ReleaseNotes.rst | 5 ++
.../cppcoreguidelines/avoid-bounds-errors.rst | 20 +
.../docs/clang-tidy/checks/list.rst | 1 +
.../cppcoreguidelines/avoid-bounds-errors.cpp | 66 +++
8 files changed, 209 insertions(+)
create mode 100644
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
create mode 100644
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
create mode 100644
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-bounds-errors.rst
create mode 100644
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-bounds-errors.cpp
diff --git
a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
new file mode 100644
index 0..524c21b5bdb81
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
@@ -0,0 +1,81 @@
+//===--- AvoidBoundsErrorsCheck.cpp - clang-tidy
--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "AvoidBoundsErrorsCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+#include
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::cppcoreguidelines {
+
+bool isApplicable(const QualType &Type) {
+ const auto TypeStr = Type.getAsString();
+ bool Result = false;
+ // Only check for containers in the std namespace
+ if (TypeStr.find("std::vector") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::array") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::deque") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::map") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::unordered_map") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::flat_map") != std::string::npos) {
+Result = true;
+ }
+ // TODO Add std::span with C++26
+ return Result;
+}
+
+void AvoidBoundsErrorsCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ callExpr(callee(cxxMethodDecl(hasName("operator[]")).bind("f")))
+ .bind("x"),
+ this);
+}
+
+void AvoidBoundsErrorsCheck::check(const MatchFinder::MatchResult &Result) {
+ const ASTContext &Context = *Result.Context;
+ const SourceManager &Source = Context.getSourceManager();
+ const auto *MatchedExpr = Result.Nodes.getNodeAs("x");
+ const auto *MatchedFunction = Result.Nodes.getNodeAs("f");
+ const auto Type = MatchedFunction->getThisType();
+ if (!isApplicable(Type)) {
+return;
+ }
+
+ // Get original code.
+ const SourceLocation b(MatchedExpr->getBeginLoc());
+ const SourceLocation e(MatchedExpr->getEndLoc());
+ const std::string OriginalCode =
+ Lexer::getSourceText(CharSourceRange::getTokenRange(b, e), Source,
+ getLangOpts())
+ .str();
+ const auto Range = SourceRange(b, e);
+
+ // Build replacement.
+ std::string NewCode = OriginalCode;
+ const auto BeginOpen = NewCode.find("[");
+ NewCode.replace(BeginOpen, 1, ".at(");
+ const auto BeginClose = NewCode.find("]");
+ NewCode.replace(BeginClose, 1, ")");
+
+ diag(MatchedExpr->getBeginLoc(), "Do not use operator[], use at() instead.")
+ << FixItHint::CreateReplacement(Range, NewCode);
+}
+
+} // namespace clang::tidy::cppcoreguidelines
diff --git
a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
new file mode 100644
index 0..f915729cd7bbe
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
@@ -0,0 +1,32 @@
+//===--- AvoidBoundsErrorsCheck.h - clang-tidy --*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef
LLV
[clang-tools-extra] Enforce SL.con.3: Add check to replace operator[] with at() [Cont.] (PR #95220)
https://github.com/PBHDK edited https://github.com/llvm/llvm-project/pull/95220 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Enforce SL.con.3: Add check to replace operator[] with at() [Cont.] (PR #95220)
https://github.com/PBHDK updated https://github.com/llvm/llvm-project/pull/95220
From 37292995de0c5aa87408586749795a97468d4725 Mon Sep 17 00:00:00 2001
From: Sebastian Wolf
Date: Wed, 17 Apr 2024 16:16:35 +0200
Subject: [PATCH 01/15] Enforce SL.con.3: Add check to replace operator[] with
at() on std containers
---
.../AvoidBoundsErrorsCheck.cpp| 81 +++
.../AvoidBoundsErrorsCheck.h | 32
.../cppcoreguidelines/CMakeLists.txt | 1 +
.../CppCoreGuidelinesTidyModule.cpp | 3 +
clang-tools-extra/docs/ReleaseNotes.rst | 5 ++
.../cppcoreguidelines/avoid-bounds-errors.rst | 20 +
.../docs/clang-tidy/checks/list.rst | 1 +
.../cppcoreguidelines/avoid-bounds-errors.cpp | 66 +++
8 files changed, 209 insertions(+)
create mode 100644
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
create mode 100644
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
create mode 100644
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-bounds-errors.rst
create mode 100644
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-bounds-errors.cpp
diff --git
a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
new file mode 100644
index 0..524c21b5bdb81
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
@@ -0,0 +1,81 @@
+//===--- AvoidBoundsErrorsCheck.cpp - clang-tidy
--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "AvoidBoundsErrorsCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+#include
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::cppcoreguidelines {
+
+bool isApplicable(const QualType &Type) {
+ const auto TypeStr = Type.getAsString();
+ bool Result = false;
+ // Only check for containers in the std namespace
+ if (TypeStr.find("std::vector") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::array") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::deque") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::map") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::unordered_map") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::flat_map") != std::string::npos) {
+Result = true;
+ }
+ // TODO Add std::span with C++26
+ return Result;
+}
+
+void AvoidBoundsErrorsCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ callExpr(callee(cxxMethodDecl(hasName("operator[]")).bind("f")))
+ .bind("x"),
+ this);
+}
+
+void AvoidBoundsErrorsCheck::check(const MatchFinder::MatchResult &Result) {
+ const ASTContext &Context = *Result.Context;
+ const SourceManager &Source = Context.getSourceManager();
+ const auto *MatchedExpr = Result.Nodes.getNodeAs("x");
+ const auto *MatchedFunction = Result.Nodes.getNodeAs("f");
+ const auto Type = MatchedFunction->getThisType();
+ if (!isApplicable(Type)) {
+return;
+ }
+
+ // Get original code.
+ const SourceLocation b(MatchedExpr->getBeginLoc());
+ const SourceLocation e(MatchedExpr->getEndLoc());
+ const std::string OriginalCode =
+ Lexer::getSourceText(CharSourceRange::getTokenRange(b, e), Source,
+ getLangOpts())
+ .str();
+ const auto Range = SourceRange(b, e);
+
+ // Build replacement.
+ std::string NewCode = OriginalCode;
+ const auto BeginOpen = NewCode.find("[");
+ NewCode.replace(BeginOpen, 1, ".at(");
+ const auto BeginClose = NewCode.find("]");
+ NewCode.replace(BeginClose, 1, ")");
+
+ diag(MatchedExpr->getBeginLoc(), "Do not use operator[], use at() instead.")
+ << FixItHint::CreateReplacement(Range, NewCode);
+}
+
+} // namespace clang::tidy::cppcoreguidelines
diff --git
a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
new file mode 100644
index 0..f915729cd7bbe
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
@@ -0,0 +1,32 @@
+//===--- AvoidBoundsErrorsCheck.h - clang-tidy --*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef
LLV
[clang-tools-extra] Enforce SL.con.3: Add check to replace operator[] with at() [Cont.] (PR #95220)
https://github.com/PBHDK updated https://github.com/llvm/llvm-project/pull/95220
From 37292995de0c5aa87408586749795a97468d4725 Mon Sep 17 00:00:00 2001
From: Sebastian Wolf
Date: Wed, 17 Apr 2024 16:16:35 +0200
Subject: [PATCH 01/16] Enforce SL.con.3: Add check to replace operator[] with
at() on std containers
---
.../AvoidBoundsErrorsCheck.cpp| 81 +++
.../AvoidBoundsErrorsCheck.h | 32
.../cppcoreguidelines/CMakeLists.txt | 1 +
.../CppCoreGuidelinesTidyModule.cpp | 3 +
clang-tools-extra/docs/ReleaseNotes.rst | 5 ++
.../cppcoreguidelines/avoid-bounds-errors.rst | 20 +
.../docs/clang-tidy/checks/list.rst | 1 +
.../cppcoreguidelines/avoid-bounds-errors.cpp | 66 +++
8 files changed, 209 insertions(+)
create mode 100644
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
create mode 100644
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
create mode 100644
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-bounds-errors.rst
create mode 100644
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-bounds-errors.cpp
diff --git
a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
new file mode 100644
index 0..524c21b5bdb81
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
@@ -0,0 +1,81 @@
+//===--- AvoidBoundsErrorsCheck.cpp - clang-tidy
--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "AvoidBoundsErrorsCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+#include
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::cppcoreguidelines {
+
+bool isApplicable(const QualType &Type) {
+ const auto TypeStr = Type.getAsString();
+ bool Result = false;
+ // Only check for containers in the std namespace
+ if (TypeStr.find("std::vector") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::array") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::deque") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::map") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::unordered_map") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::flat_map") != std::string::npos) {
+Result = true;
+ }
+ // TODO Add std::span with C++26
+ return Result;
+}
+
+void AvoidBoundsErrorsCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ callExpr(callee(cxxMethodDecl(hasName("operator[]")).bind("f")))
+ .bind("x"),
+ this);
+}
+
+void AvoidBoundsErrorsCheck::check(const MatchFinder::MatchResult &Result) {
+ const ASTContext &Context = *Result.Context;
+ const SourceManager &Source = Context.getSourceManager();
+ const auto *MatchedExpr = Result.Nodes.getNodeAs("x");
+ const auto *MatchedFunction = Result.Nodes.getNodeAs("f");
+ const auto Type = MatchedFunction->getThisType();
+ if (!isApplicable(Type)) {
+return;
+ }
+
+ // Get original code.
+ const SourceLocation b(MatchedExpr->getBeginLoc());
+ const SourceLocation e(MatchedExpr->getEndLoc());
+ const std::string OriginalCode =
+ Lexer::getSourceText(CharSourceRange::getTokenRange(b, e), Source,
+ getLangOpts())
+ .str();
+ const auto Range = SourceRange(b, e);
+
+ // Build replacement.
+ std::string NewCode = OriginalCode;
+ const auto BeginOpen = NewCode.find("[");
+ NewCode.replace(BeginOpen, 1, ".at(");
+ const auto BeginClose = NewCode.find("]");
+ NewCode.replace(BeginClose, 1, ")");
+
+ diag(MatchedExpr->getBeginLoc(), "Do not use operator[], use at() instead.")
+ << FixItHint::CreateReplacement(Range, NewCode);
+}
+
+} // namespace clang::tidy::cppcoreguidelines
diff --git
a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
new file mode 100644
index 0..f915729cd7bbe
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
@@ -0,0 +1,32 @@
+//===--- AvoidBoundsErrorsCheck.h - clang-tidy --*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef
LLV
[clang-tools-extra] Enforce SL.con.3: Add check to replace operator[] with at() [Cont.] (PR #95220)
@@ -0,0 +1,123 @@
+//===--- PreferAtOverSubscriptOperatorCheck.cpp - clang-tidy
--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "PreferAtOverSubscriptOperatorCheck.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "llvm/ADT/StringRef.h"
+#include
+#include
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::cppcoreguidelines {
+
+static constexpr std::array DefaultExclusions = {
+llvm::StringRef("std::map"), llvm::StringRef("std::unordered_map"),
+llvm::StringRef("std::flat_map")};
+
+PreferAtOverSubscriptOperatorCheck::PreferAtOverSubscriptOperatorCheck(
+StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context) {
+
+ ExcludedClasses = clang::tidy::utils::options::parseStringList(
+ Options.get("ExcludeClasses", ""));
+ ExcludedClasses.insert(ExcludedClasses.end(), DefaultExclusions.begin(),
+ DefaultExclusions.end());
+}
+
+void PreferAtOverSubscriptOperatorCheck::storeOptions(
+ClangTidyOptions::OptionMap &Opts) {
+
+ if (ExcludedClasses.size() == DefaultExclusions.size()) {
+Options.store(Opts, "ExcludeClasses", "");
+return;
+ }
+
+ // Sum up the sizes of the defaults ( + semicolons), so we can remove them
+ // from the saved options
+ size_t DefaultsStringLength =
+ std::transform_reduce(DefaultExclusions.begin(), DefaultExclusions.end(),
+DefaultExclusions.size(), std::plus<>(),
+[](llvm::StringRef Name) { return Name.size(); });
+
+ std::string Serialized =
+ clang::tidy::utils::options::serializeStringList(ExcludedClasses);
+
+ Options.store(Opts, "ExcludeClasses",
+Serialized.substr(0, Serialized.size() -
DefaultsStringLength));
+}
+
+const CXXMethodDecl *findAlternative(const CXXRecordDecl *MatchedParent,
+ const CXXMethodDecl *MatchedOperator) {
+ for (const CXXMethodDecl *Method : MatchedParent->methods()) {
+const bool CorrectName = Method->getNameInfo().getAsString() == "at";
+if (!CorrectName)
+ continue;
+
+const bool SameReturnType =
+Method->getReturnType() == MatchedOperator->getReturnType();
+if (!SameReturnType)
+ continue;
+
+const bool SameNumberOfArguments =
+Method->getNumParams() == MatchedOperator->getNumParams();
+if (!SameNumberOfArguments)
+ continue;
+
+for (unsigned ArgInd = 0; ArgInd < Method->getNumParams(); ArgInd++) {
+ const bool SameArgType =
+ Method->parameters()[ArgInd]->getOriginalType() ==
+ MatchedOperator->parameters()[ArgInd]->getOriginalType();
+ if (!SameArgType)
+continue;
+}
+
+return Method;
+ }
+ return static_cast(nullptr);
+}
+
+void PreferAtOverSubscriptOperatorCheck::registerMatchers(MatchFinder *Finder)
{
+ // Need a callExpr here to match CXXOperatorCallExpr ``(&a)->operator[](0)``
+ // and CXXMemberCallExpr ``a[0]``.
+ Finder->addMatcher(
+ callExpr(
+ callee(
+ cxxMethodDecl(hasOverloadedOperatorName("[]")).bind("operator")),
+ callee(cxxMethodDecl(hasParent(
+ cxxRecordDecl(hasMethod(hasName("at"))).bind("parent")
+ .bind("caller"),
+ this);
+}
+
+void PreferAtOverSubscriptOperatorCheck::check(
+const MatchFinder::MatchResult &Result) {
+ const auto *MatchedExpr = Result.Nodes.getNodeAs("caller");
+ const auto *MatchedOperator =
+ Result.Nodes.getNodeAs("operator");
+ const auto *MatchedParent = Result.Nodes.getNodeAs("parent");
+
+ std::string ClassIdentifier = MatchedParent->getQualifiedNameAsString();
+
+ if (std::find(ExcludedClasses.begin(), ExcludedClasses.end(),
+ClassIdentifier) != ExcludedClasses.end())
+return;
PBHDK wrote:
Like so: 5098f65dc68e33ef854335e243b9570c4b0cf696?
https://github.com/llvm/llvm-project/pull/95220
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Enforce SL.con.3: Add check to replace operator[] with at() [Cont.] (PR #95220)
https://github.com/PBHDK updated https://github.com/llvm/llvm-project/pull/95220
From 37292995de0c5aa87408586749795a97468d4725 Mon Sep 17 00:00:00 2001
From: Sebastian Wolf
Date: Wed, 17 Apr 2024 16:16:35 +0200
Subject: [PATCH 01/17] Enforce SL.con.3: Add check to replace operator[] with
at() on std containers
---
.../AvoidBoundsErrorsCheck.cpp| 81 +++
.../AvoidBoundsErrorsCheck.h | 32
.../cppcoreguidelines/CMakeLists.txt | 1 +
.../CppCoreGuidelinesTidyModule.cpp | 3 +
clang-tools-extra/docs/ReleaseNotes.rst | 5 ++
.../cppcoreguidelines/avoid-bounds-errors.rst | 20 +
.../docs/clang-tidy/checks/list.rst | 1 +
.../cppcoreguidelines/avoid-bounds-errors.cpp | 66 +++
8 files changed, 209 insertions(+)
create mode 100644
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
create mode 100644
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
create mode 100644
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-bounds-errors.rst
create mode 100644
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-bounds-errors.cpp
diff --git
a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
new file mode 100644
index 0..524c21b5bdb81
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
@@ -0,0 +1,81 @@
+//===--- AvoidBoundsErrorsCheck.cpp - clang-tidy
--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "AvoidBoundsErrorsCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+#include
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::cppcoreguidelines {
+
+bool isApplicable(const QualType &Type) {
+ const auto TypeStr = Type.getAsString();
+ bool Result = false;
+ // Only check for containers in the std namespace
+ if (TypeStr.find("std::vector") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::array") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::deque") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::map") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::unordered_map") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::flat_map") != std::string::npos) {
+Result = true;
+ }
+ // TODO Add std::span with C++26
+ return Result;
+}
+
+void AvoidBoundsErrorsCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ callExpr(callee(cxxMethodDecl(hasName("operator[]")).bind("f")))
+ .bind("x"),
+ this);
+}
+
+void AvoidBoundsErrorsCheck::check(const MatchFinder::MatchResult &Result) {
+ const ASTContext &Context = *Result.Context;
+ const SourceManager &Source = Context.getSourceManager();
+ const auto *MatchedExpr = Result.Nodes.getNodeAs("x");
+ const auto *MatchedFunction = Result.Nodes.getNodeAs("f");
+ const auto Type = MatchedFunction->getThisType();
+ if (!isApplicable(Type)) {
+return;
+ }
+
+ // Get original code.
+ const SourceLocation b(MatchedExpr->getBeginLoc());
+ const SourceLocation e(MatchedExpr->getEndLoc());
+ const std::string OriginalCode =
+ Lexer::getSourceText(CharSourceRange::getTokenRange(b, e), Source,
+ getLangOpts())
+ .str();
+ const auto Range = SourceRange(b, e);
+
+ // Build replacement.
+ std::string NewCode = OriginalCode;
+ const auto BeginOpen = NewCode.find("[");
+ NewCode.replace(BeginOpen, 1, ".at(");
+ const auto BeginClose = NewCode.find("]");
+ NewCode.replace(BeginClose, 1, ")");
+
+ diag(MatchedExpr->getBeginLoc(), "Do not use operator[], use at() instead.")
+ << FixItHint::CreateReplacement(Range, NewCode);
+}
+
+} // namespace clang::tidy::cppcoreguidelines
diff --git
a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
new file mode 100644
index 0..f915729cd7bbe
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
@@ -0,0 +1,32 @@
+//===--- AvoidBoundsErrorsCheck.h - clang-tidy --*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef
LLV
[clang-tools-extra] Enforce SL.con.3: Add check to replace operator[] with at() [Cont.] (PR #95220)
@@ -0,0 +1,40 @@
+//===--- PreferAtOverSubscriptOperatorCheck.h - clang-tidy --*- C++
-*-===//
+//===--- PreferMemberInitializerCheck.h - clang-tidy *- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef
LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PREFERATOVERSUBSCRIPTOPERATORCHECK_H
+#define
LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PREFERATOVERSUBSCRIPTOPERATORCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::cppcoreguidelines {
+
+/// Enforce CPP core guidelines SL.con.3
PBHDK wrote:
Maybe we can just replace the Doxygen comment above with what's in the release
notes + check description right now:
> Flags the unsafe ``operator[]`` and suggests replacing it with ``at()``.
What do you think?
https://github.com/llvm/llvm-project/pull/95220
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Enforce SL.con.3: Add check to replace operator[] with at() [Cont.] (PR #95220)
PBHDK wrote: > Btw, I realize that this check is part of the [bounds > profile](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#SS-bounds) > (Bounds.4), so for consistency it should probably be named > `cppcoreguidelines-pro-bounds-...`. What do you think of: `cppcoreguidelines-pro-bounds-avoid-unchecked-container-accesses`? That's just me attempting to capture all of the previous suggestions in one name. > This becomes then the last remaining check to complete the bounds profile! That was actually @leunam99's and my main our motivation for getting involved with this work :-) https://github.com/llvm/llvm-project/pull/95220 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Enforce SL.con.3: Add check to replace operator[] with at() [Cont.] (PR #95220)
PBHDK wrote: > Btw, I realize that this check is part of the [bounds > profile](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#SS-bounds) > (Bounds.4), so for consistency it should probably be named > `cppcoreguidelines-pro-bounds-...`. This becomes then the last remaining > check to complete the bounds profile! Does the fact that this check implements the bounds profile need to be mentioned anywhere else? Or is it enough to have it be implicit, e.g., via the name. https://github.com/llvm/llvm-project/pull/95220 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Enforce SL.con.3: Add check to replace operator[] with at() [Cont.] (PR #95220)
https://github.com/PBHDK updated https://github.com/llvm/llvm-project/pull/95220
From 37292995de0c5aa87408586749795a97468d4725 Mon Sep 17 00:00:00 2001
From: Sebastian Wolf
Date: Wed, 17 Apr 2024 16:16:35 +0200
Subject: [PATCH 01/18] Enforce SL.con.3: Add check to replace operator[] with
at() on std containers
---
.../AvoidBoundsErrorsCheck.cpp| 81 +++
.../AvoidBoundsErrorsCheck.h | 32
.../cppcoreguidelines/CMakeLists.txt | 1 +
.../CppCoreGuidelinesTidyModule.cpp | 3 +
clang-tools-extra/docs/ReleaseNotes.rst | 5 ++
.../cppcoreguidelines/avoid-bounds-errors.rst | 20 +
.../docs/clang-tidy/checks/list.rst | 1 +
.../cppcoreguidelines/avoid-bounds-errors.cpp | 66 +++
8 files changed, 209 insertions(+)
create mode 100644
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
create mode 100644
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
create mode 100644
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-bounds-errors.rst
create mode 100644
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-bounds-errors.cpp
diff --git
a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
new file mode 100644
index 0..524c21b5bdb81
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
@@ -0,0 +1,81 @@
+//===--- AvoidBoundsErrorsCheck.cpp - clang-tidy
--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "AvoidBoundsErrorsCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+#include
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::cppcoreguidelines {
+
+bool isApplicable(const QualType &Type) {
+ const auto TypeStr = Type.getAsString();
+ bool Result = false;
+ // Only check for containers in the std namespace
+ if (TypeStr.find("std::vector") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::array") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::deque") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::map") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::unordered_map") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::flat_map") != std::string::npos) {
+Result = true;
+ }
+ // TODO Add std::span with C++26
+ return Result;
+}
+
+void AvoidBoundsErrorsCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ callExpr(callee(cxxMethodDecl(hasName("operator[]")).bind("f")))
+ .bind("x"),
+ this);
+}
+
+void AvoidBoundsErrorsCheck::check(const MatchFinder::MatchResult &Result) {
+ const ASTContext &Context = *Result.Context;
+ const SourceManager &Source = Context.getSourceManager();
+ const auto *MatchedExpr = Result.Nodes.getNodeAs("x");
+ const auto *MatchedFunction = Result.Nodes.getNodeAs("f");
+ const auto Type = MatchedFunction->getThisType();
+ if (!isApplicable(Type)) {
+return;
+ }
+
+ // Get original code.
+ const SourceLocation b(MatchedExpr->getBeginLoc());
+ const SourceLocation e(MatchedExpr->getEndLoc());
+ const std::string OriginalCode =
+ Lexer::getSourceText(CharSourceRange::getTokenRange(b, e), Source,
+ getLangOpts())
+ .str();
+ const auto Range = SourceRange(b, e);
+
+ // Build replacement.
+ std::string NewCode = OriginalCode;
+ const auto BeginOpen = NewCode.find("[");
+ NewCode.replace(BeginOpen, 1, ".at(");
+ const auto BeginClose = NewCode.find("]");
+ NewCode.replace(BeginClose, 1, ")");
+
+ diag(MatchedExpr->getBeginLoc(), "Do not use operator[], use at() instead.")
+ << FixItHint::CreateReplacement(Range, NewCode);
+}
+
+} // namespace clang::tidy::cppcoreguidelines
diff --git
a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
new file mode 100644
index 0..f915729cd7bbe
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
@@ -0,0 +1,32 @@
+//===--- AvoidBoundsErrorsCheck.h - clang-tidy --*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef
LLV
[clang-tools-extra] Enforce SL.con.3: Add check to replace operator[] with at() [Cont.] (PR #95220)
https://github.com/PBHDK updated https://github.com/llvm/llvm-project/pull/95220
From 37292995de0c5aa87408586749795a97468d4725 Mon Sep 17 00:00:00 2001
From: Sebastian Wolf
Date: Wed, 17 Apr 2024 16:16:35 +0200
Subject: [PATCH 01/19] Enforce SL.con.3: Add check to replace operator[] with
at() on std containers
---
.../AvoidBoundsErrorsCheck.cpp| 81 +++
.../AvoidBoundsErrorsCheck.h | 32
.../cppcoreguidelines/CMakeLists.txt | 1 +
.../CppCoreGuidelinesTidyModule.cpp | 3 +
clang-tools-extra/docs/ReleaseNotes.rst | 5 ++
.../cppcoreguidelines/avoid-bounds-errors.rst | 20 +
.../docs/clang-tidy/checks/list.rst | 1 +
.../cppcoreguidelines/avoid-bounds-errors.cpp | 66 +++
8 files changed, 209 insertions(+)
create mode 100644
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
create mode 100644
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
create mode 100644
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-bounds-errors.rst
create mode 100644
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-bounds-errors.cpp
diff --git
a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
new file mode 100644
index 0..524c21b5bdb81
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
@@ -0,0 +1,81 @@
+//===--- AvoidBoundsErrorsCheck.cpp - clang-tidy
--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "AvoidBoundsErrorsCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+#include
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::cppcoreguidelines {
+
+bool isApplicable(const QualType &Type) {
+ const auto TypeStr = Type.getAsString();
+ bool Result = false;
+ // Only check for containers in the std namespace
+ if (TypeStr.find("std::vector") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::array") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::deque") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::map") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::unordered_map") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::flat_map") != std::string::npos) {
+Result = true;
+ }
+ // TODO Add std::span with C++26
+ return Result;
+}
+
+void AvoidBoundsErrorsCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ callExpr(callee(cxxMethodDecl(hasName("operator[]")).bind("f")))
+ .bind("x"),
+ this);
+}
+
+void AvoidBoundsErrorsCheck::check(const MatchFinder::MatchResult &Result) {
+ const ASTContext &Context = *Result.Context;
+ const SourceManager &Source = Context.getSourceManager();
+ const auto *MatchedExpr = Result.Nodes.getNodeAs("x");
+ const auto *MatchedFunction = Result.Nodes.getNodeAs("f");
+ const auto Type = MatchedFunction->getThisType();
+ if (!isApplicable(Type)) {
+return;
+ }
+
+ // Get original code.
+ const SourceLocation b(MatchedExpr->getBeginLoc());
+ const SourceLocation e(MatchedExpr->getEndLoc());
+ const std::string OriginalCode =
+ Lexer::getSourceText(CharSourceRange::getTokenRange(b, e), Source,
+ getLangOpts())
+ .str();
+ const auto Range = SourceRange(b, e);
+
+ // Build replacement.
+ std::string NewCode = OriginalCode;
+ const auto BeginOpen = NewCode.find("[");
+ NewCode.replace(BeginOpen, 1, ".at(");
+ const auto BeginClose = NewCode.find("]");
+ NewCode.replace(BeginClose, 1, ")");
+
+ diag(MatchedExpr->getBeginLoc(), "Do not use operator[], use at() instead.")
+ << FixItHint::CreateReplacement(Range, NewCode);
+}
+
+} // namespace clang::tidy::cppcoreguidelines
diff --git
a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
new file mode 100644
index 0..f915729cd7bbe
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
@@ -0,0 +1,32 @@
+//===--- AvoidBoundsErrorsCheck.h - clang-tidy --*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef
LLV
[clang-tools-extra] Enforce SL.con.3: Add check to replace operator[] with at() [Cont.] (PR #95220)
https://github.com/PBHDK updated https://github.com/llvm/llvm-project/pull/95220
From 37292995de0c5aa87408586749795a97468d4725 Mon Sep 17 00:00:00 2001
From: Sebastian Wolf
Date: Wed, 17 Apr 2024 16:16:35 +0200
Subject: [PATCH 01/21] Enforce SL.con.3: Add check to replace operator[] with
at() on std containers
---
.../AvoidBoundsErrorsCheck.cpp| 81 +++
.../AvoidBoundsErrorsCheck.h | 32
.../cppcoreguidelines/CMakeLists.txt | 1 +
.../CppCoreGuidelinesTidyModule.cpp | 3 +
clang-tools-extra/docs/ReleaseNotes.rst | 5 ++
.../cppcoreguidelines/avoid-bounds-errors.rst | 20 +
.../docs/clang-tidy/checks/list.rst | 1 +
.../cppcoreguidelines/avoid-bounds-errors.cpp | 66 +++
8 files changed, 209 insertions(+)
create mode 100644
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
create mode 100644
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
create mode 100644
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-bounds-errors.rst
create mode 100644
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-bounds-errors.cpp
diff --git
a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
new file mode 100644
index 0..524c21b5bdb81
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
@@ -0,0 +1,81 @@
+//===--- AvoidBoundsErrorsCheck.cpp - clang-tidy
--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "AvoidBoundsErrorsCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+#include
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::cppcoreguidelines {
+
+bool isApplicable(const QualType &Type) {
+ const auto TypeStr = Type.getAsString();
+ bool Result = false;
+ // Only check for containers in the std namespace
+ if (TypeStr.find("std::vector") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::array") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::deque") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::map") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::unordered_map") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::flat_map") != std::string::npos) {
+Result = true;
+ }
+ // TODO Add std::span with C++26
+ return Result;
+}
+
+void AvoidBoundsErrorsCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ callExpr(callee(cxxMethodDecl(hasName("operator[]")).bind("f")))
+ .bind("x"),
+ this);
+}
+
+void AvoidBoundsErrorsCheck::check(const MatchFinder::MatchResult &Result) {
+ const ASTContext &Context = *Result.Context;
+ const SourceManager &Source = Context.getSourceManager();
+ const auto *MatchedExpr = Result.Nodes.getNodeAs("x");
+ const auto *MatchedFunction = Result.Nodes.getNodeAs("f");
+ const auto Type = MatchedFunction->getThisType();
+ if (!isApplicable(Type)) {
+return;
+ }
+
+ // Get original code.
+ const SourceLocation b(MatchedExpr->getBeginLoc());
+ const SourceLocation e(MatchedExpr->getEndLoc());
+ const std::string OriginalCode =
+ Lexer::getSourceText(CharSourceRange::getTokenRange(b, e), Source,
+ getLangOpts())
+ .str();
+ const auto Range = SourceRange(b, e);
+
+ // Build replacement.
+ std::string NewCode = OriginalCode;
+ const auto BeginOpen = NewCode.find("[");
+ NewCode.replace(BeginOpen, 1, ".at(");
+ const auto BeginClose = NewCode.find("]");
+ NewCode.replace(BeginClose, 1, ")");
+
+ diag(MatchedExpr->getBeginLoc(), "Do not use operator[], use at() instead.")
+ << FixItHint::CreateReplacement(Range, NewCode);
+}
+
+} // namespace clang::tidy::cppcoreguidelines
diff --git
a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
new file mode 100644
index 0..f915729cd7bbe
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
@@ -0,0 +1,32 @@
+//===--- AvoidBoundsErrorsCheck.h - clang-tidy --*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef
LLV
[clang-tools-extra] Enforce SL.con.3: Add check to replace operator[] with at() [Cont.] (PR #95220)
@@ -0,0 +1,40 @@
+//===--- PreferAtOverSubscriptOperatorCheck.h - clang-tidy --*- C++
-*-===//
+//===--- PreferMemberInitializerCheck.h - clang-tidy *- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef
LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PREFERATOVERSUBSCRIPTOPERATORCHECK_H
+#define
LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PREFERATOVERSUBSCRIPTOPERATORCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::cppcoreguidelines {
+
+/// Enforce CPP core guidelines SL.con.3
PBHDK wrote:
> Maybe we can just replace the Doxygen comment above with what's in the
> release notes + check description right now:
> > Flags the unsafe `operator[]` and suggests replacing it with `at()`.
>
> What do you think?
@5chmidti just giving this a polite bump in case it got lost 🙂
https://github.com/llvm/llvm-project/pull/95220
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Enforce SL.con.3: Add check to replace operator[] with at() (PR #90043)
PBHDK wrote: > I am adding @leunam99 and @PBHDK to the PR, who will contribute in the next > few weeks. Since we cannot push to this PR, we will be submitting a new PR with @sebwolf-de's + our work, correct, @EugeneZelenko, @PiotrZSL, @HerrCai0907? https://github.com/llvm/llvm-project/pull/90043 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Enforce SL.con.3: Add check to replace operator[] with at() [Cont.] (PR #95220)
@@ -0,0 +1,137 @@
+//===--- ProBoundsAvoidUncheckedContainerAccesses.cpp - clang-tidy
===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ProBoundsAvoidUncheckedContainerAccesses.h"
+#include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "llvm/ADT/StringRef.h"
+#include
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::cppcoreguidelines {
+
+static constexpr std::array SubscriptDefaultExclusions = {
+llvm::StringRef("::std::map"), llvm::StringRef("::std::unordered_map"),
+llvm::StringRef("::std::flat_map")};
+
+ProBoundsAvoidUncheckedContainerAccesses::
+ProBoundsAvoidUncheckedContainerAccesses(StringRef Name,
+ ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context) {
+
+ SubscriptExcludedClasses = clang::tidy::utils::options::parseStringList(
+ Options.get("ExcludeClasses", ""));
+ SubscriptExcludedClasses.insert(SubscriptExcludedClasses.end(),
+ SubscriptDefaultExclusions.begin(),
+ SubscriptDefaultExclusions.end());
+}
+
+void ProBoundsAvoidUncheckedContainerAccesses::storeOptions(
+ClangTidyOptions::OptionMap &Opts) {
+
+ if (SubscriptExcludedClasses.size() == SubscriptDefaultExclusions.size()) {
+Options.store(Opts, "ExcludeClasses", "");
+return;
+ }
+
+ // Sum up the sizes of the defaults ( + semicolons), so we can remove them
+ // from the saved options
+ size_t DefaultsStringLength = std::transform_reduce(
+ SubscriptDefaultExclusions.begin(), SubscriptDefaultExclusions.end(),
+ SubscriptDefaultExclusions.size(), std::plus<>(),
+ [](llvm::StringRef Name) { return Name.size(); });
+
+ std::string Serialized = clang::tidy::utils::options::serializeStringList(
+ SubscriptExcludedClasses);
+
+ Options.store(Opts, "ExcludeClasses",
+Serialized.substr(0, Serialized.size() -
DefaultsStringLength));
+}
+
+static const CXXMethodDecl *
+findAlternative(const CXXMethodDecl *MatchedOperator) {
+ const CXXRecordDecl *Parent = MatchedOperator->getParent();
+ const QualType SubscriptThisObjType =
+ MatchedOperator->getFunctionObjectParameterReferenceType();
+
+ for (const CXXMethodDecl *Method : Parent->methods()) {
+// Require 'Method' to be as accessible as 'MatchedOperator' or more
+if (MatchedOperator->getAccess() < Method->getAccess())
+ continue;
+
+if (MatchedOperator->isConst() != Method->isConst())
+ continue;
+
+const QualType AtThisObjType =
+Method->getFunctionObjectParameterReferenceType();
+if (SubscriptThisObjType != AtThisObjType)
+ continue;
+
+const bool CorrectName = Method->getNameInfo().getAsString() == "at";
+if (!CorrectName)
+ continue;
+
+const bool SameReturnType =
+Method->getReturnType() == MatchedOperator->getReturnType();
+if (!SameReturnType)
+ continue;
+
+const bool SameNumberOfArguments =
+Method->getNumParams() == MatchedOperator->getNumParams();
+if (!SameNumberOfArguments)
+ continue;
+
+for (unsigned ArgInd = 0; ArgInd < Method->getNumParams(); ArgInd++) {
+ const bool SameArgType =
+ Method->parameters()[ArgInd]->getOriginalType() ==
+ MatchedOperator->parameters()[ArgInd]->getOriginalType();
+ if (!SameArgType)
+continue;
+}
+
+return Method;
+ }
+ return nullptr;
+}
+
+void ProBoundsAvoidUncheckedContainerAccesses::registerMatchers(
+MatchFinder *Finder) {
+ Finder->addMatcher(
+ mapAnyOf(cxxOperatorCallExpr, cxxMemberCallExpr)
+ .with(callee(cxxMethodDecl(hasOverloadedOperatorName("[]"),
+ ofClass(cxxRecordDecl(hasMethod(
+ cxxMethodDecl(hasName("at"),
+ unless(matchers::matchesAnyListedName(
+ SubscriptExcludedClasses)))
+ .bind("operator")))
+ .bind("caller"),
+ this);
+}
+
+void ProBoundsAvoidUncheckedContainerAccesses::check(
+const MatchFinder::MatchResult &Result) {
+ const auto *MatchedOperator =
+ Result.Nodes.getNodeAs("operator");
+ const CXXMethodDecl *Alternative = findAlternative(MatchedOperator);
+
+ if (!Alternative)
+return;
+
+ const auto *MatchedExpr = Result.Nodes.getNodeAs("caller");
+
+ diag(MatchedExpr->getBeginLoc(),
+ "found possibly unsafe 'operator[]', consider using 'at()' instead")
+ << MatchedExpr->getSourceRange();
+ diag(Alternative->getBeginLoc(), "alternative 'at()' defined here",
+
[clang-tools-extra] Enforce SL.con.3: Add check to replace operator[] with at() [Cont.] (PR #95220)
@@ -0,0 +1,137 @@
+//===--- ProBoundsAvoidUncheckedContainerAccesses.cpp - clang-tidy
===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ProBoundsAvoidUncheckedContainerAccesses.h"
+#include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "llvm/ADT/StringRef.h"
+#include
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::cppcoreguidelines {
+
+static constexpr std::array SubscriptDefaultExclusions = {
+llvm::StringRef("::std::map"), llvm::StringRef("::std::unordered_map"),
+llvm::StringRef("::std::flat_map")};
+
+ProBoundsAvoidUncheckedContainerAccesses::
+ProBoundsAvoidUncheckedContainerAccesses(StringRef Name,
+ ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context) {
+
+ SubscriptExcludedClasses = clang::tidy::utils::options::parseStringList(
+ Options.get("ExcludeClasses", ""));
+ SubscriptExcludedClasses.insert(SubscriptExcludedClasses.end(),
+ SubscriptDefaultExclusions.begin(),
+ SubscriptDefaultExclusions.end());
+}
+
+void ProBoundsAvoidUncheckedContainerAccesses::storeOptions(
+ClangTidyOptions::OptionMap &Opts) {
+
+ if (SubscriptExcludedClasses.size() == SubscriptDefaultExclusions.size()) {
+Options.store(Opts, "ExcludeClasses", "");
+return;
+ }
+
+ // Sum up the sizes of the defaults ( + semicolons), so we can remove them
+ // from the saved options
+ size_t DefaultsStringLength = std::transform_reduce(
+ SubscriptDefaultExclusions.begin(), SubscriptDefaultExclusions.end(),
+ SubscriptDefaultExclusions.size(), std::plus<>(),
+ [](llvm::StringRef Name) { return Name.size(); });
+
+ std::string Serialized = clang::tidy::utils::options::serializeStringList(
+ SubscriptExcludedClasses);
+
+ Options.store(Opts, "ExcludeClasses",
+Serialized.substr(0, Serialized.size() -
DefaultsStringLength));
+}
+
+static const CXXMethodDecl *
+findAlternative(const CXXMethodDecl *MatchedOperator) {
+ const CXXRecordDecl *Parent = MatchedOperator->getParent();
+ const QualType SubscriptThisObjType =
+ MatchedOperator->getFunctionObjectParameterReferenceType();
+
+ for (const CXXMethodDecl *Method : Parent->methods()) {
+// Require 'Method' to be as accessible as 'MatchedOperator' or more
+if (MatchedOperator->getAccess() < Method->getAccess())
+ continue;
+
+if (MatchedOperator->isConst() != Method->isConst())
+ continue;
+
+const QualType AtThisObjType =
+Method->getFunctionObjectParameterReferenceType();
+if (SubscriptThisObjType != AtThisObjType)
+ continue;
+
+const bool CorrectName = Method->getNameInfo().getAsString() == "at";
PBHDK wrote:
If I use `const bool CorrectName = Method->getName() == "at";` instead, it
immediately triggers the assert in `getName()` when running the tests.
Was your suggestion to only use `getName()` when we can and fall back to
`getNameInfo()->getAsString()` otherwise?
https://github.com/llvm/llvm-project/pull/95220
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Enforce SL.con.3: Add check to replace operator[] with at() [Cont.] (PR #95220)
@@ -0,0 +1,124 @@
+//===--- ProBoundsAvoidUncheckedContainerAccesses.cpp - clang-tidy
===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ProBoundsAvoidUncheckedContainerAccesses.h"
+#include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "llvm/ADT/StringRef.h"
+#include
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::cppcoreguidelines {
+
+static constexpr std::array SubscriptDefaultExclusions = {
+llvm::StringRef("::std::map"), llvm::StringRef("::std::unordered_map"),
+llvm::StringRef("::std::flat_map")};
PBHDK wrote:
> Should be fine, other option would be to check if at and [] parameters as
> integers, and do not come from template.
In this case, we would just iterate over all arguments of `operator[]` and
`at()` call - in C++ 23, there could be multiple - and check if they're
integers and don't come from a template. This would eliminate the need for
hard-coded types, correct? However, we would still keep the exclusion list
mechanism so that users can exclude their custom range-checked subscript
operator overloads. Do you think this can be easily implemented like this?
> Or other option even better instead of having exclusions list, have inclusion
> list. and just configure check for std::array, std::vector, std::span,
> gsl::span, std::deque. That would be better, than trying to exclude some not
> related types.
There'd still be hard-coded types in this case, but we drastically reduce the
risk of false positives, correct? However, we would still be missing a lot of
cases in custom libraries or user-defined overloads of the subscript operator,
which were explicitly requested in other discussions, IIRC. Would you still
prefer this over the first suggestion, even though we would be missing cases?
https://github.com/llvm/llvm-project/pull/95220
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Enforce SL.con.3: Add check to replace operator[] with at() [Cont.] (PR #95220)
@@ -0,0 +1,124 @@
+//===--- ProBoundsAvoidUncheckedContainerAccesses.cpp - clang-tidy
===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ProBoundsAvoidUncheckedContainerAccesses.h"
+#include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "llvm/ADT/StringRef.h"
+#include
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::cppcoreguidelines {
+
+static constexpr std::array SubscriptDefaultExclusions = {
+llvm::StringRef("::std::map"), llvm::StringRef("::std::unordered_map"),
+llvm::StringRef("::std::flat_map")};
+
+ProBoundsAvoidUncheckedContainerAccesses::
+ProBoundsAvoidUncheckedContainerAccesses(StringRef Name,
+ ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context) {
+
+ SubscriptExcludedClasses = clang::tidy::utils::options::parseStringList(
PBHDK wrote:
If we were just to rely on the `const` check, we would also be excluding
std::vectors for instance, because their subscript operator might not be
`const` either, as it returns a reference. Correct?
https://github.com/llvm/llvm-project/pull/95220
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Enforce SL.con.3: Add check to replace operator[] with at() [Cont.] (PR #95220)
https://github.com/PBHDK edited https://github.com/llvm/llvm-project/pull/95220 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Enforce SL.con.3: Add check to replace operator[] with at() [Cont.] (PR #95220)
@@ -0,0 +1,40 @@
+//===--- ProBoundsAvoidUncheckedContainerAccesses.h - clang-tidy *- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef
LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PRO_BOUNDS_AVOID_UNCHECKED_CONTAINER_ACCESSES_H
+#define
LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PRO_BOUNDS_AVOID_UNCHECKED_CONTAINER_ACCESSES_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::cppcoreguidelines {
+
+/// Enforce CPP core guidelines SL.con.3
+///
+/// See
+///
https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#slcon3-avoid-bounds-errors
+/// For the user-facing documentation see:
+///
http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/pro-bounds-avoid-unchecked-container-accesses.html
+class ProBoundsAvoidUncheckedContainerAccesses : public ClangTidyCheck {
+public:
+ ProBoundsAvoidUncheckedContainerAccesses(StringRef Name,
+ ClangTidyContext *Context);
+ bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+return LangOpts.CPlusPlus;
+ }
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+
PBHDK wrote:
> exclude implicit code via TK_IgnoreUnlessSpelledInSource here, like other
> checks.
This currently breaks the following test case. How should we handle this?
```
std::array a;
template int TestTemplate(T t){
return t[0];
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: found possibly unsafe
'operator[]', consider using 'at()' instead
[cppcoreguidelines-pro-bounds-avoid-unchecked-container-accesses]
}
auto v = TestTemplate<>(a);
```
https://github.com/llvm/llvm-project/pull/95220
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Enforce SL.con.3: Add check to replace operator[] with at() [Cont.] (PR #95220)
@@ -0,0 +1,137 @@
+//===--- ProBoundsAvoidUncheckedContainerAccesses.cpp - clang-tidy
===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ProBoundsAvoidUncheckedContainerAccesses.h"
+#include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "llvm/ADT/StringRef.h"
+#include
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::cppcoreguidelines {
+
+static constexpr std::array SubscriptDefaultExclusions = {
+llvm::StringRef("::std::map"), llvm::StringRef("::std::unordered_map"),
+llvm::StringRef("::std::flat_map")};
+
+ProBoundsAvoidUncheckedContainerAccesses::
+ProBoundsAvoidUncheckedContainerAccesses(StringRef Name,
+ ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context) {
+
+ SubscriptExcludedClasses = clang::tidy::utils::options::parseStringList(
+ Options.get("ExcludeClasses", ""));
+ SubscriptExcludedClasses.insert(SubscriptExcludedClasses.end(),
+ SubscriptDefaultExclusions.begin(),
+ SubscriptDefaultExclusions.end());
+}
+
+void ProBoundsAvoidUncheckedContainerAccesses::storeOptions(
+ClangTidyOptions::OptionMap &Opts) {
+
+ if (SubscriptExcludedClasses.size() == SubscriptDefaultExclusions.size()) {
+Options.store(Opts, "ExcludeClasses", "");
+return;
+ }
+
+ // Sum up the sizes of the defaults ( + semicolons), so we can remove them
+ // from the saved options
+ size_t DefaultsStringLength = std::transform_reduce(
+ SubscriptDefaultExclusions.begin(), SubscriptDefaultExclusions.end(),
+ SubscriptDefaultExclusions.size(), std::plus<>(),
+ [](llvm::StringRef Name) { return Name.size(); });
+
+ std::string Serialized = clang::tidy::utils::options::serializeStringList(
+ SubscriptExcludedClasses);
+
+ Options.store(Opts, "ExcludeClasses",
+Serialized.substr(0, Serialized.size() -
DefaultsStringLength));
+}
+
+static const CXXMethodDecl *
+findAlternative(const CXXMethodDecl *MatchedOperator) {
+ const CXXRecordDecl *Parent = MatchedOperator->getParent();
+ const QualType SubscriptThisObjType =
+ MatchedOperator->getFunctionObjectParameterReferenceType();
+
+ for (const CXXMethodDecl *Method : Parent->methods()) {
+// Require 'Method' to be as accessible as 'MatchedOperator' or more
+if (MatchedOperator->getAccess() < Method->getAccess())
+ continue;
+
+if (MatchedOperator->isConst() != Method->isConst())
+ continue;
+
+const QualType AtThisObjType =
+Method->getFunctionObjectParameterReferenceType();
+if (SubscriptThisObjType != AtThisObjType)
+ continue;
+
+const bool CorrectName = Method->getNameInfo().getAsString() == "at";
+if (!CorrectName)
+ continue;
+
+const bool SameReturnType =
+Method->getReturnType() == MatchedOperator->getReturnType();
+if (!SameReturnType)
+ continue;
+
+const bool SameNumberOfArguments =
+Method->getNumParams() == MatchedOperator->getNumParams();
+if (!SameNumberOfArguments)
+ continue;
+
+for (unsigned ArgInd = 0; ArgInd < Method->getNumParams(); ArgInd++) {
+ const bool SameArgType =
+ Method->parameters()[ArgInd]->getOriginalType() ==
+ MatchedOperator->parameters()[ArgInd]->getOriginalType();
+ if (!SameArgType)
+continue;
+}
+
+return Method;
+ }
+ return nullptr;
+}
+
+void ProBoundsAvoidUncheckedContainerAccesses::registerMatchers(
+MatchFinder *Finder) {
+ Finder->addMatcher(
+ mapAnyOf(cxxOperatorCallExpr, cxxMemberCallExpr)
+ .with(callee(cxxMethodDecl(hasOverloadedOperatorName("[]"),
+ ofClass(cxxRecordDecl(hasMethod(
+ cxxMethodDecl(hasName("at"),
+ unless(matchers::matchesAnyListedName(
+ SubscriptExcludedClasses)))
+ .bind("operator")))
+ .bind("caller"),
+ this);
+}
+
+void ProBoundsAvoidUncheckedContainerAccesses::check(
+const MatchFinder::MatchResult &Result) {
+ const auto *MatchedOperator =
+ Result.Nodes.getNodeAs("operator");
+ const CXXMethodDecl *Alternative = findAlternative(MatchedOperator);
+
+ if (!Alternative)
+return;
+
+ const auto *MatchedExpr = Result.Nodes.getNodeAs("caller");
+
+ diag(MatchedExpr->getBeginLoc(),
+ "found possibly unsafe 'operator[]', consider using 'at()' instead")
+ << MatchedExpr->getSourceRange();
PBHDK wrote:
Like this?
```
4:13: warning: found possibly
[clang-tools-extra] Enforce SL.con.3: Add check to replace operator[] with at() [Cont.] (PR #95220)
@@ -160,6 +160,11 @@ New checks Replaces nested ``std::min`` and ``std::max`` calls with an initializer list where applicable. +- New :doc:`cppcoreguidelines-pro-bounds-avoid-unchecked-container-accesses PBHDK wrote: Are you referring to a double "it" somewhere? I am not seeing the error in line 163 or line 150? https://github.com/llvm/llvm-project/pull/95220 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Enforce SL.con.3: Add check to replace operator[] with at() [Cont.] (PR #95220)
https://github.com/PBHDK updated https://github.com/llvm/llvm-project/pull/95220
From 37292995de0c5aa87408586749795a97468d4725 Mon Sep 17 00:00:00 2001
From: Sebastian Wolf
Date: Wed, 17 Apr 2024 16:16:35 +0200
Subject: [PATCH 01/22] Enforce SL.con.3: Add check to replace operator[] with
at() on std containers
---
.../AvoidBoundsErrorsCheck.cpp| 81 +++
.../AvoidBoundsErrorsCheck.h | 32
.../cppcoreguidelines/CMakeLists.txt | 1 +
.../CppCoreGuidelinesTidyModule.cpp | 3 +
clang-tools-extra/docs/ReleaseNotes.rst | 5 ++
.../cppcoreguidelines/avoid-bounds-errors.rst | 20 +
.../docs/clang-tidy/checks/list.rst | 1 +
.../cppcoreguidelines/avoid-bounds-errors.cpp | 66 +++
8 files changed, 209 insertions(+)
create mode 100644
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
create mode 100644
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
create mode 100644
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-bounds-errors.rst
create mode 100644
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-bounds-errors.cpp
diff --git
a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
new file mode 100644
index 0..524c21b5bdb81
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
@@ -0,0 +1,81 @@
+//===--- AvoidBoundsErrorsCheck.cpp - clang-tidy
--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "AvoidBoundsErrorsCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+#include
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::cppcoreguidelines {
+
+bool isApplicable(const QualType &Type) {
+ const auto TypeStr = Type.getAsString();
+ bool Result = false;
+ // Only check for containers in the std namespace
+ if (TypeStr.find("std::vector") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::array") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::deque") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::map") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::unordered_map") != std::string::npos) {
+Result = true;
+ }
+ if (TypeStr.find("std::flat_map") != std::string::npos) {
+Result = true;
+ }
+ // TODO Add std::span with C++26
+ return Result;
+}
+
+void AvoidBoundsErrorsCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ callExpr(callee(cxxMethodDecl(hasName("operator[]")).bind("f")))
+ .bind("x"),
+ this);
+}
+
+void AvoidBoundsErrorsCheck::check(const MatchFinder::MatchResult &Result) {
+ const ASTContext &Context = *Result.Context;
+ const SourceManager &Source = Context.getSourceManager();
+ const auto *MatchedExpr = Result.Nodes.getNodeAs("x");
+ const auto *MatchedFunction = Result.Nodes.getNodeAs("f");
+ const auto Type = MatchedFunction->getThisType();
+ if (!isApplicable(Type)) {
+return;
+ }
+
+ // Get original code.
+ const SourceLocation b(MatchedExpr->getBeginLoc());
+ const SourceLocation e(MatchedExpr->getEndLoc());
+ const std::string OriginalCode =
+ Lexer::getSourceText(CharSourceRange::getTokenRange(b, e), Source,
+ getLangOpts())
+ .str();
+ const auto Range = SourceRange(b, e);
+
+ // Build replacement.
+ std::string NewCode = OriginalCode;
+ const auto BeginOpen = NewCode.find("[");
+ NewCode.replace(BeginOpen, 1, ".at(");
+ const auto BeginClose = NewCode.find("]");
+ NewCode.replace(BeginClose, 1, ")");
+
+ diag(MatchedExpr->getBeginLoc(), "Do not use operator[], use at() instead.")
+ << FixItHint::CreateReplacement(Range, NewCode);
+}
+
+} // namespace clang::tidy::cppcoreguidelines
diff --git
a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
new file mode 100644
index 0..f915729cd7bbe
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
@@ -0,0 +1,32 @@
+//===--- AvoidBoundsErrorsCheck.h - clang-tidy --*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef
LLV
[clang-tools-extra] Enforce SL.con.3: Add check to replace operator[] with at() [Cont.] (PR #95220)
PBHDK wrote: > The name sounds nice. Thanks. > Please address the concerns here and in #90043 to always suggest `at` as > well. An option to enable suggesting and providing a fix to use `at` (or > `gsl::at`), like others have suggested, sounds like the best option. Thank you for taking another look as well as your comments. On it! https://github.com/llvm/llvm-project/pull/95220 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Enforce SL.con.3: Add check to replace operator[] with at() [Cont.] (PR #95220)
paulhdk wrote: Hi all, just giving this a polite bump. Please do let us know if you have any questions or if there's anything we can do to make reviewing easier. https://github.com/llvm/llvm-project/pull/95220 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Enforce SL.con.3: Add check to replace operator[] with at() [Cont.] (PR #95220)
@@ -0,0 +1,227 @@
+// RUN: %check_clang_tidy -std=c++2b -check-suffix=DEFAULT %s \
+// RUN: cppcoreguidelines-pro-bounds-avoid-unchecked-container-accesses %t -- \
+// RUN: -config='{CheckOptions:
{cppcoreguidelines-pro-bounds-avoid-unchecked-container-accesses.ExcludeClasses:
"::ExcludedClass1;::ExcludedClass2"}}'
+
+// RUN: %check_clang_tidy -std=c++2b -check-suffix=AT %s \
+// RUN: cppcoreguidelines-pro-bounds-avoid-unchecked-container-accesses %t -- \
+// RUN: -config='{CheckOptions:
{cppcoreguidelines-pro-bounds-avoid-unchecked-container-accesses.ExcludeClasses:
"::ExcludedClass1;::ExcludedClass2", \
+// RUN:
cppcoreguidelines-pro-bounds-avoid-unchecked-container-accesses.SubscriptFixMode:
at}}'
+
+// RUN: %check_clang_tidy -std=c++2b -check-suffix=FUNC %s \
+// RUN: cppcoreguidelines-pro-bounds-avoid-unchecked-container-accesses %t -- \
+// RUN: -config='{CheckOptions:
{cppcoreguidelines-pro-bounds-avoid-unchecked-container-accesses.ExcludeClasses:
"::ExcludedClass1;::ExcludedClass2", \
+// RUN:
cppcoreguidelines-pro-bounds-avoid-unchecked-container-accesses.SubscriptFixMode:
function, \
+// RUN:
cppcoreguidelines-pro-bounds-avoid-unchecked-container-accesses.SubscriptFixFunction:
"f"}}'
+
+namespace std {
+ template
+ struct array {
+T operator[](unsigned i) {
+ return T{1};
+}
+T operator[]() {
+ return T{1};
+}
+T at(unsigned i) {
+ return T{1};
+}
+T at() {
+ return T{1};
+}
+ };
+
+ template
+ struct map {
+T operator[](unsigned i) {
+ return T{1};
+}
+T at(unsigned i) {
+ return T{1};
+}
+ };
+
+ template
+ struct unique_ptr {
+T operator[](unsigned i) {
+ return T{1};
+}
+ };
+
+ template
+ struct span {
+T operator[](unsigned i) {
+ return T{1};
+}
+ };
+} // namespace std
+
+namespace json {
+ template
+ struct node{
+T operator[](unsigned i) {
+ return T{1};
+}
+ };
+} // namespace json
+
+struct SubClass : std::array {};
+
+class ExcludedClass1 {
+ public:
+int operator[](unsigned i) {
+ return 1;
+}
+int at(unsigned i) {
+ return 1;
+}
+};
+
+class ExcludedClass2 {
+ public:
+int operator[](unsigned i) {
+ return 1;
+}
+int at(unsigned i) {
+ return 1;
+}
+};
+
+template int f(T, unsigned){ return 0;}
+template int f(T){ return 0;}
+
+std::array a;
+
+auto b = a[0];
+// CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:11: warning: possibly unsafe
'operator[]', consider bound-safe alternatives
[cppcoreguidelines-pro-bounds-avoid-unchecked-container-accesses]
+// CHECK-FIXES-AT: auto b = a.at(0);
+// CHECK-FIXES-FUNC: auto b = f(a, 0);
+
+auto b23 = a[];
paulhdk wrote:
https://github.com/llvm/llvm-project/blob/a7be15062a853218f5b577d0c4973d96b66682ee/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-avoid-unchecked-container-accesses.cpp#L16-L31
We overload `std::array`'s subscript operator in line 22 s.t. that it can be
called without an index. This should be legal as of C++23. See
[p2128r5](https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p2128r5.pdf).
Also, clang-tidy is giving me the respective warning here:
https://github.com/llvm/llvm-project/blob/a7be15062a853218f5b577d0c4973d96b66682ee/clang/test/Parser/cxx2b-subscript.cpp?plain=1#L13-L15
What do you think?
https://github.com/llvm/llvm-project/pull/95220
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Enforce SL.con.3: Add check to replace operator[] with at() [Cont.] (PR #95220)
@@ -0,0 +1,124 @@
+//===--- ProBoundsAvoidUncheckedContainerAccesses.cpp - clang-tidy
===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ProBoundsAvoidUncheckedContainerAccesses.h"
+#include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "llvm/ADT/StringRef.h"
+#include
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::cppcoreguidelines {
+
+static constexpr std::array SubscriptDefaultExclusions = {
+llvm::StringRef("::std::map"), llvm::StringRef("::std::unordered_map"),
+llvm::StringRef("::std::flat_map")};
+
+ProBoundsAvoidUncheckedContainerAccesses::
+ProBoundsAvoidUncheckedContainerAccesses(StringRef Name,
+ ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context) {
+
+ SubscriptExcludedClasses = clang::tidy::utils::options::parseStringList(
+ Options.get("ExcludeClasses", ""));
+ SubscriptExcludedClasses.insert(SubscriptExcludedClasses.end(),
+ SubscriptDefaultExclusions.begin(),
+ SubscriptDefaultExclusions.end());
+}
+
+void ProBoundsAvoidUncheckedContainerAccesses::storeOptions(
+ClangTidyOptions::OptionMap &Opts) {
+
+ if (SubscriptExcludedClasses.size() == SubscriptDefaultExclusions.size()) {
+Options.store(Opts, "ExcludeClasses", "");
+return;
+ }
+
+ // Sum up the sizes of the defaults ( + semicolons), so we can remove them
+ // from the saved options
+ size_t DefaultsStringLength = std::transform_reduce(
+ SubscriptDefaultExclusions.begin(), SubscriptDefaultExclusions.end(),
+ SubscriptDefaultExclusions.size(), std::plus<>(),
+ [](llvm::StringRef Name) { return Name.size(); });
+
+ std::string Serialized = clang::tidy::utils::options::serializeStringList(
+ SubscriptExcludedClasses);
+
+ Options.store(Opts, "ExcludeClasses",
+Serialized.substr(0, Serialized.size() -
DefaultsStringLength));
+}
+
+const CXXMethodDecl *findAlternative(const CXXRecordDecl *MatchedParent,
paulhdk wrote:
I'm unsure what you are referring to. Does this still apply to the current
version of the code?
https://github.com/llvm/llvm-project/pull/95220
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Enforce SL.con.3: Add check to replace operator[] with at() [Cont.] (PR #95220)
paulhdk wrote: Sorry for letting this sit for long! I've addressed the most recent comments. Based on what @leunam99 wrote above, the following questions are still unresolved: * It is still unclear to us how templates should be addressed when suggesting fixes. For instance, what should happen in this case: https://github.com/llvm/llvm-project/blob/98483ae1581c9a12fc7b4c8b5b64330db8292c29/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-avoid-unchecked-container-accesses.cpp?plain=1#L176-L184 * Should we worry about the cases where the subscript operator can have 0 parameters or more than 1 parameter in C++23? At the moment we’re accounting for the case where there is no parameter, but don't explicitly handle multiple parameters. * As @carlosgalvezp noted, there are still open comments. I’ve resolved them or responded to those that we’re uncertain about. @PiotrZSL, it would be great if you could have another look! Sorry again, for taking so long. https://github.com/llvm/llvm-project/pull/95220 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Enforce SL.con.3: Add check to replace operator[] with at() [Cont.] (PR #95220)
paulhdk wrote: Gentle bump. Thank you for taking a look! https://github.com/llvm/llvm-project/pull/95220 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
