[clang][analyzer] Add support for C++23 container methods releated to iterator in ContainerModeling #129719
Conversation
This file contains hidden or 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
… iterator in ContainerModeling
llvmbot
added
clang
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-static-analyzer-1 Author: None (flovent) Changesthis PR aims to add support for the following container methods introduced by C++23 in ContainerModeling:
And for Patch is 29.17 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/129719.diff 3 Files Affected:
diff --git a/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
index 55ed809bfed6c..04e7696ed5b9d 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
@@ -74,19 +74,25 @@ class ContainerModeling
CallDescriptionMap<NoItParamFn> NoIterParamFunctions = {
{{CDM::CXXMethod, {"clear"}, 0}, &ContainerModeling::handleClear},
{{CDM::CXXMethod, {"assign"}, 2}, &ContainerModeling::handleAssign},
+ {{CDM::CXXMethod, {"assign_range"}, 1}, &ContainerModeling::handleAssign},
{{CDM::CXXMethod, {"push_back"}, 1}, &ContainerModeling::handlePushBack},
{{CDM::CXXMethod, {"emplace_back"}, 1},
&ContainerModeling::handlePushBack},
+ {{CDM::CXXMethod, {"append_range"}, 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, {"prepend_range"}, 1},
+ &ContainerModeling::handlePushFront},
{{CDM::CXXMethod, {"pop_front"}, 0}, &ContainerModeling::handlePopFront},
};
CallDescriptionMap<OneItParamFn> OneIterParamFunctions = {
{{CDM::CXXMethod, {"insert"}, 2}, &ContainerModeling::handleInsert},
+ {{CDM::CXXMethod, {"insert_range"}, 2}, &ContainerModeling::handleInsert},
{{CDM::CXXMethod, {"emplace"}, 2}, &ContainerModeling::handleInsert},
{{CDM::CXXMethod, {"erase"}, 1}, &ContainerModeling::handleErase},
{{CDM::CXXMethod, {"erase_after"}, 1},
diff --git a/clang/test/Analysis/Inputs/system-header-simulator-cxx.h b/clang/test/Analysis/Inputs/system-header-simulator-cxx.h
index a379a47515668..5a42a952cca85 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator-cxx.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator-cxx.h
@@ -336,6 +336,15 @@ namespace std {
iterator erase(const_iterator position);
iterator erase(const_iterator first, const_iterator last);
+ template<typename R>
+ void assign_range(R&& rg);
+
+ template<typename R>
+ iterator insert_range(const_iterator position, R&& rg);
+
+ template<typename R>
+ void append_range(R&& rg);
+
T &operator[](size_t n) {
return _start[n];
}
@@ -414,6 +423,18 @@ namespace std {
iterator erase(const_iterator position);
iterator erase(const_iterator first, const_iterator last);
+ template<typename R>
+ void assign_range(R&& rg);
+
+ template<typename R>
+ iterator insert_range(const_iterator position, R&& rg);
+
+ template<typename R>
+ void append_range(R&& rg);
+
+ template<typename R>
+ void prepend_range(R&& rg);
+
iterator begin() { return iterator(_start); }
const_iterator begin() const { return const_iterator(_start); }
const_iterator cbegin() const { return const_iterator(_start); }
@@ -488,6 +509,18 @@ namespace std {
iterator erase(const_iterator position);
iterator erase(const_iterator first, const_iterator last);
+ template<typename R>
+ void assign_range(R&& rg);
+
+ template<typename R>
+ iterator insert_range(const_iterator position, R&& rg);
+
+ template<typename R>
+ void append_range(R&& rg);
+
+ template<typename R>
+ void prepend_range(R&& rg);
+
T &operator[](size_t n) {
return _start[n];
}
@@ -561,6 +594,15 @@ namespace std {
iterator erase_after(const_iterator position);
iterator erase_after(const_iterator first, const_iterator last);
+ template<typename R>
+ void assign_range(R&& rg);
+
+ template<typename R>
+ void prepend_range(R&& rg);
+
+ template<typename R>
+ iterator insert_range_after(const_iterator pos, R&& rg);
+
iterator begin() { return iterator(_start); }
const_iterator begin() const { return const_iterator(_start); }
const_iterator cbegin() const { return const_iterator(_start); }
diff --git a/clang/test/Analysis/iterator-modeling.cpp b/clang/test/Analysis/iterator-modeling.cpp
index 78882da4431fd..a5be1666f1bed 100644
--- a/clang/test/Analysis/iterator-modeling.cpp
+++ b/clang/test/Analysis/iterator-modeling.cpp
@@ -482,6 +482,39 @@ void forward_list_assign(std::forward_list<int> &FL, int n) {
clang_analyzer_eval(clang_analyzer_iterator_validity(i1)); //expected-warning{{FALSE}}
}
+/// assign_range()
+///
+/// - Invalidates all iterators, including the past-the-end iterator for all
+/// container types.
+
+void list_assign_range(std::list<int> &L, std::vector<int> vec) {
+ auto i0 = L.cbegin(), i1 = L.cend();
+ L.assign_range(vec);
+ clang_analyzer_eval(clang_analyzer_iterator_validity(i0)); //expected-warning{{FALSE}}
+ clang_analyzer_eval(clang_analyzer_iterator_validity(i1)); //expected-warning{{FALSE}}
+}
+
+void vector_assign_range(std::vector<int> &V, std::vector<int> vec) {
+ auto i0 = V.cbegin(), i1 = V.cend();
+ V.assign_range(vec);
+ clang_analyzer_eval(clang_analyzer_iterator_validity(i0)); //expected-warning{{FALSE}}
+ clang_analyzer_eval(clang_analyzer_iterator_validity(i1)); //expected-warning{{FALSE}}
+}
+
+void deque_assign_range(std::deque<int> &D, std::vector<int> vec) {
+ auto i0 = D.cbegin(), i1 = D.cend();
+ D.assign_range(vec);
+ clang_analyzer_eval(clang_analyzer_iterator_validity(i0)); //expected-warning{{FALSE}}
+ clang_analyzer_eval(clang_analyzer_iterator_validity(i1)); //expected-warning{{FALSE}}
+}
+
+void forward_list_assign_range(std::forward_list<int> &FL, std::vector<int> vec) {
+ auto i0 = FL.cbegin(), i1 = FL.cend();
+ FL.assign_range(vec);
+ clang_analyzer_eval(clang_analyzer_iterator_validity(i0)); //expected-warning{{FALSE}}
+ clang_analyzer_eval(clang_analyzer_iterator_validity(i1)); //expected-warning{{FALSE}}
+}
+
/// clear()
///
/// - Invalidates all iterators, including the past-the-end iterator for all
@@ -635,6 +668,66 @@ void deque_emplace_back(std::deque<int> &D, int n) {
clang_analyzer_eval(clang_analyzer_iterator_validity(i2)); //expected-warning{{FALSE}}
}
+/// append_range()
+///
+/// - Design decision: extends containers to the ->RIGHT-> (i.e. the
+/// past-the-end position of the container is incremented).
+///
+/// - Iterator invalidation rules depend the container type.
+
+/// std::list-like containers: No iterators are invalidated.
+
+void list_append_range(std::list<int> &L, std::vector<int>& vec) {
+ auto i0 = L.cbegin(), i1 = --L.cend(), i2 = L.cend();
+
+ clang_analyzer_denote(clang_analyzer_container_begin(L), "$L.begin()");
+ clang_analyzer_denote(clang_analyzer_container_end(L), "$L.end()");
+
+ L.append_range(vec);
+
+ clang_analyzer_eval(clang_analyzer_iterator_validity(i0)); //expected-warning{{TRUE}}
+ clang_analyzer_eval(clang_analyzer_iterator_validity(i1)); //expected-warning{{TRUE}}
+ clang_analyzer_eval(clang_analyzer_iterator_validity(i2)); //expected-warning{{TRUE}}
+
+ clang_analyzer_express(clang_analyzer_iterator_position(i0)); // expected-warning-re {{$L.begin(){{$}}}}
+ clang_analyzer_express(clang_analyzer_iterator_position(i1)); // expected-warning-re {{$L.end() - 1{{$}}}}
+ clang_analyzer_express(clang_analyzer_iterator_position(i2)); // expected-warning-re {{$L.end(){{$}}}} FIXME: Should be $L.end() + 1
+}
+
+/// std::vector-like containers: The past-the-end iterator is invalidated.
+
+void vector_append_range(std::vector<int> &V, std::vector<int>& vec) {
+ auto i0 = V.cbegin(), i1 = --V.cend(), i2 = V.cend();
+
+ clang_analyzer_denote(clang_analyzer_container_begin(V), "$V.begin()");
+ clang_analyzer_denote(clang_analyzer_container_end(V), "$V.end()");
+
+ V.emplace_back(vec);
+
+ clang_analyzer_eval(clang_analyzer_iterator_validity(i0)); //expected-warning{{TRUE}}
+ clang_analyzer_eval(clang_analyzer_iterator_validity(i1)); //expected-warning{{TRUE}}
+ clang_analyzer_eval(clang_analyzer_iterator_validity(i2)); //expected-warning{{FALSE}}
+
+ clang_analyzer_express(clang_analyzer_iterator_position(i0)); // expected-warning-re {{$V.begin(){{$}}}}
+ clang_analyzer_express(clang_analyzer_iterator_position(i1)); // expected-warning-re {{$V.end() - 1{{$}}}}
+}
+
+/// std::deque-like containers: All iterators, including the past-the-end
+/// iterator, are invalidated.
+
+void deque_append_range(std::deque<int> &D, std::vector<int>& vec) {
+ auto i0 = D.cbegin(), i1 = --D.cend(), i2 = D.cend();
+
+ clang_analyzer_denote(clang_analyzer_container_begin(D), "$D.begin()");
+ clang_analyzer_denote(clang_analyzer_container_end(D), "$D.end()");
+
+ D.append_range(vec);
+
+ clang_analyzer_eval(clang_analyzer_iterator_validity(i0)); //expected-warning{{FALSE}}
+ clang_analyzer_eval(clang_analyzer_iterator_validity(i1)); //expected-warning{{FALSE}}
+ clang_analyzer_eval(clang_analyzer_iterator_validity(i2)); //expected-warning{{FALSE}}
+}
+
/// pop_back()
///
/// - Design decision: shrinks containers to the <-LEFT<- (i.e. the
@@ -811,6 +904,63 @@ void forward_list_emplace_front(std::forward_list<int> &FL, int n) {
clang_analyzer_express(clang_analyzer_iterator_position(i1)); // expected-warning-re {{$FL.end(){{$}}}}
}
+/// prepend_range()
+///
+/// - Design decision: extends containers to the <-LEFT<- (i.e. the first
+/// position of the container is decremented).
+///
+/// - Iterator invalidation rules depend the container type.
+
+/// std::list-like containers: No iterators are invalidated.
+
+void list_prepend_range(std::list<int> &L, std::vector<int>& vec) {
+ auto i0 = L.cbegin(), i1 = L.cend();
+
+ clang_analyzer_denote(clang_analyzer_container_begin(L), "$L.begin()");
+ clang_analyzer_denote(clang_analyzer_container_end(L), "$L.end()");
+
+ L.prepend_range(vec);
+
+ clang_analyzer_eval(clang_analyzer_iterator_validity(i0)); //expected-warning{{TRUE}}
+ clang_analyzer_eval(clang_analyzer_iterator_validity(i1)); //expected-warning{{TRUE}}
+
+ clang_analyzer_express(clang_analyzer_iterator_position(i0)); // expected-warning-re {{$L.begin(){{$}}}}
+ clang_analyzer_express(clang_analyzer_iterator_position(i1)); // expected-warning-re {{$L.end(){{$}}}}
+}
+
+/// std::deque-like containers: All iterators, including the past-the-end
+/// iterator, are invalidated.
+
+void deque_prepend_range(std::deque<int> &D, std::vector<int>& vec) {
+ auto i0 = D.cbegin(), i1 = --D.cend(), i2 = D.cend();
+
+ clang_analyzer_denote(clang_analyzer_container_begin(D), "$D.begin()");
+ clang_analyzer_denote(clang_analyzer_container_end(D), "$D.end()");
+
+ D.prepend_range(vec);
+
+ clang_analyzer_eval(clang_analyzer_iterator_validity(i0)); //expected-warning{{FALSE}}
+ clang_analyzer_eval(clang_analyzer_iterator_validity(i1)); //expected-warning{{FALSE}}
+ clang_analyzer_eval(clang_analyzer_iterator_validity(i2)); //expected-warning{{FALSE}}
+}
+
+/// std::forward_list-like containers: No iterators are invalidated.
+
+void forward_list_prepend_range(std::forward_list<int> &FL, std::vector<int>& vec) {
+ auto i0 = FL.cbegin(), i1 = FL.cend();
+
+ clang_analyzer_denote(clang_analyzer_container_begin(FL), "$FL.begin()");
+ clang_analyzer_denote(clang_analyzer_container_end(FL), "$FL.end()");
+
+ FL.prepend_range(vec);
+
+ clang_analyzer_eval(clang_analyzer_iterator_validity(i0)); //expected-warning{{TRUE}}
+ clang_analyzer_eval(clang_analyzer_iterator_validity(i1)); //expected-warning{{TRUE}}
+
+ clang_analyzer_express(clang_analyzer_iterator_position(i0)); // expected-warning-re {{$FL.begin(){{$}}}}
+ clang_analyzer_express(clang_analyzer_iterator_position(i1)); // expected-warning-re {{$FL.end(){{$}}}}
+}
+
/// pop_front()
///
/// - Design decision: shrinks containers to the ->RIGHT-> (i.e. the first
@@ -1139,6 +1289,271 @@ void deque_insert_end(std::deque<int> &D, int n) {
// clang_analyzer_express(clang_analyzer_iterator_position(i3)); FIXME: expect warning $D.end() - 1
}
+/// insert_range()
+///
+/// - Design decision: shifts positions to the <-LEFT<- (i.e. all iterator
+/// ahead of the insertion point are decremented; if the
+/// relation between the insertion point and the first
+/// position of the container is known, the first position
+/// of the container is also decremented).
+///
+/// - Iterator invalidation rules depend the container type.
+
+/// std::list-like containers: No iterators are invalidated.
+
+void list_insert_range_begin(std::list<int> &L, std::vector<int>& vec) {
+ auto i0 = L.cbegin(), i1 = L.cend();
+
+ clang_analyzer_denote(clang_analyzer_container_begin(L), "$L.begin()");
+ clang_analyzer_denote(clang_analyzer_container_end(L), "$L.end()");
+
+ auto i2 = L.insert_range(i0, vec);
+
+ clang_analyzer_eval(clang_analyzer_iterator_validity(i0)); //expected-warning{{TRUE}}
+ clang_analyzer_eval(clang_analyzer_iterator_validity(i1)); //expected-warning{{TRUE}}
+
+ clang_analyzer_express(clang_analyzer_iterator_position(i0)); // expected-warning-re {{$L.begin(){{$}}}}
+ // clang_analyzer_express(clang_analyzer_iterator_position(i2)); FIXME: expect warning $L.begin() - 1
+ clang_analyzer_express(clang_analyzer_iterator_position(i1)); // expected-warning-re {{$L.end(){{$}}}}
+}
+
+void list_insert_range_behind_begin(std::list<int> &L, std::vector<int>& vec) {
+ auto i0 = L.cbegin(), i1 = ++L.cbegin(), i2 = L.cend();
+
+ clang_analyzer_denote(clang_analyzer_container_begin(L), "$L.begin()");
+ clang_analyzer_denote(clang_analyzer_container_end(L), "$L.end()");
+
+ auto i3 = L.insert_range(i1, vec);
+
+ clang_analyzer_eval(clang_analyzer_iterator_validity(i0)); //expected-warning{{TRUE}}
+ clang_analyzer_eval(clang_analyzer_iterator_validity(i1)); //expected-warning{{TRUE}}
+ clang_analyzer_eval(clang_analyzer_iterator_validity(i2)); //expected-warning{{TRUE}}
+
+ clang_analyzer_express(clang_analyzer_iterator_position(i0)); // expected-warning-re {{$L.begin(){{$}}}} FIXME: Should be $L.begin() - 1
+ clang_analyzer_express(clang_analyzer_iterator_position(i1)); // expected-warning-re {{$L.begin() + 1{{$}}}}
+ // clang_analyzer_express(clang_analyzer_iterator_position(i3)); FIXME: expect warning $L.begin()
+ clang_analyzer_express(clang_analyzer_iterator_position(i2)); // expected-warning-re {{$L.end(){{$}}}}
+}
+
+template <typename Iter> Iter return_any_iterator(const Iter &It);
+
+void list_insert_range_unknown(std::list<int> &L, std::vector<int>& vec) {
+ auto i0 = L.cbegin(), i1 = return_any_iterator(L.cbegin()), i2 = L.cend();
+
+ clang_analyzer_denote(clang_analyzer_container_begin(L), "$L.begin()");
+ clang_analyzer_denote(clang_analyzer_container_end(L), "$L.end()");
+ clang_analyzer_denote(clang_analyzer_iterator_position(i1), "$i1");
+
+ auto i3 = L.insert_range(i1, vec);
+
+ clang_analyzer_eval(clang_analyzer_iterator_validity(i0)); //expected-warning{{TRUE}}
+ clang_analyzer_eval(clang_analyzer_iterator_validity(i1)); //expected-warning{{TRUE}}
+ clang_analyzer_eval(clang_analyzer_iterator_validity(i2)); //expected-warning{{TRUE}}
+
+ clang_analyzer_express(clang_analyzer_iterator_position(i0)); // expected-warning-re {{$L.begin(){{$}}}}
+ clang_analyzer_express(clang_analyzer_iterator_position(i1)); // expected-warning-re {{$i1{{$}}}}
+ // clang_analyzer_express(clang_analyzer_iterator_position(i3)); FIXME: expect warning $i - 1
+ clang_analyzer_express(clang_analyzer_iterator_position(i2)); // expected-warning-re {{$L.end(){{$}}}}
+}
+
+void list_insert_range_ahead_of_end(std::list<int> &L, std::vector<int>& vec) {
+ auto i0 = L.cbegin(), i1 = --L.cend(), i2 = L.cend();
+
+ clang_analyzer_denote(clang_analyzer_container_begin(L), "$L.begin()");
+ clang_analyzer_denote(clang_analyzer_container_end(L), "$L.end()");
+
+ auto i3 = L.insert_range(i1, vec);
+
+ clang_analyzer_eval(clang_analyzer_iterator_validity(i0)); //expected-warning{{TRUE}}
+ clang_analyzer_eval(clang_analyzer_iterator_validity(i1)); //expected-warning{{TRUE}}
+ clang_analyzer_eval(clang_analyzer_iterator_validity(i2)); //expected-warning{{TRUE}}
+
+ clang_analyzer_express(clang_analyzer_iterator_position(i0)); // expected-warning-re {{$L.begin(){{$}}}}
+ clang_analyzer_express(clang_analyzer_iterator_position(i1)); // expected-warning-re {{$L.end() - 1{{$}}}}
+ clang_analyzer_express(clang_analyzer_iterator_position(i2)); // expected-warning-re {{$L.end(){{$}}}}
+ // clang_analyzer_express(clang_analyzer_iterator_position(i3)); FIXME: expect warning $L.end() - 2
+}
+
+void list_insert_range_end(std::list<int> &L, std::vector<int>& vec) {
+ auto i0 = L.cbegin(), i1 = --L.cend(), i2 = L.cend();
+
+ clang_analyzer_denote(clang_analyzer_container_begin(L), "$L.begin()");
+ clang_analyzer_denote(clang_analyzer_container_end(L), "$L.end()");
+
+ auto i3 = L.insert_range(i2, vec);
+
+ clang_analyzer_eval(clang_analyzer_iterator_validity(i0)); //expected-warning{{TRUE}}
+ clang_analyzer_eval(clang_analyzer_iterator_validity(i1)); //expected-warning{{TRUE}}
+ clang_analyzer_eval(clang_analyzer_iterator_validity(i2)); //expected-warning{{TRUE}}
+
+ clang_analyzer_express(clang_analyzer_iterator_position(i0)); // expected-warning-re {{$L.begin(){{$}}}}
+ clang_analyzer_express(clang_analyzer_iterator_position(i1)); // expected-warning-re {{$L.end() - 1{{$}}}} FIXME: should be $L.end() - 2
+ clang_analyzer_express(clang_analyzer_iterator_position(i2)); // expected-warning-re {{$L.end(){{$}}}}
+ // clang_analyzer_express(clang_analyzer_iterator_position(i3)); FIXME: expect warning $L.end() - 1
+}
+
+/// std::vector-like containers: Only the iterators before the insertion point
+/// remain valid. The past-the-end iterator is also
+/// invalidated.
+
+void vector_insert_range_begin(std::vector<int> &V, std::vector<int>& vec) {
+ auto i0 = V.cbegin(), i1 = V.cend();
+
+ clang_analyzer_denote(clang_analyzer_container_begin(V), "$V.begin()");
+ clang_analyzer_denote(clang_analyzer_container_end(V), "$V.end()");
+
+ auto i2 = V.insert_range(i0, vec);
+
+ clang_analyzer_eval(clang_analyzer_iterator_validity(i0)); //expected-warning{{FALSE}}
+ clang_analyzer_eval(clang_analyzer_iterator_validity(i1)); //expected-warning{{FALSE}}
+
+ // clang_analyzer_express(clang_analyzer_iterator_position(i2)); FIXME: expect warning $V.begin() - 1
+}
+
+void vector_insert_range_behind_begin(std::vector<int> &V, std::vector<int>& vec) {
+ auto i0 = V.cbegin(), i1 = ++V.cbegin(), i2 = V.cend();
+
+ clang_analyzer_denote(clang_analyzer_container_begin(V), "$V.begin()");
+ clang_analyzer_denote(clang_analyzer_container_end(V), "$V.end()");
+
+ auto i3 = V.insert_range(i1, vec);
+
+ clang_analyzer_eval(clang_analyzer_iterator_validity(i0)); //expected-warning{{TRUE}}
+ clang_analyzer_eval(clang_analyzer_iterator_validity(i1)); //expected-warning{{FALSE}}
+ clang_analyzer_eval(clang_analyzer_iterator_validity(i2)); //expected-warning{{FALSE}}
+
+ clang_analyzer_express(clang_analyzer_iterator_position(i0)); // expected-warning-re {{$V.begin(){{$}}}} FIXME: Should be $V.begin() - 1
+ // clang_analyzer_express(clang_analyzer_iterator_position(i3)); // FIXME: expect -warning $V.begin()
+}
+
+void vector_insert_range_unknown(std::vector<int> &V, std::vector<int>& vec) {
+ auto i0 = V.cbegin(), i1 = return_any_iterator(V.cbegin()), i2 = V.cend();
+
+ clang_analyzer_denote(clang_analyzer_container_begin(V), "$V.begin()");
+ clang_analyzer_denote(clang_analyzer_container_end(V), "$V.end()");
+ clang_analyzer_denote(clang_analyzer_iterator_position(i1), "$i1");
+
+ auto i3 = V.insert_range(i1, vec);
+
+ clang_analyzer_eval(clang_analyzer_iterator_validity(i0)); //expected-warning{{TRUE}}
+ clang_analyzer_eval(clang_analyzer_iterator_validity(i1)); //expected-warning{{FALSE}}
+ clang_analyzer_eval(clang_analyzer_iterator_validity(i2)); //expected-warning{{FALSE}}
+
+ clang_analyzer_express(clang_analyzer_iterator_position(i0)); // expected-warning-re {{$V.begin(){{$}}}}
+ // clang_analyzer_express(clang_analyzer_iterator_position(i3)); FIXME: expecte warning $i1 - 1
+}
+
+void vector_insert_range_ahead_of_end(std::vector<int> &V, std::vector<int>& vec) {
+ auto i0 = V.cbegin(), i1 = --V.cend(), i2 = V.cend();
+
+ clang_analyzer_de...
[truncated]
|
|
@NagyDonat Hi, sorry to bother, can you review this PR? i don't have access to request review and i see that the last commit of this checker was submitted by you. |
NagyDonat
approved these changes
Mar 11, 2025
There was a problem hiding this comment.
Well, I have to admit that I'm not familiar with this checker -- my recent change is just part of a semi-automated commit series that mechanically changed the same trivial thing in many checkers.
However, your change appears to be straightforward and well-tested, so I'm giving a tentative approval. I also added a few other reviewers, so please wait with merging it for a few days or until somebody else approves this change.
| @@ -635,6 +668,66 @@ void deque_emplace_back(std::deque<int> &D, int n) { | |||
| clang_analyzer_eval(clang_analyzer_iterator_validity(i2)); //expected-warning{{FALSE}} | |||
| } | |||
|
|
|||
| /// append_range() | |||
| /// | |||
| /// - Design decision: extends containers to the ->RIGHT-> (i.e. the | |||
There was a problem hiding this comment.
I don't understand what does Design decision: mean in this context (is it some weird boilerplate from a rigid test-driven design pattern??), but it's consistently applied in all other test blocks, so it's probably better to use it here.
|
thank you! I will wait for other reviewer's approval. |
steakhal
requested changes
Mar 17, 2025
| @@ -635,6 +668,66 @@ void deque_emplace_back(std::deque<int> &D, int n) { | |||
| clang_analyzer_eval(clang_analyzer_iterator_validity(i2)); //expected-warning{{FALSE}} | |||
| } | |||
|
|
|||
| /// append_range() | |||
| /// | |||
| /// - Design decision: extends containers to the ->RIGHT-> (i.e. the | |||
|
|
||
| clang_analyzer_express(clang_analyzer_iterator_position(i0)); // expected-warning-re {{$L.begin(){{$}}}} | ||
| clang_analyzer_express(clang_analyzer_iterator_position(i1)); // expected-warning-re {{$L.end() - 1{{$}}}} | ||
| clang_analyzer_express(clang_analyzer_iterator_position(i2)); // expected-warning-re {{$L.end(){{$}}}} FIXME: Should be $L.end() + 1 |
There was a problem hiding this comment.
Why should this be $L.end() + 1? i2 was defined by L.cend(), so I think the actual output is correct.
There was a problem hiding this comment.
This message is not from the initialization statement of this iterator, ContainerModeling deal cbegin and begin or the corresponding iterator the same way, this is the function used to determine if this function returns container's first iterator.
bool isBeginCall(const FunctionDecl *Func) {
const auto *IdInfo = Func->getIdentifier();
if (!IdInfo)
return false;
return IdInfo->getName().ends_with_insensitive("begin");
}
And this message seems to be just for test and will not shown for user, $L.end() is setted in previous line:
clang_analyzer_denote(clang_analyzer_container_end(L), "$L.end()");
| clang_analyzer_eval(clang_analyzer_iterator_validity(i2)); //expected-warning{{FALSE}} | ||
|
|
||
| clang_analyzer_express(clang_analyzer_iterator_position(i0)); // expected-warning-re {{$V.begin(){{$}}}} | ||
| clang_analyzer_express(clang_analyzer_iterator_position(i1)); // expected-warning-re {{$V.end() - 1{{$}}}} |
There was a problem hiding this comment.
clang_analyzer_express(clang_analyzer_iterator_position(i2)); is not checked here; intentional?
There was a problem hiding this comment.
clang_analyzer_express(clang_analyzer_iterator_position(i2)); // expected-warning-re {{$L.end(){{$}}}}
I add this in my local environment, it can pass the test.
Similar test code is missed in several non-range version test function, they doesn't check all iterator using clang_analyzer_express(clang_analyzer_iterator_position()), but they are basically the same thing as other tests which has all iterators' test, should i add it and sync the change to original testcase ? or keep the same as original?
| clang_analyzer_eval(clang_analyzer_iterator_validity(i1)); //expected-warning{{TRUE}} | ||
|
|
||
| clang_analyzer_express(clang_analyzer_iterator_position(i0)); // expected-warning-re {{$L.begin(){{$}}}} | ||
| // clang_analyzer_express(clang_analyzer_iterator_position(i2)); FIXME: expect warning $L.begin() - 1 |
There was a problem hiding this comment.
Why is this line disabled? We could (and should) expect the actual output; and put a FIXME there if it's actually not what is desired.
|
|
||
| clang_analyzer_express(clang_analyzer_iterator_position(i0)); // expected-warning-re {{$L.begin(){{$}}}} FIXME: Should be $L.begin() - 1 | ||
| clang_analyzer_express(clang_analyzer_iterator_position(i1)); // expected-warning-re {{$L.begin() + 1{{$}}}} | ||
| // clang_analyzer_express(clang_analyzer_iterator_position(i3)); FIXME: expect warning $L.begin() |
There was a problem hiding this comment.
Same here. We should not have disabled check lines.
This applies to all of the currently disabled lines.
There was a problem hiding this comment.
These check lines is disabled because currently ContainerModeling doesn't model these function's return value like insert, this checker only models impact for container now except begin(cbegin) and end(cend) call
|
@steakhal Thank you for review! I forgot to mention that the testcases added is copied from their non-range verison since they have same effect to iterator which mentioned in PR description, the only change is the method's name and a parameter named So the But it does seems that the problem you mentioned is suspicious, i will take a look on them and try to find the cause. |
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 PR aims to add support for the following container methods introduced by C++23 in ContainerModeling:
assign_range: range version ofassign, have the same effect for iteratorappend_range: range version ofpush_back, have the same effect for iteratorprepend_range: range version ofpush_front, have the same effect for iteratorinsert_range: range version ofinsert, have the same effect for iteratorAnd for
insert_range_afterwhich is range version ofinsert_afterand have no effect for iterator, no explicit modeling is needed