fix #13452: iterator for library container not always detected #7248
Conversation
|
@firewave does this look OK? |
|
Given the results of the unit tests it is. The code duplication and additional I wrecked my brain multiple times already in making this function just slightly faster in the past so figuring this out might take a bit. Please give me a few days since I am not feeling well at the moment. |
| if (isIterator) | ||
| *isIterator = false; | ||
| return &container; | ||
| } | ||
|
|
||
| if (!firstLinkedTok) | ||
| if (!firstLinkedTok) { |
There was a problem hiding this comment.
In this case you never match the start pattern.
There was a problem hiding this comment.
That needs a negative test case.
There was a problem hiding this comment.
Hm, sorry I'm not sure if I understand. The start pattern is implicitly matched in the "combined" pattern below as far as I can tell.
There was a problem hiding this comment.
Okay, I missed that but that causes that pattern to be matched twice if the first match fails.
The start match should be cached.
You can just modify the existing code with passing typeStart instead of firstLinkedTok->link() . Just store it in a variable.
The negative tests also already exist.
I will provide a branch with some WIP code (which does not work right now).
There was a problem hiding this comment.
I think we might be missing std::F<int>::iterator f_it; as a test case.
There was a problem hiding this comment.
https://github.com/firewave/cppcheck/tree/lib-it
This is broken because I misunderstood the matching in your new code. We need to adjust the end matching for that case.
| continue; | ||
|
|
||
| if (detect != IteratorOnly && container.endPattern.empty() && | ||
| Token::Match(typeStart, container.startPattern2.c_str() + offset)) { |
There was a problem hiding this comment.
The Token::Match() might be performed twice and should be cached.
Just off the top of my head I'm thinking we could store the result of this function in Get well soon! |
In reference to #6595.