[ADT] Add makeVisitor to STLExtras.h

[scott.linder]

Summary

Adds a utility to combine multiple Callables into a single Callable.
This is useful to make constructing a visitor for std::visit-like
functions more natural; functions like this will be added in future
patches.

Intended to supercede https://reviews.llvm.org/D99560 by
perfectly-forwarding the combined Callables.

[dblaikie]

The intent is that this supersedes/is an alternative to https://reviews.llvm.org/D99560 ? (probably worth mentioning that in the patch/review description then)

[scott.linder]

• Update description to reference https://reviews.llvm.org/D99560
• Move code to fall under "Extra additions to <utility>"

[dblaikie]

llvm/unittests/ADT/STLExtrasTest.cpp

210–211

There's a lot of, what seem to be, unrelated test changes in this patch - if they are needed (to account for refactoring of the Range type, for instance) - perhaps they could go in a separate patch?

(though on this particular change - I don't think it's an improvement (to change "X y = 0; Y z = 0; .." to "X y{}, z{}") - I'd say leave it as-is, change the type if needed, and the order of declarations if that helps makes things more consistent.

808–809

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)

[scott.linder]

llvm/unittests/ADT/STLExtrasTest.cpp

808–809

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

[dblaikie]

llvm/unittests/ADT/STLExtrasTest.cpp

808–809

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)

[scott.linder]

llvm/unittests/ADT/STLExtrasTest.cpp

808–809

Ah, right, the copy/move is occurring inside of makeVisitor. Then I think my TODO could be "C++14 compilers are not required to elide this temporary Functor, so Moves could also be 2u here" but if the existing tests are doing OK then I imagine every compiler is eliding it anyway?

[dblaikie]

llvm/unittests/ADT/STLExtrasTest.cpp

808–809

Given makeVisitor takes the parameters by reference, not value (right?) I don't think there's any scope for an implementation to produce answers other than the ones tested here - (1 move, 0 copies, 1 destruction). If the parameter was passed by value, I think /maybe/ there would be room for implementation variation.

[scott.linder]

llvm/unittests/ADT/STLExtrasTest.cpp

808–809

Right, I was missing a few things :^) I think I now understand where the move occurs and why it is moved from exactly once. I'll remove the comments when I make the other changes you suggested!

[scott.linder]

Rebase and remove incorrect comments

[scott.linder]

Ping

[dblaikie]

Looks good, thanks!

[scott.linder]

rebase

[stella.stamenova]

A couple of the new test cases are consistently failing on Windows when building Debug. They do pass in Release

