llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.88k stars 11.93k forks source link

`modernize-use-emplace` fix-it leads to compilation error #104565

Open firewave opened 2 months ago

firewave commented 2 months ago
#include <list>
#include <string>
#include <vector>

struct S {
    S(std::list<std::string> l)
        : l(std::move(l))
    {}
    std::list<std::string> l;
};

void f()
{
    std::list<std::string> l;
    std::vector<S> v;
    v.push_back({std::move(l)});
}
<source>:16:7: warning: use emplace_back instead of push_back [modernize-use-emplace]
   16 |     v.push_back({std::move(l)});
      |       ^~~~~~~~~~~            ~
      |       emplace_back(

https://godbolt.org/z/33981TM1n

Fixing it as suggested will lead the following error:

<source>:16:7: error: no matching member function for call to 'emplace_back' [clang-diagnostic-error]
   16 |     v.emplace_back({std::move(l)});
      |     ~~^~~~~~~~~~~~
/opt/compiler-explorer/gcc-snapshot/lib/gcc/x86_64-linux-gnu/15.0.0/../../../../include/c++/15.0.0/bits/stl_vector.h:1310:2: note: candidate template ignored: substitution failure: deduced incomplete pack <(no value)> for template parameter '_Args'
 1303 |       template<typename... _Args>
      |                            ~~~~~
 1304 | #if __cplusplus > 201402L
 1305 |         _GLIBCXX20_CONSTEXPR
 1306 |         reference
 1307 | #else
 1308 |         void
 1309 | #endif
 1310 |         emplace_back(_Args&&... __args);
      |         ^

https://godbolt.org/z/vz1shKrvf

llvmbot commented 2 months ago

@llvm/issue-subscribers-clang-tidy

Author: Oliver Stöneberg (firewave)

``` #include <list> #include <string> #include <vector> struct S { S(std::list<std::string> l) : l(std::move(l)) {} std::list<std::string> l; }; void f() { std::list<std::string> l; std::vector<S> v; v.push_back({std::move(l)}); } ``` ``` <source>:16:7: warning: use emplace_back instead of push_back [modernize-use-emplace] 16 | v.push_back({std::move(l)}); | ^~~~~~~~~~~ ~ | emplace_back( ``` https://godbolt.org/z/33981TM1n Fixing it as suggested will lead the following error: ``` <source>:16:7: error: no matching member function for call to 'emplace_back' [clang-diagnostic-error] 16 | v.emplace_back({std::move(l)}); | ~~^~~~~~~~~~~~ /opt/compiler-explorer/gcc-snapshot/lib/gcc/x86_64-linux-gnu/15.0.0/../../../../include/c++/15.0.0/bits/stl_vector.h:1310:2: note: candidate template ignored: substitution failure: deduced incomplete pack <(no value)> for template parameter '_Args' 1303 | template<typename... _Args> | ~~~~~ 1304 | #if __cplusplus > 201402L 1305 | _GLIBCXX20_CONSTEXPR 1306 | reference 1307 | #else 1308 | void 1309 | #endif 1310 | emplace_back(_Args&&... __args); | ^ ``` https://godbolt.org/z/vz1shKrvf
MitalAshok commented 2 months ago

The suggestion is this:

@@ -13,6 +13,6 @@
 {
     std::list<std::string> l;
     std::vector<S> v;
-    v.push_back({std::move(l)});
+    v.emplace_back(std::move(l));
 }

Which does compile (even when automatically fixed with -fix): https://godbolt.org/z/5PvWMrYrE

firewave commented 2 months ago

Uh, that is very subtle.