[clang-tidy] fix PR36489 - respect deduced pointer types from auto as well

[JonasToth]

Summary

The cppcoreguidelines-pro-bounds-pointer-arithmetic warns on all occassion where
pointer arithmetic is used, but does not check values where the pointer types
is deduced via `auto`. This patch adjusts this behaviour and solved
PR36489.

[alexfh]

clang-tidy/cppcoreguidelines/ProBoundsPointerArithmeticCheck.cpp

24 ↗

(On Diff #153297)

Can anyOf be pushed inside hasType? (hasType(anyOf(pointerType(), autoType(...))))

[alexfh]

LG

[aaron.ballman]

test/clang-tidy/cppcoreguidelines-pro-bounds-pointer-arithmetic-pr36489.cpp

4 ↗

(On Diff #153297)

Can this also include tests for other deduced types, like decltype(expr) and decltype(auto)?

test/clang-tidy/cppcoreguidelines-pro-bounds-pointer-arithmetic.cpp

88–115

Spurious whitespace change.

[JonasToth]

clang-tidy/cppcoreguidelines/ProBoundsPointerArithmeticCheck.cpp

24 ↗

(On Diff #153297)

Unfortunatly no. It is ambigous then.

[JonasToth]

• address review comments

[aaron.ballman]

LGTM!

[JonasToth]

test/clang-tidy/cppcoreguidelines-pro-bounds-pointer-arithmetic-pr36489.cpp

4 ↗

(On Diff #153297)

decltype(getPtr()) actually breaks the code. I have to look into this, but tomorrow or on the weekend.

[JonasToth]

• fix decltype deduction with new matcher

I had to introduce a new matcher in clang for DecltypeType to reach its
underlying type. It would be better to have hasType resolve all these issues
but i dont know how much work that would be.

We can discuss that in the patch for the matcher i guess.

[aaron.ballman]

In D48717#1147644, @JonasToth wrote:

• fix decltype deduction with new matcher

I had to introduce a new matcher in clang for DecltypeType to reach its
underlying type. It would be better to have hasType resolve all these issues
but i dont know how much work that would be.

I don't think we want to modify hasType() -- that would strip off too much type information if it automatically reached through to the underlying type. However, we may want to consider adding something like hasUnderlyingType() that checks the matcher against each level of type sugar. I'm not certain this would be required for your patch, however.

[JonasToth]

I don't think we want to modify hasType() -- that would strip off too much type information if it automatically reached through to the underlying type. However, we may want to consider adding something like hasUnderlyingType() that checks the matcher against each level of type sugar. I'm not certain this would be required for your patch, however.

Yes, an additional facility that does not care how the type got produced (auto, decltype, written, template, ...) and just finds all places with that type.

[JonasToth]

The tests do run now with decltype too.

Still ready to land?

[aaron.ballman]

Yes, this looks good to me. Thanks!

[JonasToth]

rebase to master

test/clang-tidy/cppcoreguidelines-pro-bounds-pointer-arithmetic.cpp

void okay() {
   --i;
   i += 1;
   i -= 1;
   i = j + 1;
   i = j - 1;
 
   auto diff = p - q; // OK, result is arithmetic
 
-  for(int ii : a) ; // OK, pointer arithmetic generated by compiler
+  for (int ii : a)
+    ; // OK, pointer arithmetic generated by compiler
+}
+
+// Fix PR36207
+namespace std {
+template <typename CharT>
+struct char_traits {};
+
+template <typename T>
+struct allocator {};
+
+template <typename CharT,
+          typename Traits = char_traits<CharT>,
+          typename Allocator = allocator<CharT>>
+class basic_string {};
+
+template <class CharT, class Traits, class Alloc>
+basic_string<CharT, Traits, Alloc> operator+(const basic_string<CharT, Traits, Alloc> &lhs,
+                                             const CharT *rhs) {}
+
+using string = basic_string<char>;
+} // namespace std
+
+std::string str_generated() {}
+
+void problematic_addition() {
+  std::string status = str_generated() + " is not found";
 }