2021-07-01T09:35:59.0150364Z -- Testing: 76257 tests, 32 workers --
2021-07-01T10:01:33.7697594Z Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80..
2021-07-01T10:01:33.7699468Z FAIL: LLVM-Unit :: ADT/./ADTTests.exe/STLExtrasTest.MakeVisitorDefaultCase (67224 of 76257)
2021-07-01T10:01:33.7701921Z ******************** TEST 'LLVM-Unit :: ADT/./ADTTests.exe/STLExtrasTest.MakeVisitorDefaultCase' FAILED ********************
2021-07-01T10:01:33.7705176Z Script:
2021-07-01T10:01:33.7707482Z --
2021-07-01T10:01:33.7708449Z D:\a\_work\1\b\llvm\Debug\unittests\ADT\.\ADTTests.exe --gtest_filter=STLExtrasTest.MakeVisitorDefaultCase
2021-07-01T10:01:33.7710536Z --
2021-07-01T10:01:33.7711290Z Note: Google Test filter = STLExtrasTest.MakeVisitorDefaultCase
2021-07-01T10:01:33.7711592Z 
2021-07-01T10:01:33.7712142Z [==========] Running 1 test from 1 test suite.
2021-07-01T10:01:33.7712386Z 
2021-07-01T10:01:33.7712823Z [----------] Global test environment set-up.
2021-07-01T10:01:33.7713592Z 
2021-07-01T10:01:33.7717503Z [----------] 1 test from STLExtrasTest
2021-07-01T10:01:33.7718505Z 
2021-07-01T10:01:33.7720213Z [ RUN      ] STLExtrasTest.MakeVisitorDefaultCase
2021-07-01T10:01:33.7720490Z 
2021-07-01T10:01:33.7721127Z D:\a\_work\1\s\llvm\unittests\ADT\STLExtrasTest.cpp(799): error: Expected equality of these values:
2021-07-01T10:01:33.7721574Z 
2021-07-01T10:01:33.7721953Z   Visitor(2.)
2021-07-01T10:01:33.7722097Z 
2021-07-01T10:01:33.7722524Z     Which is: 00007FF724E66A30
2021-07-01T10:01:33.7722710Z 
2021-07-01T10:01:33.7723131Z   "unhandled type"
2021-07-01T10:01:33.7723283Z 
2021-07-01T10:01:33.7723697Z     Which is: 00007FF724E53048
2021-07-01T10:01:33.7723881Z 
2021-07-01T10:01:33.7724532Z D:\a\_work\1\s\llvm\unittests\ADT\STLExtrasTest.cpp(800): error: Expected equality of these values:
2021-07-01T10:01:33.7724903Z 
2021-07-01T10:01:33.7725284Z   Visitor(Visitor)
2021-07-01T10:01:33.7725438Z 
2021-07-01T10:01:33.7725887Z     Which is: 00007FF724E66A30
2021-07-01T10:01:33.7726076Z 
2021-07-01T10:01:33.7726464Z   "unhandled type"
2021-07-01T10:01:33.7726655Z 
2021-07-01T10:01:33.7727072Z     Which is: 00007FF724E530C8
2021-07-01T10:01:33.7727802Z 
2021-07-01T10:01:33.7728452Z D:\a\_work\1\s\llvm\unittests\ADT\STLExtrasTest.cpp(808): error: Expected equality of these values:
2021-07-01T10:01:33.7729535Z 
2021-07-01T10:01:33.7730317Z   Visitor(2.)
2021-07-01T10:01:33.7730682Z 
2021-07-01T10:01:33.7731329Z     Which is: 00007FF724E66A30
2021-07-01T10:01:33.7731592Z 
2021-07-01T10:01:33.7732027Z   "unhandled type"
2021-07-01T10:01:33.7732182Z 
2021-07-01T10:01:33.7732641Z     Which is: 00007FF724E531E0
2021-07-01T10:01:33.7732826Z 
2021-07-01T10:01:33.7733469Z D:\a\_work\1\s\llvm\unittests\ADT\STLExtrasTest.cpp(809): error: Expected equality of these values:
2021-07-01T10:01:33.7733845Z 
2021-07-01T10:01:33.7734228Z   Visitor(Visitor)
2021-07-01T10:01:33.7734407Z 
2021-07-01T10:01:33.7735018Z     Which is: 00007FF724E66A30
2021-07-01T10:01:33.7735513Z 
2021-07-01T10:01:33.7736034Z   "unhandled type"
2021-07-01T10:01:33.7736306Z 
2021-07-01T10:01:33.7736778Z     Which is: 00007FF724E53258
2021-07-01T10:01:33.7737025Z 
2021-07-01T10:01:33.7739580Z [  FAILED  ] STLExtrasTest.MakeVisitorDefaultCase (2 ms)
2021-07-01T10:01:33.7739859Z 
2021-07-01T10:01:33.7740503Z [----------] 1 test from STLExtrasTest (2 ms total)
2021-07-01T10:01:33.7740744Z 
2021-07-01T10:01:33.7740854Z 
2021-07-01T10:01:33.7740961Z 
2021-07-01T10:01:33.7741379Z [----------] Global test environment tear-down
2021-07-01T10:01:33.7741628Z 
2021-07-01T10:01:33.7742090Z [==========] 1 test from 1 test suite ran. (2 ms total)
2021-07-01T10:01:33.7742296Z 
2021-07-01T10:01:33.7742663Z [  PASSED  ] 0 tests.
2021-07-01T10:01:33.7742832Z 
2021-07-01T10:01:33.7743244Z [  FAILED  ] 1 test, listed below:
2021-07-01T10:01:33.7743439Z 
2021-07-01T10:01:33.7743884Z [  FAILED  ] STLExtrasTest.MakeVisitorDefaultCase
2021-07-01T10:01:33.7744073Z 
2021-07-01T10:01:33.7744183Z 
2021-07-01T10:01:33.7744290Z 
2021-07-01T10:01:33.7744639Z  1 FAILED TEST
2021-07-01T10:01:33.7744812Z 
2021-07-01T10:01:33.7744922Z 
2021-07-01T10:01:33.7745284Z ********************
2021-07-01T10:01:33.7903181Z Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80..
2021-07-01T10:01:33.8009590Z FAIL: LLVM-Unit :: ADT/./ADTTests.exe/STLExtrasTest.MakeVisitorTwoCallables (67229 of 76257)
2021-07-01T10:01:33.8011983Z ******************** TEST 'LLVM-Unit :: ADT/./ADTTests.exe/STLExtrasTest.MakeVisitorTwoCallables' FAILED ********************
2021-07-01T10:01:33.8029530Z Script:
2021-07-01T10:01:33.8061592Z --
2021-07-01T10:01:33.8062299Z D:\a\_work\1\b\llvm\Debug\unittests\ADT\.\ADTTests.exe --gtest_filter=STLExtrasTest.MakeVisitorTwoCallables
2021-07-01T10:01:33.8062912Z --
2021-07-01T10:01:33.8063391Z Note: Google Test filter = STLExtrasTest.MakeVisitorTwoCallables
2021-07-01T10:01:33.8063636Z 
2021-07-01T10:01:33.8064103Z [==========] Running 1 test from 1 test suite.
2021-07-01T10:01:33.8064308Z 
2021-07-01T10:01:33.8064738Z [----------] Global test environment set-up.
2021-07-01T10:01:33.8065209Z 
2021-07-01T10:01:33.8065628Z [----------] 1 test from STLExtrasTest
2021-07-01T10:01:33.8065845Z 
2021-07-01T10:01:33.8066280Z [ RUN      ] STLExtrasTest.MakeVisitorTwoCallables
2021-07-01T10:01:33.8066511Z 
2021-07-01T10:01:33.8067118Z D:\a\_work\1\s\llvm\unittests\ADT\STLExtrasTest.cpp(781): error: Expected equality of these values:
2021-07-01T10:01:33.8067483Z 
2021-07-01T10:01:33.8068073Z   Visitor(42)
2021-07-01T10:01:33.8068229Z 
2021-07-01T10:01:33.8068651Z     Which is: 00007FF724E66994
2021-07-01T10:01:33.8068833Z 
2021-07-01T10:01:33.8069212Z   "int"
2021-07-01T10:01:33.8069387Z 
2021-07-01T10:01:33.8069800Z     Which is: 00007FF724E51D5C
2021-07-01T10:01:33.8070133Z 
2021-07-01T10:01:33.8070722Z D:\a\_work\1\s\llvm\unittests\ADT\STLExtrasTest.cpp(782): error: Expected equality of these values:
2021-07-01T10:01:33.8071077Z 
2021-07-01T10:01:33.8071466Z   Visitor("foo")
2021-07-01T10:01:33.8071646Z 
2021-07-01T10:01:33.8072206Z     Which is: 00007FF724E66998
2021-07-01T10:01:33.8072395Z 
2021-07-01T10:01:33.8072764Z   "str"
2021-07-01T10:01:33.8073016Z 
2021-07-01T10:01:33.8073418Z     Which is: 00007FF724E51D94
2021-07-01T10:01:33.8073593Z 
2021-07-01T10:01:33.8074050Z [  FAILED  ] STLExtrasTest.MakeVisitorTwoCallables (0 ms)
2021-07-01T10:01:33.8074268Z 
2021-07-01T10:01:33.8074693Z [----------] 1 test from STLExtrasTest (0 ms total)
2021-07-01T10:01:33.8074919Z 
2021-07-01T10:01:33.8075033Z 
2021-07-01T10:01:33.8075144Z 
2021-07-01T10:01:33.8075553Z [----------] Global test environment tear-down
2021-07-01T10:01:33.8075771Z 
2021-07-01T10:01:33.8076204Z [==========] 1 test from 1 test suite ran. (2 ms total)
2021-07-01T10:01:33.8076417Z 
2021-07-01T10:01:33.8076786Z [  PASSED  ] 0 tests.
2021-07-01T10:01:33.8076958Z 
2021-07-01T10:01:33.8077371Z [  FAILED  ] 1 test, listed below:
2021-07-01T10:01:33.8077568Z 
2021-07-01T10:01:33.8078005Z [  FAILED  ] STLExtrasTest.MakeVisitorTwoCallables
2021-07-01T10:01:33.8078209Z 
2021-07-01T10:01:33.8078330Z 
2021-07-01T10:01:33.8078459Z 
2021-07-01T10:01:33.8078817Z  1 FAILED TEST
2021-07-01T10:01:33.8080335Z 
2021-07-01T10:01:33.8080521Z 
2021-07-01T10:01:33.8081113Z ********************
2021-07-01T10:02:59.3969341Z Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 
2021-07-01T10:02:59.7189730Z 
2021-07-01T10:02:59.7190464Z 1 warning(s) in tests
2021-07-01T10:02:59.7711389Z ********************
2021-07-01T10:02:59.7713124Z Failed Tests (2):
2021-07-01T10:02:59.7713692Z   LLVM-Unit :: ADT/./ADTTests.exe/STLExtrasTest.MakeVisitorDefaultCase
2021-07-01T10:02:59.7714235Z   LLVM-Unit :: ADT/./ADTTests.exe/STLExtrasTest.MakeVisitorTwoCallables
2021-07-01T10:02:59.7714448Z 
2021-07-01T10:02:59.7714545Z 
2021-07-01T10:02:59.7714906Z Testing Time: 1646.03s
2021-07-01T10:02:59.7715299Z   Skipped          :    39
2021-07-01T10:02:59.7715766Z   Unsupported      : 21785
2021-07-01T10:02:59.7716168Z   Passed           : 54327
2021-07-01T10:02:59.7716570Z   Expectedly Failed:   104
2021-07-01T10:02:59.7717012Z   Failed           :     2

