[clang-tidy] modernize-use-emplace suggests C++20 code under C++17

[maflcko]

Assuming a C++ file with:

#include <vector>

struct Marker {
    Marker();
};

struct Number {
    int num;
    Marker dummy;
};

int main() {
    std::vector<Number> vec;
    vec.emplace_back(Number{.num = 2});
    // vec.emplace_back(2); // C++20
}

Clang Tidy will print:

[<source>:14:22: warning: unnecessary temporary object created while calling emplace_back [modernize-use-emplace]]
   14 |     vec.emplace_back(Number{.num = 2});
      |                      ^~~~~~~~~~~~~~~~
1 warning generated.

This is wrong, when using C++17, because fixing the warning requires C++20.

Godbolt: https://godbolt.org/z/xn1Y3xMYo

[maflcko]

cc @BigPeet from 7db780d

[BigPeet]

Hi @maflcko,

while this false-positive is irritating, the code in your example is not exactly standard C++17. Designated initializers are only standardized with C++20 (see https://en.cppreference.com/w/cpp/language/initialization).
The compiler only accepts your code with "-std=c++17" due to a non-standard extension.

You can simply verify this by adding the "-pedantic" flag: https://godbolt.org/z/1Px567YK6

warning: C++ designated initializers only available with '-std=c++20' or '-std=gnu++20' [-Wc++20-extensions]

IMO, the check could still be updated, but I'm not sure it should.
If you write non-standard code, you can expect false-positives in certain cases.
But I don't know what llvm's / clang-tidy's overall stance in such cases is.

(I.e. since clang supports this non-standard extensions, it can be argued that clang-tidy should handle it correctly.)

[maflcko]

Ah, sorry, I forgot to remove them. The same should happen for emplace_back(Number{2}), which I think is standard C++11.

[BigPeet]

Ah, okay. Then you're right and this is an issue.

I guess the check would need to verify that an initialization with the given "args..." would be actually valid, since in some cases (e.g. aggregate initialization) the parameter list might be incomplete or a type might not have a matching (implicit) constructor. If not, then no warning should be issued.

At least that would be my first idea. I would be interested in fixing this, but don't have that much time currently. I will update here, if/when I find time to take a look.

[maflcko]

What I found is that clang-tidy works correctly if the Marker dummy; field is removed. But I couldn't yet figure out why that would influence the AST matcher query.

[maflcko]

Fixed in 03ef103