Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FR]: Add NOLINT to quiet clang-tidy on EXPECT_CALL #4531

Open
Maxxus220 opened this issue May 7, 2024 · 0 comments
Open

[FR]: Add NOLINT to quiet clang-tidy on EXPECT_CALL #4531

Maxxus220 opened this issue May 7, 2024 · 0 comments

Comments

@Maxxus220
Copy link

Does the feature exist in the most recent commit?

No

Why do we need this feature?

I can't find a way to quiet this warning nicely:

/usr/projects/lm75/build/_deps/googletest-src/googlemock/include/gmock/gmock-spec-builders.h:1010:3: warning: Potential leak of memory pointed to by field '_M_pi' [clang-analyzer-cplusplus.NewDeleteLeaks]
 1010 |   }
      |   ^
/usr/projects/lm75/tests/read_temp_test.cpp:37:7: note: Assuming the condition is false
   37 |   if (expected_temp_reg[1] != 0)
      |       ^~~~~~~~~~~~~~~~~~~~~~~~~
/usr/projects/lm75/tests/read_temp_test.cpp:37:3: note: Taking false branch
   37 |   if (expected_temp_reg[1] != 0)
      |   ^
/usr/projects/lm75/tests/read_temp_test.cpp:42:3: note: Calling 'TypedExpectation::WillOnce'
   42 |   EXPECT_CALL(mock_lm75_i2c_master_, Write(kSlaveAddress, _, _)).WillOnce(testing::Return(0));
      |   ^
/usr/projects/lm75/build/_deps/googletest-src/googlemock/include/gmock/gmock-spec-builders.h:2146:3: note: expanded from macro 'EXPECT_CALL'
 2146 |   GMOCK_ON_CALL_IMPL_(obj, InternalExpectedAt, call)
      |   ^