[scott.linder]

In D100670#2853327, @stella.stamenova wrote:

A couple of the new test cases are consistently failing on Windows when building Debug. They do pass in Release

2021-07-01T09:35:59.0150364Z -- Testing: 76257 tests, 32 workers --
2021-07-01T10:01:33.7697594Z Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80..
2021-07-01T10:01:33.7699468Z FAIL: LLVM-Unit :: ADT/./ADTTests.exe/STLExtrasTest.MakeVisitorDefaultCase (67224 of 76257)
2021-07-01T10:01:33.7701921Z ******************** TEST 'LLVM-Unit :: ADT/./ADTTests.exe/STLExtrasTest.MakeVisitorDefaultCase' FAILED ********************
2021-07-01T10:01:33.7705176Z Script:
2021-07-01T10:01:33.7707482Z --
2021-07-01T10:01:33.7708449Z D:\a\_work\1\b\llvm\Debug\unittests\ADT\.\ADTTests.exe --gtest_filter=STLExtrasTest.MakeVisitorDefaultCase
2021-07-01T10:01:33.7710536Z --
2021-07-01T10:01:33.7711290Z Note: Google Test filter = STLExtrasTest.MakeVisitorDefaultCase
2021-07-01T10:01:33.7711592Z 
2021-07-01T10:01:33.7712142Z [==========] Running 1 test from 1 test suite.
2021-07-01T10:01:33.7712386Z 
2021-07-01T10:01:33.7712823Z [----------] Global test environment set-up.
2021-07-01T10:01:33.7713592Z 
2021-07-01T10:01:33.7717503Z [----------] 1 test from STLExtrasTest
2021-07-01T10:01:33.7718505Z 
2021-07-01T10:01:33.7720213Z [ RUN      ] STLExtrasTest.MakeVisitorDefaultCase
2021-07-01T10:01:33.7720490Z 
2021-07-01T10:01:33.7721127Z D:\a\_work\1\s\llvm\unittests\ADT\STLExtrasTest.cpp(799): error: Expected equality of these values:
2021-07-01T10:01:33.7721574Z 
2021-07-01T10:01:33.7721953Z   Visitor(2.)
2021-07-01T10:01:33.7722097Z 
2021-07-01T10:01:33.7722524Z     Which is: 00007FF724E66A30
2021-07-01T10:01:33.7722710Z 
2021-07-01T10:01:33.7723131Z   "unhandled type"
2021-07-01T10:01:33.7723283Z 
2021-07-01T10:01:33.7723697Z     Which is: 00007FF724E53048
2021-07-01T10:01:33.7723881Z 
2021-07-01T10:01:33.7724532Z D:\a\_work\1\s\llvm\unittests\ADT\STLExtrasTest.cpp(800): error: Expected equality of these values:
2021-07-01T10:01:33.7724903Z 
2021-07-01T10:01:33.7725284Z   Visitor(Visitor)
2021-07-01T10:01:33.7725438Z 
2021-07-01T10:01:33.7725887Z     Which is: 00007FF724E66A30
2021-07-01T10:01:33.7726076Z 
2021-07-01T10:01:33.7726464Z   "unhandled type"
2021-07-01T10:01:33.7726655Z 
2021-07-01T10:01:33.7727072Z     Which is: 00007FF724E530C8
2021-07-01T10:01:33.7727802Z 
2021-07-01T10:01:33.7728452Z D:\a\_work\1\s\llvm\unittests\ADT\STLExtrasTest.cpp(808): error: Expected equality of these values:
2021-07-01T10:01:33.7729535Z 
2021-07-01T10:01:33.7730317Z   Visitor(2.)
2021-07-01T10:01:33.7730682Z 
2021-07-01T10:01:33.7731329Z     Which is: 00007FF724E66A30
2021-07-01T10:01:33.7731592Z 
2021-07-01T10:01:33.7732027Z   "unhandled type"
2021-07-01T10:01:33.7732182Z 
2021-07-01T10:01:33.7732641Z     Which is: 00007FF724E531E0
2021-07-01T10:01:33.7732826Z 
2021-07-01T10:01:33.7733469Z D:\a\_work\1\s\llvm\unittests\ADT\STLExtrasTest.cpp(809): error: Expected equality of these values:
2021-07-01T10:01:33.7733845Z 
2021-07-01T10:01:33.7734228Z   Visitor(Visitor)
2021-07-01T10:01:33.7734407Z 
2021-07-01T10:01:33.7735018Z     Which is: 00007FF724E66A30
2021-07-01T10:01:33.7735513Z 
2021-07-01T10:01:33.7736034Z   "unhandled type"
2021-07-01T10:01:33.7736306Z 
2021-07-01T10:01:33.7736778Z     Which is: 00007FF724E53258
2021-07-01T10:01:33.7737025Z 
2021-07-01T10:01:33.7739580Z [  FAILED  ] STLExtrasTest.MakeVisitorDefaultCase (2 ms)
2021-07-01T10:01:33.7739859Z 
2021-07-01T10:01:33.7740503Z [----------] 1 test from STLExtrasTest (2 ms total)
2021-07-01T10:01:33.7740744Z 
2021-07-01T10:01:33.7740854Z 
2021-07-01T10:01:33.7740961Z 
2021-07-01T10:01:33.7741379Z [----------] Global test environment tear-down
2021-07-01T10:01:33.7741628Z 
2021-07-01T10:01:33.7742090Z [==========] 1 test from 1 test suite ran. (2 ms total)
2021-07-01T10:01:33.7742296Z 
2021-07-01T10:01:33.7742663Z [  PASSED  ] 0 tests.
2021-07-01T10:01:33.7742832Z 
2021-07-01T10:01:33.7743244Z [  FAILED  ] 1 test, listed below:
2021-07-01T10:01:33.7743439Z 
2021-07-01T10:01:33.7743884Z [  FAILED  ] STLExtrasTest.MakeVisitorDefaultCase
2021-07-01T10:01:33.7744073Z 
2021-07-01T10:01:33.7744183Z 
2021-07-01T10:01:33.7744290Z 
2021-07-01T10:01:33.7744639Z  1 FAILED TEST
2021-07-01T10:01:33.7744812Z 
2021-07-01T10:01:33.7744922Z 
2021-07-01T10:01:33.7745284Z ********************
2021-07-01T10:01:33.7903181Z Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80..
2021-07-01T10:01:33.8009590Z FAIL: LLVM-Unit :: ADT/./ADTTests.exe/STLExtrasTest.MakeVisitorTwoCallables (67229 of 76257)
2021-07-01T10:01:33.8011983Z ******************** TEST 'LLVM-Unit :: ADT/./ADTTests.exe/STLExtrasTest.MakeVisitorTwoCallables' FAILED ********************
2021-07-01T10:01:33.8029530Z Script:
2021-07-01T10:01:33.8061592Z --
2021-07-01T10:01:33.8062299Z D:\a\_work\1\b\llvm\Debug\unittests\ADT\.\ADTTests.exe --gtest_filter=STLExtrasTest.MakeVisitorTwoCallables
2021-07-01T10:01:33.8062912Z --
2021-07-01T10:01:33.8063391Z Note: Google Test filter = STLExtrasTest.MakeVisitorTwoCallables
2021-07-01T10:01:33.8063636Z 
2021-07-01T10:01:33.8064103Z [==========] Running 1 test from 1 test suite.
2021-07-01T10:01:33.8064308Z 
2021-07-01T10:01:33.8064738Z [----------] Global test environment set-up.
2021-07-01T10:01:33.8065209Z 
2021-07-01T10:01:33.8065628Z [----------] 1 test from STLExtrasTest
2021-07-01T10:01:33.8065845Z 
2021-07-01T10:01:33.8066280Z [ RUN      ] STLExtrasTest.MakeVisitorTwoCallables
2021-07-01T10:01:33.8066511Z 
2021-07-01T10:01:33.8067118Z D:\a\_work\1\s\llvm\unittests\ADT\STLExtrasTest.cpp(781): error: Expected equality of these values:
2021-07-01T10:01:33.8067483Z 
2021-07-01T10:01:33.8068073Z   Visitor(42)
2021-07-01T10:01:33.8068229Z 
2021-07-01T10:01:33.8068651Z     Which is: 00007FF724E66994
2021-07-01T10:01:33.8068833Z 
2021-07-01T10:01:33.8069212Z   "int"
2021-07-01T10:01:33.8069387Z 
2021-07-01T10:01:33.8069800Z     Which is: 00007FF724E51D5C
2021-07-01T10:01:33.8070133Z 
2021-07-01T10:01:33.8070722Z D:\a\_work\1\s\llvm\unittests\ADT\STLExtrasTest.cpp(782): error: Expected equality of these values:
2021-07-01T10:01:33.8071077Z 
2021-07-01T10:01:33.8071466Z   Visitor("foo")
2021-07-01T10:01:33.8071646Z 
2021-07-01T10:01:33.8072206Z     Which is: 00007FF724E66998
2021-07-01T10:01:33.8072395Z 
2021-07-01T10:01:33.8072764Z   "str"
2021-07-01T10:01:33.8073016Z 
2021-07-01T10:01:33.8073418Z     Which is: 00007FF724E51D94
2021-07-01T10:01:33.8073593Z 
2021-07-01T10:01:33.8074050Z [  FAILED  ] STLExtrasTest.MakeVisitorTwoCallables (0 ms)
2021-07-01T10:01:33.8074268Z 
2021-07-01T10:01:33.8074693Z [----------] 1 test from STLExtrasTest (0 ms total)
2021-07-01T10:01:33.8074919Z 
2021-07-01T10:01:33.8075033Z 
2021-07-01T10:01:33.8075144Z 
2021-07-01T10:01:33.8075553Z [----------] Global test environment tear-down
2021-07-01T10:01:33.8075771Z 
2021-07-01T10:01:33.8076204Z [==========] 1 test from 1 test suite ran. (2 ms total)
2021-07-01T10:01:33.8076417Z 
2021-07-01T10:01:33.8076786Z [  PASSED  ] 0 tests.
2021-07-01T10:01:33.8076958Z 
2021-07-01T10:01:33.8077371Z [  FAILED  ] 1 test, listed below:
2021-07-01T10:01:33.8077568Z 
2021-07-01T10:01:33.8078005Z [  FAILED  ] STLExtrasTest.MakeVisitorTwoCallables
2021-07-01T10:01:33.8078209Z 
2021-07-01T10:01:33.8078330Z 
2021-07-01T10:01:33.8078459Z 
2021-07-01T10:01:33.8078817Z  1 FAILED TEST
2021-07-01T10:01:33.8080335Z 
2021-07-01T10:01:33.8080521Z 
2021-07-01T10:01:33.8081113Z ********************
2021-07-01T10:02:59.3969341Z Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 
2021-07-01T10:02:59.7189730Z 
2021-07-01T10:02:59.7190464Z 1 warning(s) in tests
2021-07-01T10:02:59.7711389Z ********************
2021-07-01T10:02:59.7713124Z Failed Tests (2):
2021-07-01T10:02:59.7713692Z   LLVM-Unit :: ADT/./ADTTests.exe/STLExtrasTest.MakeVisitorDefaultCase
2021-07-01T10:02:59.7714235Z   LLVM-Unit :: ADT/./ADTTests.exe/STLExtrasTest.MakeVisitorTwoCallables
2021-07-01T10:02:59.7714448Z 
2021-07-01T10:02:59.7714545Z 
2021-07-01T10:02:59.7714906Z Testing Time: 1646.03s
2021-07-01T10:02:59.7715299Z   Skipped          :    39
2021-07-01T10:02:59.7715766Z   Unsupported      : 21785
2021-07-01T10:02:59.7716168Z   Passed           : 54327
2021-07-01T10:02:59.7716570Z   Expectedly Failed:   104
2021-07-01T10:02:59.7717012Z   Failed           :     2

