[PATCH] D100670: [ADT] Add makeVisitor to STLExtras.h

[Scott Linder via Phabricator via llvm-commits]

scott.linder added inline comments.


================
Comment at: llvm/unittests/ADT/STLExtrasTest.cpp:844-845
+    (void)V;
+    // FIXME: Can a C++14 compiler elide this temporary entirely? As of C++20
+    // this will be required (i.e. no temporary may exist in this case.)
+    EXPECT_EQ(1u, Moves);
----------------
dblaikie wrote:
> Which temporary? (perhaps the comment could be more descriptive/clear) The "Functor" temporary that is eventually moved into the visitor object? I don't think that can be elided (because at the time it's created, in the caller's stack, the final location inside the visitor object is not known - so it can't be constructed in-place)
Fair point about the ambiguity, but yes I was angling at the `Functor` temporary

I tried to get some practical answer by feeding something similar into a C++14 compiler and a C++17 compiler and asking both to not elide copies. Note that there is no call to the move constructor `Foo(Foo&&)` in the listing from the C++17 compiler https://godbolt.org/z/aMdbYT51h

I'm not clear why there is a 1 move constructor call before the lexical block containing this comment ends, as without the -fno-elide-constructors flag the C++14 compiler doesn't move or copy anything in the godbolt example


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100670/new/

https://reviews.llvm.org/D100670