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

[David Blaikie via Phabricator via llvm-commits]

dblaikie 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);
----------------
scott.linder wrote:
> 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
I'm not following how this example (the godbolt link) is analogous to the code being tested here.

In the code under test, it's more equivalent to this (ignoring moves, just focussing on copies):
```
struct t1 {
  t1();
  t1(const t1&);
};
struct t2 {
  t1 m1;
  t2(const t1& m1) : m1(m1) { }
  t2(const t2&);
};
t2 f1(const t1& v1) {
  return t2(v1);
}
void f2() {
  t2 v2 = f1(t1());
}
```
And in this code the copy from the m1 parameter to the m1 member (there was nothing like this in the godbolt link - it's an implementation detail of the `g` function in your example) must be a copy (or a move if we were doing all the move things) - everything else can be elided (or is required to be elided in certain versions of C++). If `f1` took its parameter by value, similarly - that could be elided (since `t1()` is a temporary, so the compiler can construct that temporary right in the by-value parameter location and wouldn't need to copy/move it into position)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100670