Looking at this now, if I can't root cause quickly I will revert again

[scott.linder]

llvm/unittests/ADT/STLExtrasTest.cpp

818

(Accidentally posted this on the commit itself first, reposting here)

Ah, I believe my mistake was assuming EXPECT_EQ treated char * as a string, rather than like any other pointer. I didn't intend to compare the pointer values here, but it accidentally succeeds when the compiler merges the storage for both string literals.

I'll have a patch up shortly to fix the bug.

[scott.linder]

The fix should be in 83887df15597990308e9903d0480fa7676d772a1, let me know if there are still failures, and sorry for the noise!

llvm/include/llvm/ADT/STLExtras.h

 /// This implementation can be removed once we move to C++20 where it's defined
 /// as std::to_address().
 ///
 /// The std::pointer_traits<>::to_address(p) variations of these overloads has
 /// not been implemented.
 template <class Ptr> auto to_address(const Ptr &P) { return P.operator->(); }
 template <class T> constexpr T *to_address(T *P) { return P; }
 
+namespace detail {
+
+template <typename... Ts> struct Visitor;
+
+template <typename HeadT, typename... TailTs>
+struct Visitor<HeadT, TailTs...> : remove_cvref_t<HeadT>, Visitor<TailTs...> {
+  explicit constexpr Visitor(HeadT &&Head, TailTs &&...Tail)
+      : remove_cvref_t<HeadT>(std::forward<HeadT>(Head)),
+        Visitor<TailTs...>(std::forward<TailTs>(Tail)...) {}
+  using remove_cvref_t<HeadT>::operator();
+  using Visitor<TailTs...>::operator();
+};
+
+template <typename HeadT> struct Visitor<HeadT> : remove_cvref_t<HeadT> {
+  explicit constexpr Visitor(HeadT &&Head)
+      : remove_cvref_t<HeadT>(std::forward<HeadT>(Head)) {}
+  using remove_cvref_t<HeadT>::operator();
+};
+
+} // namespace detail
+
+/// Returns an opaquely-typed Callable object whose operator() overload set is
+/// the sum of the operator() overload sets of each CallableT in CallableTs.
+///
+/// The type of the returned object derives from each CallableT in CallableTs.
+/// The returned object is constructed by invoking the appropriate copy or move
+/// constructor of each CallableT, as selected by overload resolution on the
+/// corresponding argument to makeVisitor.
+///
+/// Example:
+///
+/// \code
+/// auto visitor = makeVisitor([](auto) { return "unhandled type"; },
+///                            [](int i) { return "int"; },
+///                            [](std::string s) { return "str"; });
+/// auto a = visitor(42);    // `a` is now "int".
+/// auto b = visitor("foo"); // `b` is now "str".
+/// auto c = visitor(3.14f); // `c` is now "unhandled type".
+/// \endcode
+///
+/// Example of making a visitor with a lambda which captures a move-only type:
+///
+/// \code
+/// std::unique_ptr<FooHandler> FH = /* ... */;
+/// auto visitor = makeVisitor(
+///     [FH{std::move(FH)}](Foo F) { return FH->handle(F); },
+///     [](int i) { return i; },
+///     [](std::string s) { return atoi(s); });
+/// \endcode
+template <typename... CallableTs>
+constexpr decltype(auto) makeVisitor(CallableTs &&...Callables) {
+  return detail::Visitor<CallableTs...>(std::forward<CallableTs>(Callables)...);
+}
+
 } // end namespace llvm
 
 #endif // LLVM_ADT_STLEXTRAS_H

