This is an archive of the discontinued LLVM Phabricator instance.

[ADT] Add makeVisitor to STLExtras.h
ClosedPublic

Authored by scott.linder on Apr 16 2021, 11:32 AM.

Details

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.

Diff Detail

Event Timeline

scott.linder created this revision.Apr 16 2021, 11:32 AM
scott.linder requested review of this revision.Apr 16 2021, 11:32 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 16 2021, 11:32 AM

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 edited the summary of this revision. (Show Details)
dblaikie added inline comments.May 21 2021, 12:48 PM
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.

859–860

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 added inline comments.May 21 2021, 2:12 PM
llvm/unittests/ADT/STLExtrasTest.cpp
859–860

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 added inline comments.May 21 2021, 2:50 PM
llvm/unittests/ADT/STLExtrasTest.cpp
859–860

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 added inline comments.May 24 2021, 6:57 AM
llvm/unittests/ADT/STLExtrasTest.cpp
859–860

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 added inline comments.May 24 2021, 4:59 PM
llvm/unittests/ADT/STLExtrasTest.cpp
859–860

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 added inline comments.May 26 2021, 1:06 PM
llvm/unittests/ADT/STLExtrasTest.cpp
859–860

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!

Rebase and remove incorrect comments

dblaikie accepted this revision.Jun 14 2021, 7:00 PM

Looks good, thanks!

This revision is now accepted and ready to land.Jun 14 2021, 7:00 PM
This revision was landed with ongoing or failed builds.Jun 28 2021, 12:36 PM
This revision was automatically updated to reflect the committed changes.

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

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 added inline comments.Jul 1 2021, 11:16 AM
llvm/unittests/ADT/STLExtrasTest.cpp
834

(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.

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