[analyzer] Use explicit call description mode in iterator checkers #88913
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit explicitly specifies the matching mode (C library function, any non-method function, or C++ method) for the `CallDescription`s constructed in the iterator/container checkers. This change won't cause major functional changes, but isn't NFC because it ensures that e.g. call descriptions for a non-method function won't accidentally match a method that has the same name. Separate commits will perform (or have already performed) this change in other checkers. My goal is to ensure that the call description mode is always explicitly specified and eliminate (or strongly restrict) the vague "may be either a method or a simple function" mode that's the current default. I'm handling the iterator checkers in this separate commit because they're infamously complex; but I don't expect any trouble because this transition doesn't interact with the "central" logic of iterator handling.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-static-analyzer-1 Author: None (NagyDonat) ChangesThis commit explicitly specifies the matching mode (C library function, any non-method function, or C++ method) for the This change won't cause major functional changes, but isn't NFC because it ensures that e.g. call descriptions for a non-method function won't accidentally match a method that has the same name. Separate commits will perform (or have already performed) this change in other checkers. My goal is to ensure that the call description mode is always explicitly specified and eliminate (or strongly restrict) the vague "may be either a method or a simple function" mode that's the current default. I'm handling the iterator checkers in this separate commit because they're infamously complex; but I don't expect any trouble because this transition doesn't interact with the "central" logic of iterator handling. Full diff: https://github.com/llvm/llvm-project/pull/88913.diff 6 Files Affected:
diff --git a/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
index 009c0d3fb93686..55ed809bfed6ce 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
@@ -72,26 +72,31 @@ class ContainerModeling
SVal) const;
CallDescriptionMap<NoItParamFn> NoIterParamFunctions = {
- {{{"clear"}, 0}, &ContainerModeling::handleClear},
- {{{"assign"}, 2}, &ContainerModeling::handleAssign},
- {{{"push_back"}, 1}, &ContainerModeling::handlePushBack},
- {{{"emplace_back"}, 1}, &ContainerModeling::handlePushBack},
- {{{"pop_back"}, 0}, &ContainerModeling::handlePopBack},
- {{{"push_front"}, 1}, &ContainerModeling::handlePushFront},
- {{{"emplace_front"}, 1}, &ContainerModeling::handlePushFront},
- {{{"pop_front"}, 0}, &ContainerModeling::handlePopFront},
+ {{CDM::CXXMethod, {"clear"}, 0}, &ContainerModeling::handleClear},
+ {{CDM::CXXMethod, {"assign"}, 2}, &ContainerModeling::handleAssign},
+ {{CDM::CXXMethod, {"push_back"}, 1}, &ContainerModeling::handlePushBack},
+ {{CDM::CXXMethod, {"emplace_back"}, 1},
+ &ContainerModeling::handlePushBack},
+ {{CDM::CXXMethod, {"pop_back"}, 0}, &ContainerModeling::handlePopBack},
+ {{CDM::CXXMethod, {"push_front"}, 1},
+ &ContainerModeling::handlePushFront},
+ {{CDM::CXXMethod, {"emplace_front"}, 1},
+ &ContainerModeling::handlePushFront},
+ {{CDM::CXXMethod, {"pop_front"}, 0}, &ContainerModeling::handlePopFront},
};
CallDescriptionMap<OneItParamFn> OneIterParamFunctions = {
- {{{"insert"}, 2}, &ContainerModeling::handleInsert},
- {{{"emplace"}, 2}, &ContainerModeling::handleInsert},
- {{{"erase"}, 1}, &ContainerModeling::handleErase},
- {{{"erase_after"}, 1}, &ContainerModeling::handleEraseAfter},
+ {{CDM::CXXMethod, {"insert"}, 2}, &ContainerModeling::handleInsert},
+ {{CDM::CXXMethod, {"emplace"}, 2}, &ContainerModeling::handleInsert},
+ {{CDM::CXXMethod, {"erase"}, 1}, &ContainerModeling::handleErase},
+ {{CDM::CXXMethod, {"erase_after"}, 1},
+ &ContainerModeling::handleEraseAfter},
};
CallDescriptionMap<TwoItParamFn> TwoIterParamFunctions = {
- {{{"erase"}, 2}, &ContainerModeling::handleErase},
- {{{"erase_after"}, 2}, &ContainerModeling::handleEraseAfter},
+ {{CDM::CXXMethod, {"erase"}, 2}, &ContainerModeling::handleErase},
+ {{CDM::CXXMethod, {"erase_after"}, 2},
+ &ContainerModeling::handleEraseAfter},
};
};
diff --git a/clang/lib/StaticAnalyzer/Checkers/DebugContainerModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/DebugContainerModeling.cpp
index 72186a99d94358..d3830a01dd0cbd 100644
--- a/clang/lib/StaticAnalyzer/Checkers/DebugContainerModeling.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/DebugContainerModeling.cpp
@@ -42,9 +42,9 @@ class DebugContainerModeling
CheckerContext &) const;
CallDescriptionMap<FnCheck> Callbacks = {
- {{{"clang_analyzer_container_begin"}, 1},
+ {{CDM::SimpleFunc, {"clang_analyzer_container_begin"}, 1},
&DebugContainerModeling::analyzerContainerBegin},
- {{{"clang_analyzer_container_end"}, 1},
+ {{CDM::SimpleFunc, {"clang_analyzer_container_end"}, 1},
&DebugContainerModeling::analyzerContainerEnd},
};
diff --git a/clang/lib/StaticAnalyzer/Checkers/DebugIteratorModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/DebugIteratorModeling.cpp
index 79ab71d7829db7..203743dacda636 100644
--- a/clang/lib/StaticAnalyzer/Checkers/DebugIteratorModeling.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/DebugIteratorModeling.cpp
@@ -43,11 +43,11 @@ class DebugIteratorModeling
CheckerContext &) const;
CallDescriptionMap<FnCheck> Callbacks = {
- {{{"clang_analyzer_iterator_position"}, 1},
+ {{CDM::SimpleFunc, {"clang_analyzer_iterator_position"}, 1},
&DebugIteratorModeling::analyzerIteratorPosition},
- {{{"clang_analyzer_iterator_container"}, 1},
+ {{CDM::SimpleFunc, {"clang_analyzer_iterator_container"}, 1},
&DebugIteratorModeling::analyzerIteratorContainer},
- {{{"clang_analyzer_iterator_validity"}, 1},
+ {{CDM::SimpleFunc, {"clang_analyzer_iterator_validity"}, 1},
&DebugIteratorModeling::analyzerIteratorValidity},
};
diff --git a/clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
index a95e811c2a4181..5649454b4cd47e 100644
--- a/clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
@@ -129,19 +129,20 @@ class IteratorModeling
CallDescriptionMap<AdvanceFn> AdvanceLikeFunctions = {
// template<class InputIt, class Distance>
// void advance(InputIt& it, Distance n);
- {{{"std", "advance"}, 2}, &IteratorModeling::handleAdvance},
+ {{CDM::SimpleFunc, {"std", "advance"}, 2},
+ &IteratorModeling::handleAdvance},
// template<class BidirIt>
// BidirIt prev(
// BidirIt it,
// typename std::iterator_traits<BidirIt>::difference_type n = 1);
- {{{"std", "prev"}, 2}, &IteratorModeling::handlePrev},
+ {{CDM::SimpleFunc, {"std", "prev"}, 2}, &IteratorModeling::handlePrev},
// template<class ForwardIt>
// ForwardIt next(
// ForwardIt it,
// typename std::iterator_traits<ForwardIt>::difference_type n = 1);
- {{{"std", "next"}, 2}, &IteratorModeling::handleNext},
+ {{CDM::SimpleFunc, {"std", "next"}, 2}, &IteratorModeling::handleNext},
};
public:
diff --git a/clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
index d2b61fb92483c3..4dd2f700a2a0eb 100644
--- a/clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
@@ -56,10 +56,15 @@ class IteratorRangeChecker
using AdvanceFn = void (IteratorRangeChecker::*)(CheckerContext &, SVal,
SVal) const;
+ // FIXME: these three functions are also listed in IteratorModeling.cpp,
+ // perhaps unify their handling?
CallDescriptionMap<AdvanceFn> AdvanceFunctions = {
- {{{"std", "advance"}, 2}, &IteratorRangeChecker::verifyAdvance},
- {{{"std", "prev"}, 2}, &IteratorRangeChecker::verifyPrev},
- {{{"std", "next"}, 2}, &IteratorRangeChecker::verifyNext},
+ {{CDM::SimpleFunc, {"std", "advance"}, 2},
+ &IteratorRangeChecker::verifyAdvance},
+ {{CDM::SimpleFunc, {"std", "prev"}, 2},
+ &IteratorRangeChecker::verifyPrev},
+ {{CDM::SimpleFunc, {"std", "next"}, 2},
+ &IteratorRangeChecker::verifyNext},
};
};
diff --git a/clang/lib/StaticAnalyzer/Checkers/STLAlgorithmModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/STLAlgorithmModeling.cpp
index a5173a05636a09..e037719b902986 100644
--- a/clang/lib/StaticAnalyzer/Checkers/STLAlgorithmModeling.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/STLAlgorithmModeling.cpp
@@ -33,29 +33,50 @@ class STLAlgorithmModeling : public Checker<eval::Call> {
const CallExpr *) const;
const CallDescriptionMap<FnCheck> Callbacks = {
- {{{"std", "find"}, 3}, &STLAlgorithmModeling::evalFind},
- {{{"std", "find"}, 4}, &STLAlgorithmModeling::evalFind},
- {{{"std", "find_if"}, 3}, &STLAlgorithmModeling::evalFind},
- {{{"std", "find_if"}, 4}, &STLAlgorithmModeling::evalFind},
- {{{"std", "find_if_not"}, 3}, &STLAlgorithmModeling::evalFind},
- {{{"std", "find_if_not"}, 4}, &STLAlgorithmModeling::evalFind},
- {{{"std", "find_first_of"}, 4}, &STLAlgorithmModeling::evalFind},
- {{{"std", "find_first_of"}, 5}, &STLAlgorithmModeling::evalFind},
- {{{"std", "find_first_of"}, 6}, &STLAlgorithmModeling::evalFind},
- {{{"std", "find_end"}, 4}, &STLAlgorithmModeling::evalFind},
- {{{"std", "find_end"}, 5}, &STLAlgorithmModeling::evalFind},
- {{{"std", "find_end"}, 6}, &STLAlgorithmModeling::evalFind},
- {{{"std", "lower_bound"}, 3}, &STLAlgorithmModeling::evalFind},
- {{{"std", "lower_bound"}, 4}, &STLAlgorithmModeling::evalFind},
- {{{"std", "upper_bound"}, 3}, &STLAlgorithmModeling::evalFind},
- {{{"std", "upper_bound"}, 4}, &STLAlgorithmModeling::evalFind},
- {{{"std", "search"}, 3}, &STLAlgorithmModeling::evalFind},
- {{{"std", "search"}, 4}, &STLAlgorithmModeling::evalFind},
- {{{"std", "search"}, 5}, &STLAlgorithmModeling::evalFind},
- {{{"std", "search"}, 6}, &STLAlgorithmModeling::evalFind},
- {{{"std", "search_n"}, 4}, &STLAlgorithmModeling::evalFind},
- {{{"std", "search_n"}, 5}, &STLAlgorithmModeling::evalFind},
- {{{"std", "search_n"}, 6}, &STLAlgorithmModeling::evalFind},
+ {{CDM::SimpleFunc, {"std", "find"}, 3}, &STLAlgorithmModeling::evalFind},
+ {{CDM::SimpleFunc, {"std", "find"}, 4}, &STLAlgorithmModeling::evalFind},
+ {{CDM::SimpleFunc, {"std", "find_if"}, 3},
+ &STLAlgorithmModeling::evalFind},
+ {{CDM::SimpleFunc, {"std", "find_if"}, 4},
+ &STLAlgorithmModeling::evalFind},
+ {{CDM::SimpleFunc, {"std", "find_if_not"}, 3},
+ &STLAlgorithmModeling::evalFind},
+ {{CDM::SimpleFunc, {"std", "find_if_not"}, 4},
+ &STLAlgorithmModeling::evalFind},
+ {{CDM::SimpleFunc, {"std", "find_first_of"}, 4},
+ &STLAlgorithmModeling::evalFind},
+ {{CDM::SimpleFunc, {"std", "find_first_of"}, 5},
+ &STLAlgorithmModeling::evalFind},
+ {{CDM::SimpleFunc, {"std", "find_first_of"}, 6},
+ &STLAlgorithmModeling::evalFind},
+ {{CDM::SimpleFunc, {"std", "find_end"}, 4},
+ &STLAlgorithmModeling::evalFind},
+ {{CDM::SimpleFunc, {"std", "find_end"}, 5},
+ &STLAlgorithmModeling::evalFind},
+ {{CDM::SimpleFunc, {"std", "find_end"}, 6},
+ &STLAlgorithmModeling::evalFind},
+ {{CDM::SimpleFunc, {"std", "lower_bound"}, 3},
+ &STLAlgorithmModeling::evalFind},
+ {{CDM::SimpleFunc, {"std", "lower_bound"}, 4},
+ &STLAlgorithmModeling::evalFind},
+ {{CDM::SimpleFunc, {"std", "upper_bound"}, 3},
+ &STLAlgorithmModeling::evalFind},
+ {{CDM::SimpleFunc, {"std", "upper_bound"}, 4},
+ &STLAlgorithmModeling::evalFind},
+ {{CDM::SimpleFunc, {"std", "search"}, 3},
+ &STLAlgorithmModeling::evalFind},
+ {{CDM::SimpleFunc, {"std", "search"}, 4},
+ &STLAlgorithmModeling::evalFind},
+ {{CDM::SimpleFunc, {"std", "search"}, 5},
+ &STLAlgorithmModeling::evalFind},
+ {{CDM::SimpleFunc, {"std", "search"}, 6},
+ &STLAlgorithmModeling::evalFind},
+ {{CDM::SimpleFunc, {"std", "search_n"}, 4},
+ &STLAlgorithmModeling::evalFind},
+ {{CDM::SimpleFunc, {"std", "search_n"}, 5},
+ &STLAlgorithmModeling::evalFind},
+ {{CDM::SimpleFunc, {"std", "search_n"}, 6},
+ &STLAlgorithmModeling::evalFind},
};
public:
|
steakhal
approved these changes
Apr 16, 2024
|
Uh oh, those niebloids are really deep black magic ☠️ I didn't know about them previously, so thanks for mentioning them. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit explicitly specifies the matching mode (C library function, any non-method function, or C++ method) for the
CallDescriptions constructed in the iterator/container checkers.This change won't cause major functional changes, but isn't NFC because it ensures that e.g. call descriptions for a non-method function won't accidentally match a method that has the same name.
Separate commits will perform (or have already performed) this change in other checkers. My goal is to ensure that the call description mode is always explicitly specified and eliminate (or strongly restrict) the vague "may be either a method or a simple function" mode that's the current default.
I'm handling the iterator checkers in this separate commit because they're infamously complex; but I don't expect any trouble because this transition doesn't interact with the "central" logic of iterator handling.