llvm/unittests/ADT/STLExtrasTest.cpp

 template <bool B> struct CanCopy {};
 template <> struct CanCopy<false> {
   CanCopy(const CanCopy &) = delete;
 
   CanCopy() = default;
   CanCopy(CanCopy &&) = default;
 };
 
-template <bool Moveable, bool Copyable>
-struct Range : CanMove<Moveable>, CanCopy<Copyable> {
-  explicit Range(int &C, int &M, int &D) : C(C), M(M), D(D) {}
-  Range(const Range &R) : CanCopy<Copyable>(R), C(R.C), M(R.M), D(R.D) { ++C; }
-  Range(Range &&R) : CanMove<Moveable>(std::move(R)), C(R.C), M(R.M), D(R.D) {
-    ++M;
-  }
-  ~Range() { ++D; }
-
-  int &C;
-  int &M;
-  int &D;
+template <bool Movable, bool Copyable>
+class Counted : CanMove<Movable>, CanCopy<Copyable> {
+  unsigned &Moves, &Copies, &Destructs;
+
+public:
+  explicit Counted(unsigned &Moves, unsigned &Copies, unsigned &Destructs)
+      : Moves(Moves), Copies(Copies), Destructs(Destructs) {}
+  Counted(Counted &&O)
+      : CanMove<Movable>(std::move(O)), Moves(O.Moves), Copies(O.Copies),
+        Destructs(O.Destructs) {
+    ++Moves;
+  }
+  Counted(const Counted &O)
+      : CanCopy<Copyable>(O), Moves(O.Moves), Copies(O.Copies),
+        Destructs(O.Destructs) {
+    ++Copies;
+  }
+  virtual ~Counted() { ++Destructs; }
+};
 
+template <bool Moveable, bool Copyable>
+struct Range : Counted<Moveable, Copyable> {
+  using Counted<Moveable, Copyable>::Counted;
   int *begin() { return nullptr; }
   int *end() { return nullptr; }
 };
 
-TEST(STLExtrasTest, EnumerateLifetimeSemantics) {
-  // Test that when enumerating lvalues and rvalues, there are no surprise
-  // copies or moves.
+TEST(STLExtrasTest, EnumerateLifetimeSemanticsPRValue) {
+  unsigned Moves{}, Copies{}, Destructs{};
+  {
+    auto E = enumerate(Range<true, false>(Moves, Copies, Destructs));
+    (void)E;
+    // Doesn't compile.  rvalue ranges must be moveable.
+    // auto E2 = enumerate(Range<false, true>(Copies, Moves, Destructors));
+    // 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);
+    EXPECT_EQ(0u, Copies);
+    EXPECT_EQ(1u, Destructs);
+  }
+  EXPECT_EQ(1u, Moves);
+  EXPECT_EQ(0u, Copies);
+  EXPECT_EQ(2u, Destructs);
+}
 
+TEST(STLExtrasTest, EnumerateLifetimeSemanticsRValue) {
   // With an rvalue, it should not be destroyed until the end of the scope.
-  int Copies = 0;
-  int Moves = 0;
-  int Destructors = 0;
+  unsigned Moves{}, Copies{}, Destructs{};
   {
-    auto E1 = enumerate(Range<true, false>(Copies, Moves, Destructors));
+    Range<true, false> R(Moves, Copies, Destructs);
+    {
+      auto E = enumerate(std::move(R));
+      (void)E;
       // Doesn't compile.  rvalue ranges must be moveable.
       // auto E2 = enumerate(Range<false, true>(Copies, Moves, Destructors));
-    EXPECT_EQ(0, Copies);
-    EXPECT_EQ(1, Moves);
-    EXPECT_EQ(1, Destructors);
+      EXPECT_EQ(1u, Moves);
+      EXPECT_EQ(0u, Copies);
+      EXPECT_EQ(0u, Destructs);
     }
-  EXPECT_EQ(0, Copies);
-  EXPECT_EQ(1, Moves);
-  EXPECT_EQ(2, Destructors);
+    EXPECT_EQ(1u, Moves);
+    EXPECT_EQ(0u, Copies);
+    EXPECT_EQ(1u, Destructs);
+  }
+  EXPECT_EQ(1u, Moves);
+  EXPECT_EQ(0u, Copies);
+  EXPECT_EQ(2u, Destructs);
+}
 
-  Copies = Moves = Destructors = 0;
+TEST(STLExtrasTest, EnumerateLifetimeSemanticsLValue) {
   // With an lvalue, it should not be destroyed even after the end of the scope.
   // lvalue ranges need be neither copyable nor moveable.
-  Range<false, false> R(Copies, Moves, Destructors);
+  unsigned Moves{}, Copies{}, Destructs{};
+  {
+    Range<false, false> R(Moves, Copies, Destructs);
     {
-    auto Enumerator = enumerate(R);
-    (void)Enumerator;
-    EXPECT_EQ(0, Copies);
-    EXPECT_EQ(0, Moves);
-    EXPECT_EQ(0, Destructors);
+      auto E = enumerate(R);
+      (void)E;
+      EXPECT_EQ(0u, Moves);
+      EXPECT_EQ(0u, Copies);
+      EXPECT_EQ(0u, Destructs);
     }
-  EXPECT_EQ(0, Copies);
-  EXPECT_EQ(0, Moves);
-  EXPECT_EQ(0, Destructors);
+    EXPECT_EQ(0u, Moves);
+    EXPECT_EQ(0u, Copies);
+    EXPECT_EQ(0u, Destructs);
+  }
+  EXPECT_EQ(0u, Moves);
+  EXPECT_EQ(0u, Copies);
+  EXPECT_EQ(1u, Destructs);
 }
 
 TEST(STLExtrasTest, ApplyTuple) {
   auto T = std::make_tuple(1, 3, 7);
   auto U = llvm::apply_tuple(
       [](int A, int B, int C) { return std::make_tuple(A - B, B - C, C - A); },
       T);
 
TEST(STLExtras, MoveRange) {
   EXPECT_TRUE(llvm::none_of(V2, HasVal));
   EXPECT_EQ(V3.size(), 4U);
   EXPECT_TRUE(llvm::all_of(V3, HasVal));
 
   llvm::move(Build(), std::back_inserter(V4));
   EXPECT_EQ(V4.size(), 4U);
   EXPECT_TRUE(llvm::all_of(V4, HasVal));
 }
+
+TEST(STLExtras, MakeVisitorOneCallable) {
+  auto IdentityLambda = [](auto X) { return X; };
+  auto IdentityVisitor = makeVisitor(IdentityLambda);
+  EXPECT_EQ(IdentityLambda(1), IdentityVisitor(1));
+  EXPECT_EQ(IdentityLambda(2.0f), IdentityVisitor(2.0f));
+  EXPECT_TRUE((std::is_same<decltype(IdentityLambda(IdentityLambda)),
+                            decltype(IdentityLambda)>::value));
+  EXPECT_TRUE((std::is_same<decltype(IdentityVisitor(IdentityVisitor)),
+                            decltype(IdentityVisitor)>::value));
+}
+
+TEST(STLExtras, MakeVisitorTwoCallables) {
+  auto Visitor =
+      makeVisitor([](int) { return "int"; }, [](std::string) { return "str"; });
+  EXPECT_EQ(Visitor(42), "int");
+  EXPECT_EQ(Visitor("foo"), "str");
+}
+
+TEST(STLExtras, MakeVisitorCallableMultipleOperands) {
+  auto Second = makeVisitor([](int I, float F) { return F; },
+                            [](float F, int I) { return I; });
+  EXPECT_EQ(Second(1.f, 1), 1);
+  EXPECT_EQ(Second(1, 1.f), 1.f);
+}
+
+TEST(STLExtras, MakeVisitorDefaultCase) {
+  {
+    auto Visitor = makeVisitor([](int I) { return I + 100; },
+                               [](float F) { return F * 2; },
+                               [](auto) { return "unhandled type"; });
+    EXPECT_EQ(Visitor(24), 124);
+    EXPECT_EQ(Visitor(2.f), 4.f);
+    EXPECT_EQ(Visitor(2.), "unhandled type");
+    EXPECT_EQ(Visitor(Visitor), "unhandled type");
+  }
+  {
+    auto Visitor = makeVisitor([](auto) { return "unhandled type"; },
+                               [](int I) { return I + 100; },
+                               [](float F) { return F * 2; });
+    EXPECT_EQ(Visitor(24), 124);
+    EXPECT_EQ(Visitor(2.f), 4.f);
+    EXPECT_EQ(Visitor(2.), "unhandled type");
+    EXPECT_EQ(Visitor(Visitor), "unhandled type");
+  }
+}
+
+template <bool Moveable, bool Copyable>
+struct Functor : Counted<Moveable, Copyable> {
+  using Counted<Moveable, Copyable>::Counted;
+  void operator()() {}
+};
+
+TEST(STLExtrasTest, MakeVisitorLifetimeSemanticsPRValue) {
+  unsigned Moves{}, Copies{}, Destructs{};
+  {
+    auto V = makeVisitor(Functor<true, false>(Moves, Copies, Destructs));
+    (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);
+    EXPECT_EQ(0u, Copies);
+    EXPECT_EQ(1u, Destructs);
+  }
+  EXPECT_EQ(1u, Moves);
+  EXPECT_EQ(0u, Copies);
+  EXPECT_EQ(2u, Destructs);
+}
+
+TEST(STLExtrasTest, MakeVisitorLifetimeSemanticsRValue) {
+  unsigned Moves{}, Copies{}, Destructs{};
+  {
+    Functor<true, false> F(Moves, Copies, Destructs);
+    {
+      auto V = makeVisitor(std::move(F));
+      (void)V;
+      EXPECT_EQ(1u, Moves);
+      EXPECT_EQ(0u, Copies);
+      EXPECT_EQ(0u, Destructs);
+    }
+    EXPECT_EQ(1u, Moves);
+    EXPECT_EQ(0u, Copies);
+    EXPECT_EQ(1u, Destructs);
+  }
+  EXPECT_EQ(1u, Moves);
+  EXPECT_EQ(0u, Copies);
+  EXPECT_EQ(2u, Destructs);
+}
+
+TEST(STLExtrasTest, MakeVisitorLifetimeSemanticsLValue) {
+  unsigned Moves{}, Copies{}, Destructs{};
+  {
+    Functor<true, true> F(Moves, Copies, Destructs);
+    {
+      auto V = makeVisitor(F);
+      (void)V;
+      EXPECT_EQ(0u, Moves);
+      EXPECT_EQ(1u, Copies);
+      EXPECT_EQ(0u, Destructs);
+    }
+    EXPECT_EQ(0u, Moves);
+    EXPECT_EQ(1u, Copies);
+    EXPECT_EQ(1u, Destructs);
+  }
+  EXPECT_EQ(0u, Moves);
+  EXPECT_EQ(1u, Copies);
+  EXPECT_EQ(2u, Destructs);
+}
+
 } // namespace