clangd icon indicating copy to clipboard operation
clangd copied to clipboard

`clangd` still shows some `clang-tidy` warnings from inside a macro coming from a system header

Open philippewarren opened this issue 10 months ago • 1 comments

Like in #1448, I'm using GoogleTest and GoogleMock. The macro TEST from GoogleTest behaves correctly, and it does not give any error, as it comes from a system header. But the macros MATCHER and MATCHER_P don't: they give me errors from clang-tidy at the use point of the macro. I get modernize-use-trailing-return-type warnings, as well as cppcoreguidelines-avoid-const-or-ref-data-members coming from these macros.

I've checked my compile_commands.json, the include path for both <gtest/gtest.h>(where the TEST macro comes from) and <gmock/gmock.h> (where the MATCHER and MATCHER_P macros are defined) is the same, and it is marked as -isystem.

Code example: The trailing-return-type warning is flagged on the PointEq word, and the const-or-ref-data-members on the p word, on the first line

MATCHER_P(PointEq, p, absl::StrFormat("has fields {.x = %s, .y = %s}", PrintToString(p.x), PrintToString(p.y)))
{
    return ExplainMatchResult(
        AllOf(Field("x", &Point::x, Eq(p.x)), Field("y", &Point::y, Eq(p.y))),
        arg,
        result_listener);
}

Expanded version, with added comments marking the points where the warnings are shown:

template<typename p_type>
class PointEqMatcherP : public ::testing ::internal ::MatcherBaseImpl<PointEqMatcherP<p_type>>
{
public:
    using PointEqMatcherP ::MatcherBaseImpl ::MatcherBaseImpl;

    template<typename arg_type>
    class gmock_Impl : public ::testing ::MatcherInterface<const arg_type&>
    {
    public:
        explicit gmock_Impl(p_type gmock_p0) : p(::std ::forward<p_type>(gmock_p0)) {}

// The next line gives the trailing-return-type warning
        bool MatchAndExplain(const arg_type& arg, ::testing ::MatchResultListener* result_listener) const override;

        void DescribeTo(::std ::ostream* gmock_os) const override { *gmock_os << FormatDescription(false); }

        void DescribeNegationTo(::std ::ostream* gmock_os) const override { *gmock_os << FormatDescription(true); }

// This next line gives the const-data-member warning
        const p_type p;

    private:
// The next line gives the trailing-return-type warning
        ::std ::string FormatDescription(bool negation) const
        {
            ::std ::string gmock_description;
            gmock_description =
                (absl ::StrFormat("has fields {.x = %s, .y = %s}", PrintToString(p.x), PrintToString(p.y)));
            if (!gmock_description.empty())
            {
                return gmock_description;
            }
            return ::testing ::internal ::FormatMatcherDescription(
                negation,
                "PointEq",
                {"p"},
                ::testing ::internal ::UniversalTersePrintTupleFieldsToStrings(::std ::tuple<p_type>(p)));
        }
    };
};

template<typename p_type>
// The next line gives the trailing-return-type warning
inline PointEqMatcherP<p_type> PointEq(p_type gmock_p0)
{
    return PointEqMatcherP<p_type>(gmock_p0);
}

template<typename p_type>
template<typename arg_type>
// The next line gives the trailing-return-type warning
bool PointEqMatcherP<p_type>::gmock_Impl<arg_type>::MatchAndExplain(
    const arg_type& arg,
    ::testing ::MatchResultListener* result_listener __attribute__((unused))) const
{
    return ExplainMatchResult(
        AllOf(Field("x", &Point::x, Eq(p.x)), Field("y", &Point::y, Eq(p.y))),
        arg,
        result_listener);
}

Interestingly enough, I get a bunch of clangd(missing-includes) warning in the expanded version that don't show using the macro. This seems to indicate that the clangd missing includes diagnostics are correctly silenced in a system-header-provided macro, but not those from clang-tidy, sometimes. The TEST macros, like the example in #1448, works fine in my case too: no error is shown.

Is there a way to validate that clangd correctly considers the gmock header as a system header?

Logs clangd.log

System information

Output of clangd --version:

Ubuntu clangd version 18.1.3 (++20240322073153+ef6d1ec07c69-1~exp1~20240322193300.86)
Features: linux+grpc
Platform: x86_64-pc-linux-gnu

Editor/LSP plugin: VSCode with Clangd extension

Operating system: Ubuntu 20.04, but running in a docker container based on Ubuntu 22.04. Clangd is clangd-18 from the LLVM apt repository.

philippewarren avatar Apr 03 '24 20:04 philippewarren

This is not specific to clangd, I can reproduce this issue using clang-tidy on the command line.

With test.cpp containing the following, and compile_commands.json containing suitable -isystem flags:

#include <gtest/gtest.h>
#include <gmock/gmock.h>

MATCHER_P(PointEq, p, "stuff") {}

I get:

$ clang-tidy --system-headers=0 --checks=modernize-use-trailing-return-type test.cpp
12906 warnings generated.
test.cpp:4:11: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
    4 | MATCHER_P(PointEq, p, "stuff") {}
      |           ^
Suppressed 12935 warnings (12905 in non-user code, 30 NOLINT).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.

Could you file it upstream at https://github.com/llvm/llvm-project/issues please?

HighCommander4 avatar Apr 05 '24 06:04 HighCommander4