# \[Bug\] unqualified auto on C++ container iterator **URL:** \ **Category:** SonarQube Server / Community Build\ **Tags:** cfamily, sonarqube\ **Created:** [January 18, 2023, 11:14am UTC](https://community.sonarsource.com/t/bug-unqualified-auto-on-c-container-iterator/81501 "2023-01-18T11:14:01Z")\ **Posts on this page:** 6\ **Page:** 1
**Author:** ![Carl](https://avatars.discourse-cdn.com/v4/letter/c/ee7513/32.png) [@Carl](https://community.sonarsource.com/u/Carl)\ **Post date:** [January 18, 2023, 11:14am UTC](https://community.sonarsource.com/t/bug-unqualified-auto-on-c-container-iterator/81501/1 "2023-01-18T11:14:02Z")
Versions: - SonarQube 9.7.1 Enterprise - sonar-scanner 4.7.0.2747 I saw this code change in a PR. This was due to SonarQube complaining about the unqualified auto swallowing the pointer of the iterator. However the return type of begin() of our container is `Container::iterator` with iterator defined as `typedef T* iterator;`. ```diff - for (auto it = container.begin(); it != container.end(); ++it) { + for (auto* it = container.begin(); it != container.end(); ++it) { ``` In terms of iterators this is a false positive. Since the typedef already contains that this type is a pointer. If we exchanged this container with a different container that uses a class as iterator, e.g. `std::unordered_set` this would break. The analysis should differentiate between a pointer to a type and if there is a `typedef`/`using` declaration that already contains the pointer. A small reproduction shows that `clang-tidy` analyzes this correctly. Best regards, Carl ---
**Author:** ![Abbas](https://dub1.discourse-cdn.com/sonarsource/user_avatar/community.sonarsource.com/abbas/32/5717_2.png) [@Abbas](https://community.sonarsource.com/u/Abbas)\ **Post date:** [January 18, 2023, 12:41pm UTC](https://community.sonarsource.com/t/bug-unqualified-auto-on-c-container-iterator/81501/3 "2023-01-18T12:41:43Z")
Hi @Carl, To confirm, are you talking about this rule [RSPEC](https://sonarsource.github.io/rspec/#/rspec/S4334)? This rule is disabled by default for the same argument that you made. It is subjective: - The argument for it is usually readability/avoiding confusion. - The argument against it is what you mentioned about maintainability and being able to refactor the code without breaking it. Many of us (the analyzer developers) tend to favor the second argument, and that is why it didn’t end up enabled by default. As for exceptions, I don’t think it makes sense if you are a believer in the first argument. You can apply the argument you provided and say: if we exchange the pointer for a smart pointer, the code will break. Hence, the rule should never raise. In short, the rule tells you when a raw pointer is swallowed by auto. If you enable it, it means that you want your code to break when the type is no longer a pointer. Thanks, ---
**Author:** ![Carl](https://avatars.discourse-cdn.com/v4/letter/c/ee7513/32.png) [@Carl](https://community.sonarsource.com/u/Carl)\ **Post date:** [January 18, 2023, 12:59pm UTC](https://community.sonarsource.com/t/bug-unqualified-auto-on-c-container-iterator/81501/4 "2023-01-18T12:59:21Z")
Hi @Abbas, yes, this is the rule we are using. My argumentation is that this is not an exception. A typedef that contains a pointer is not the same as a pointer, since the auto isn’t “hiding” the fact that this is a pointer anymore, the typedef is. Since iterators may or may not be pointers (e.g. for contiguous vs non-contiguous containers), this would mean different iteration code in both cases. clang-tidy offers the same rule and seems to distinguish the two cases typedef including pointer, and pointer, which makes intuitive sense. Since in one case the auto is not swallowing. Also the auto is deducing a typedef iterator, not a raw pointer, so we think this is wrong. Best regards, Carl ---
**Author:** ![Abbas](https://dub1.discourse-cdn.com/sonarsource/user_avatar/community.sonarsource.com/abbas/32/5717_2.png) [@Abbas](https://community.sonarsource.com/u/Abbas)\ **Post date:** [January 18, 2023, 1:27pm UTC](https://community.sonarsource.com/t/bug-unqualified-auto-on-c-container-iterator/81501/5 "2023-01-18T13:27:07Z")
@Carl, Thanks for the explanation, I think the argument of typedef swallowing the pointer instead of the auto makes more sense than the previous argument about breaking the code. Since I’m against enabling this rule in general, I will flag this post to get a second opinion from the implementer. Thanks for the feedback! ---
**Author:** ![JolyLoic](https://dub1.discourse-cdn.com/sonarsource/user_avatar/community.sonarsource.com/jolyloic/32/199_2.png) [@JolyLoic](https://community.sonarsource.com/u/JolyLoic)\ **Post date:** [January 23, 2023, 6:21pm UTC](https://community.sonarsource.com/t/bug-unqualified-auto-on-c-container-iterator/81501/8 "2023-01-23T18:21:55Z")
@Carl, First, a disclaimer: Given the choice, I would not enable this rule on my code, which means I might not be the best person to discuss it, but I will share my reasoning when the rule was put in place: Why is showing the pointer so important? Because the fact that the type is a pointer matters, you don’t work with pointers the same way as you work with other types. You must care about reference semantics, you must ensure that the pointed-to object outlives its use through the pointer, and maybe you must care about deleting the pointed-to object. All these concerns remain valid, no matter if the pointer is hidden by a typedef or not. When using a typedef to a pointer, the fact that there is a pointer is usually not really hidden: The name of the typedef usually helps to remember that the code is dealing with a pointer (I’ve seen many typedefs with a `Ptr` suffix, for instance). I even suspect that codebases that require pointers to be explicit in `auto` cases would not accept a typedef to a pointer unless the typedef name clearly states what is happening (it’s however not easy for us to write a rule about that because it would require dabbling with the semantic of a name). When using `auto`, there is no such possible help, therefore the pointer should remain visible for the declaration. Therefore, I don’t think that, in general, ignoring a pointer contained within a typedef would be the right choice for this rule. I’m however willing to consider the fact that this rule should not trigger if the “this is a pointer” information might be given by another name. For instance, in the following C++20 code: ```auto for (std::forward_iterator auto it = cont.begin(); it != cont.end() ; ++it) { ``` The “this is a pointer” is no longer hidden, because it is explicit that `it` is an iterator (for which the same kind of extra care must be taken as for pointers) Do you think such a change would resolve your concern? ---
**Author:** ![Carl](https://avatars.discourse-cdn.com/v4/letter/c/ee7513/32.png) [@Carl](https://community.sonarsource.com/u/Carl)\ **Post date:** [January 27, 2023, 8:28am UTC](https://community.sonarsource.com/t/bug-unqualified-auto-on-c-container-iterator/81501/9 "2023-01-27T08:28:38Z")
Since we don’t use C++20 this wouldn’t help us. I understand your reasoning, I still find the clang-tidy way to be better to only complain about raw pointers here. As you can see nobody enables this rule because it would complain in places like this. So if everybody disables it because it doesn’t care about typedefs it seems broken in terms of what the user actually wants to check.