[clang-tools-extra] Enforce SL.con.3: Add check to replace operator[] with at() [Cont.] (PR #95220)
Paul =?utf-8?q?Heidekrüger?= , Paul =?utf-8?q?Heidekrüger?= , Paul =?utf-8?q?Heidekrüger?= , Paul =?utf-8?q?Heidekrüger?= , Paul =?utf-8?q?Heidekrüger?= Message-ID: In-Reply-To: leunam99 wrote: > I still think "AvoidBoundErrors" is the best name since it maps exactly to > what the guidelines call it. For me, this sounds to general for what the check currently does. The Enforcement section at the end of the rule SL.con.3 states: > Issue a diagnostic for any call to a standard-library function that is not > bounds-checked. The guidelines do not provide a list of such functions and we are not familliar enough with the standard library to create a comprehensive list ourself, so at this time, we are unable to implement the full rule. 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)
@@ -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;
leunam99 wrote:
We had a look at the standard library; the same applies to `std::unordered_map`
and `std::flat_map` too, correct? So, by default, we'd be excluding `std::map`,
`std::unordered_map`, and `std::flat_map`. Can you think of any others?
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)
@@ -0,0 +1,66 @@
+namespace std {
+ template
+ struct array {
+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
+
+
+// RUN: %check_clang_tidy %s cppcoreguidelines-avoid-bounds-errors %t
+std::array a;
leunam99 wrote:
As suggested, we added a test where the object is a template parameter and the
method is called once with a class that has `at()` and once with a class that
has not, but we are not sure what the best behaviour is. Should we
- not warn, because the fix suggested in the message will lead the other
instantiation to 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?
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)
Paul =?utf-8?q?Heidekrüger?= , Paul =?utf-8?q?Heidekrüger?= , Paul =?utf-8?q?Heidekrüger?= , Paul =?utf-8?q?Heidekrüger?= , Paul =?utf-8?q?Heidekrüger?= , Paul =?utf-8?q?Heidekrüger?= , Paul =?utf-8?q?Heidekrüger?= , Paul =?utf-8?q?Heidekrüger?= , Paul =?utf-8?q?Heidekrüger?= , Paul =?utf-8?q?Heidekrüger?= , Paul =?utf-8?q?Heidekrüger?= , Paul =?utf-8?q?Heidekrüger?= , Paul =?utf-8?q?Heidekrüger?= , Paul =?utf-8?q?Heidekrüger?= , Paul =?utf-8?q?Heidekrüger?= , Paul =?utf-8?q?Heidekrüger?= , Paul =?utf-8?q?Heidekrüger?= , Paul =?utf-8?q?Heidekrüger?= , Paul =?utf-8?q?Heidekrüger?= , Paul =?utf-8?q?Heidekrüger?= , Paul =?utf-8?q?Heidekrüger?= , Paul =?utf-8?q?Heidekrüger?= , Paul =?utf-8?q?Heidekrüger?= , Paul =?utf-8?q?Heidekrüger?= , Paul =?utf-8?q?Heidekrüger?= , Paul =?utf-8?q?Heidekrüger?= , Paul =?utf-8?q?Heidekrüger?= , Paul =?utf-8?q?Heidekrüger?= , Paul =?utf-8?q?Heidekrüger?= , Paul =?utf-8?q?Heidekrüger?= , Paul =?utf-8?q?Heidekrüger?= , Paul =?utf-8?q?Heidekrüger?= , Paul =?utf-8?q?Heidekrüger?= , Paul =?utf-8?q?Heidekrüger?= , Paul =?utf-8?q?Heidekrüger?= , Paul =?utf-8?q?Heidekrüger?= , Paul =?utf-8?q?Heidekrüger?= , Paul =?utf-8?q?Heidekrüger?= , Paul =?utf-8?q?Heidekrüger?= , Paul =?utf-8?q?Heidekrüger?= Message-ID: In-Reply-To: leunam99 wrote: We now added FIXITs with the different modes as suggested. Some notes about the implementation: - In the None / Function mode, we thought that checking if an at method exists does not make sense anymore and warn on every subscript operator use (besides the explicit exclusions) Then, it might be unintuitive that changing a setting called FIXMODE affects if a warning is emmitted, so we emit this general warning in any mode if we do not have a concrete alternative. Do you agree with this behaviour? Or will this result in too many false positives? - The Function mode does not yet check if a valid version of the function exists. Is there a simple way to check if a function template (or function) exists that can fit some specific parameters? Should/Can we do something reasonable if the function is not imported? - We prefixed the option names with 'Subscript' so that if the check later will be extended, the naming does not get confusing. - For Templates the fix does not work. We will handle this after it is more clear how Templates should be treated. 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
