Search code examples
c++clang-tidy

Why does clang-tidy's modernize-use-emplace miss this warning?

a.cpp:

#include <bits/stdc++.h>
using namespace std;

int main() {
    vector<pair<int, int>> a;
    a.push_back(make_pair(1, 2)); //caught

    vector<vector<pair<int, int>>> b(1);
    b[0].push_back(make_pair(1, 2)); //not caught

    return 0;
}

clang-tidy -config="{Checks: 'modernize-use-emplace'}" a.cpp

a.cpp:6:4: warning: use emplace_back instead of push_back [modernize-use-emplace]
        a.push_back(make_pair(1, 2)); //caught
          ^~~~~~~~~~~~~~~~~~~     ~
          emplace_back

clang-tidy --version

LLVM (http://llvm.org/):
  LLVM version 14.0.6
  Optimized build.
  Default target: x86_64-pc-linux-gnu
  Host CPU: skylake

clang-tidy wiki

Solution

  • Short answer: This behavior is due to bug 56721 in clang-tidy.

    Long answer, based on my comment on that bug:

    The bug relates to how reference is declared in the container. That in turn causes clang-tidy to not realize that the return value of the accessor is an instance of a relevant container.

    The core of UseEmplaceCheck.cpp is:

      auto CallPushBack = cxxMemberCallExpr(
      hasDeclaration(functionDecl(hasName("push_back"))),
      on(hasType(cxxRecordDecl(hasAnyName(SmallVector<StringRef, 5>(
          ContainersWithPushBack.begin(), ContainersWithPushBack.end()))))));

    Using the example from the question, the following simplified matcher (which can be passed to clang-query) will report the non-nested case, as expected:

      cxxMemberCallExpr(
    hasDeclaration(functionDecl(hasName("push_back"))),
    on(hasType(cxxRecordDecl(hasName("std::vector"))))
  )

    Reports:

      vector<pair<int, int>> a;
  a.push_back(make_pair(1, 2)); //caught

    However, to get it to match the nested case (when compiling with GCC-9.3.0 headers), it must be changed to:

      cxxMemberCallExpr(
    hasDeclaration(functionDecl(hasName("push_back"))),
    on(hasType(namedDecl(hasName("::__gnu_cxx::__alloc_traits<std::allocator<std::vector<std::pair<int, int>>>, std::vector<std::pair<int, int>>>::value_type"))))
  )

    Reports:

      vector<vector<pair<int, int>>> b(1);
  b[0].push_back(make_pair(1, 2)); //not caught

    That is, the type is no longer a cxxRecordDecl, and its name is a long template-id. That template-id is derived from the code used to declare std::vector::reference in the headers. It should be semantically equivalent to std::pair<int, int>, but that equivalence is evidently obscured from clang-tidy here.