[clang-tidy] "readability-implicit-bool-conversion" - should catch conversions that occur in a template function

[denzor200]

template<typename T1, typename T2>
bool compare(const T1& a, const T2& b) {
    return a == b ; // BAD - no warning
}

int main() {
    int a = 0;
    bool b = false;
    compare(a, b);
    return 0;
}

The full snippet here: https://godbolt.org/z/Yvs77cjej

[llvmbot]

@llvm/issue-subscribers-clang-tidy

Author: Denis Mikhailov (denzor200)

``` template<typename T1, typename T2> bool compare(const T1& a, const T2& b) { return a == b ; // BAD - no warning }

int main() {
int a = 0;
bool b = false;
compare(a, b);
return 0;
}

The full snippet here: https://godbolt.org/z/Yvs77cjej
</details>

[vbvictor]

It should not, from checks docs
"Occurrences of implicit conversions inside macros and template instantiations are deliberately ignored, as it is not clear how to deal with such cases."

What is your proposal for this case?
Function can be included in many translational units and instantiated with any template parameters

[denzor200]

My proposal is going to cover a real-life sample like this:

#include "gtest/gtest.h"

TEST(SampleTest, General) {
  int A = get();
  const bool expA = false;
  ASSERT_EQ(A, expA); // no warning at all
}

[vbvictor]

Adding support for template functions wouldn't likely catch the problem with ASSERT_EQ macros in gtest.

[vbvictor]

I agree that macros could be enabled via IngoreMacros option, but I still don't know what to do when template function has a warning. It may not be obvious for the user where to fix the issue, should we add explicit conversion in template function, or we should change variable types?

[denzor200]

I agree that macros could be enabled via IngoreMacros option, but I still don't know what to do when template function has a warning. It may not be obvious for the user where to fix the issue, should we add explicit conversion in template function, or we should change variable types?

The user can't change template inside Gtest library, so the warning should be provided outside template function:

#include "gtest/gtest.h"

TEST(SampleTest, General) {
  int A = get();
  const bool expA = false;
  ASSERT_EQ(A, expA); // < here is the best place for warning
}