/usr/projects/lm75/build/_deps/googletest-src/googlemock/include/gmock/gmock-spec-builders.h:2138:3: note: expanded from macro 'GMOCK_ON_CALL_IMPL_'
 2138 |   ((mock_expr).gmock_##call)(::testing::internal::GetWithoutMatchers(), \
      |   ^
/usr/projects/lm75/build/_deps/googletest-src/googlemock/include/gmock/gmock-spec-builders.h:1008:9: note: Calling 'make_shared<testing::OnceAction<int (unsigned short, etl::span<const unsigned char>, unsigned long)>, testing::OnceAction<int (unsigned short, etl::span<const unsigned char>, unsigned long)>>'
 1008 |         std::make_shared<OnceAction<F>>(std::move(once_action)),
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/shared_ptr.h:875:14: note: Calling 'allocate_shared<testing::OnceAction<int (unsigned short, etl::span<const unsigned char>, unsigned long)>, std::allocator<testing::OnceAction<int (unsigned short, etl::span<const unsigned char>, unsigned long)>>, testing::OnceAction<int (unsigned short, etl::span<const unsigned char>, unsigned long)>>'
  875 |       return std::allocate_shared<_Tp>(std::allocator<_Tp_nc>(),
      |              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  876 |                                        std::forward<_Args>(__args)...);
      |                                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/shared_ptr.h:859:14: note: Calling constructor for 'shared_ptr<testing::OnceAction<int (unsigned short, etl::span<const unsigned char>, unsigned long)>>'
  859 |       return shared_ptr<_Tp>(_Sp_alloc_shared_tag<_Alloc>{__a},
      |              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  860 |                              std::forward<_Args>(__args)...);
      |                              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/shared_ptr.h:408:4: note: Calling constructor for '__shared_ptr<testing::OnceAction<int (unsigned short, etl::span<const unsigned char>, unsigned long)>, __gnu_cxx::_S_atomic>'
  408 |         : __shared_ptr<_Tp>(__tag, std::forward<_Args>(__args)...)
      |           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/shared_ptr_base.h:1371:14: note: Calling constructor for '__shared_count<__gnu_cxx::_S_atomic>'
 1371 |         : _M_ptr(), _M_refcount(_M_ptr, __tag, std::forward<_Args>(__args)...)
      |                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/shared_ptr_base.h:680:19: note: Calling '__allocate_guarded<std::allocator<std::_Sp_counted_ptr_inplace<testing::OnceAction<int (unsigned short, etl::span<const unsigned char>, unsigned long)>, std::allocator<testing::OnceAction<int (unsigned short, etl::span<const unsigned char>, unsigned long)>>, __gnu_cxx::_S_atomic>>>'
  680 |           auto __guard = std::__allocate_guarded(__a2);
      |                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/allocated_ptr.h:97:21: note: Calling 'allocator_traits::allocate'
   97 |       return { __a, std::allocator_traits<_Alloc>::allocate(__a, 1) };
      |                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/alloc_traits.h:460:16: note: Calling 'allocator::allocate'
  460 |       { return __a.allocate(__n); }
      |                ^~~~~~~~~~~~~~~~~
/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/allocator.h:170:6: note: Assuming the condition is false
  170 |         if (std::is_constant_evaluated())
      |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/allocator.h:170:2: note: Taking false branch
  170 |         if (std::is_constant_evaluated())
      |         ^
/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/allocator.h:173:9: note: Calling 'new_allocator::allocate'
  173 |         return __allocator_base<_Tp>::allocate(__n, 0);
      |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/ext/new_allocator.h:105:2: note: Taking false branch
  105 |         if (__n > this->_M_max_size())
      |         ^
/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/ext/new_allocator.h:109:2: note: Taking false branch
  109 |         if (alignof(_Tp) > __STDCPP_DEFAULT_NEW_ALIGNMENT__)
      |         ^
/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/ext/new_allocator.h:115:27: note: Memory is allocated
  115 |         return static_cast<_Tp*>(::operator new(__n * sizeof(_Tp)));
      |                                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/allocator.h:173:9: note: Returned allocated memory
  173 |         return __allocator_base<_Tp>::allocate(__n, 0);
      |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/alloc_traits.h:460:16: note: Returned allocated memory
  460 |       { return __a.allocate(__n); }
      |                ^~~~~~~~~~~~~~~~~
/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/allocated_ptr.h:97:21: note: Returned allocated memory
   97 |       return { __a, std::allocator_traits<_Alloc>::allocate(__a, 1) };
      |                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/shared_ptr_base.h:680:19: note: Returned allocated memory
  680 |           auto __guard = std::__allocate_guarded(__a2);
      |                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/shared_ptr_base.h:1371:14: note: Returning from constructor for '__shared_count<__gnu_cxx::_S_atomic>'
 1371 |         : _M_ptr(), _M_refcount(_M_ptr, __tag, std::forward<_Args>(__args)...)
      |                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/shared_ptr.h:408:4: note: Returning from constructor for '__shared_ptr<testing::OnceAction<int (unsigned short, etl::span<const unsigned char>, unsigned long)>, __gnu_cxx::_S_atomic>'
  408 |         : __shared_ptr<_Tp>(__tag, std::forward<_Args>(__args)...)
      |           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/shared_ptr.h:859:14: note: Returning from constructor for 'shared_ptr<testing::OnceAction<int (unsigned short, etl::span<const unsigned char>, unsigned long)>>'
  859 |       return shared_ptr<_Tp>(_Sp_alloc_shared_tag<_Alloc>{__a},
      |              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  860 |                              std::forward<_Args>(__args)...);
      |                              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/shared_ptr.h:875:14: note: Returned allocated memory
  875 |       return std::allocate_shared<_Tp>(std::allocator<_Tp_nc>(),
      |              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  876 |                                        std::forward<_Args>(__args)...);
      |                                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/projects/lm75/build/_deps/googletest-src/googlemock/include/gmock/gmock-spec-builders.h:1008:9: note: Returned allocated memory
 1008 |         std::make_shared<OnceAction<F>>(std::move(once_action)),
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/projects/lm75/build/_deps/googletest-src/googlemock/include/gmock/gmock-spec-builders.h:1010:3: note: Potential leak of memory pointed to by field '_M_pi'
 1010 |   }

From this code:

clang_tidy_helpers.h:

// Definitions to make unit tests pass static analysis more easily.
#pragma once

// GoogleTest's macro expansions do some things that Clang Tidy doesn't like, so we work around
// them here using NOLINT directives.

// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables,cppcoreguidelines-macro-usage,cppcoreguidelines-owning-memory)
#define TIDY_TEST(test_suite_name, test_name) TEST(test_suite_name, test_name)

// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables,cppcoreguidelines-macro-usage,cppcoreguidelines-owning-memory)
#define TIDY_TEST_F(test_fixture, test_name) TEST_F(test_fixture, test_name)

// NOLINTNEXTLINE(cppcoreguidelines-macro-usage,cppcoreguidelines-avoid-goto)
#define TIDY_EXPECT_NO_THROW(statement) EXPECT_NO_THROW(statement)

mock_lm75_i2c_master.h:

#pragma once

#include <gmock/gmock.h>
#include "etl/span.h"

class MockLm75I2cMaster
{
public:
  MOCK_METHOD(int, Write, (uint16_t i2c_address, etl::span<const uint8_t> tx_buf, size_t timeout_ms));
  MOCK_METHOD(int, Read, (uint16_t i2c_address, etl::span<uint8_t> rx_buf, size_t timeout_ms));
};

read_temp_test.cpp:

#include <cmath>
#include <cstdlib>
#include <ctime>
#include <gtest/gtest.h>
#include "helpers/clang_tidy_helpers.h"
#include "mocks/mock_lm75_i2c_master.h"
#include "cri/lm75/lm75.h"

using ::testing::_;

class Lm75Fixture : public ::testing::Test
{
protected:
  static constexpr uint8_t kSlaveAddress = 0x48;

  MockLm75I2cMaster         mock_lm75_i2c_master_;
  cri::lm75::ILm75I2cMaster i2c_api_{[this](uint16_t i2c_address, etl::span<const uint8_t> tx_buf, size_t timeout_ms) {
                                       return mock_lm75_i2c_master_.Write(i2c_address, tx_buf, timeout_ms);
                                     },
                                     [this](uint16_t i2c_address, etl::span<uint8_t> rx_buf, size_t timeout_ms) {
                                       return mock_lm75_i2c_master_.Read(i2c_address, rx_buf, timeout_ms);
                                     }};
  cri::lm75::Lm75           lm75_{&i2c_api_, 0};
};

TIDY_TEST_F(Lm75Fixture, ReadRandomTemp)
{
  constexpr int   kBitsInByte = 8;
  constexpr float kHalfDegree = 0.5;

  std::array<uint8_t, 2> expected_temp_reg = {
      static_cast<uint8_t>(std::rand() % static_cast<int>(std::pow(2, kBitsInByte))),
      static_cast<uint8_t>((std::rand() % 2) << (kBitsInByte - 1))};
  std::cout << "ExpectedTempReg[0]: " << expected_temp_reg[0] << std::endl;
  std::cout << "ExpectedTempReg[1]: " << expected_temp_reg[1] << std::endl;
  auto expected_temp = static_cast<float>(static_cast<int8_t>(expected_temp_reg[0]));
  if (expected_temp_reg[1] != 0)
  {
    expected_temp += kHalfDegree;
  }

  EXPECT_CALL(mock_lm75_i2c_master_, Write(kSlaveAddress, _, _)).WillOnce(testing::Return(0));
  EXPECT_CALL(mock_lm75_i2c_master_, Read(kSlaveAddress, _, _))
      .WillOnce(testing::DoAll(
          testing::Invoke([expected_temp_reg](uint8_t i2c_address, etl::span<uint8_t> rx_buf, size_t timeout_ms) {
            (void)i2c_address;
            (void)timeout_ms;
            rx_buf[0] = expected_temp_reg[0];
            rx_buf[1] = expected_temp_reg[1];
          }),
          testing::Return(0)));

  float temp = 0;
  int   err  = lm75_.ReadTemp(temp);
  EXPECT_EQ(err, 0);
  EXPECT_EQ(temp, expected_temp);
}

Describe the proposal.

All that would have to be added is a NOLINT comment at the end of the WillOnce function. Line 1010. Tried building with that NOLINT and the warning dissapears.

Is the feature specific to an operating system, compiler, or build system version?

Not entirely sure. For some reason, building on Windows I don't run into issues, but I do on Linux. I'm using the LLVM tools at 18.1.4. Specific to clang-tidy. The main differences I could potentially see is just that I'm using Unix Makefiles for the build on Linux instead of Ninja and that maybe the LLVM tools are different just cause of platform? They're both tagged at the same version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant