[PATCH] D93510: Fix llvm::Optional build breaks in MSVC using std::is_trivially_copyable

[Alexandre Ganea via Phabricator via llvm-commits]

aganea added a subscriber: rsmith.
aganea added inline comments.


================
Comment at: llvm/unittests/ADT/OptionalTest.cpp:415
+
+#if defined(_MSC_VER) && _MSC_VER >= 1927
+// Currently only true on recent MSVC releases.
----------------
dblaikie wrote:
> aganea wrote:
> > When compiling with clang-cl 11.0 into a "x64 Native Tools Command Prompt for VS 2019" (16.8.4) cmd.exe, I see:
> > ```
> > F:\aganea\llvm-project\llvm\unittests\ADT\OptionalTest.cpp(417,1): error: static_assert failed due to requirement
> >       'std::is_trivially_copyable<(anonymous namespace)::NonTCopy>::value' "Expect NonTCopy to be trivially copyable"
> > static_assert(std::is_trivially_copyable<NonTCopy>::value,
> > ^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > F:\aganea\llvm-project\llvm\unittests\ADT\OptionalTest.cpp(465,1): error: static_assert failed due to requirement
> >       'std::is_trivially_copyable<(anonymous namespace)::NonTAssign>::value' "Expect NonTAssign to be trivially copyable"
> > static_assert(std::is_trivially_copyable<NonTAssign>::value,
> > ^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > 2 errors generated.
> > ```
> > This is because Clang's `-fms-compatibility-version` inherits its version from the `cl.exe` in `%PATH%`.
> > 
> > I've tried with Clang 11.0 and Clang 12.0 at HEAD (rGeec856848ccc481b2704ebf64d725e635a3d7dca). They both insist that `NonTCopy` isn't `__is_trivially_copyable` but MSVC 16.8.4 and GCC at HEAD says it is.
> Since I don't have a setup for this - would you mind testing (& if it works, committing) adding a `&& !defined(__clang__)` to these #ifs?
I've reverted the patch since I think it needs a bit more testing.

@dblaikie @rsmith Do you think the `NonTCopy` test case above requires fixes in Clang? (so that `__is_trivially_copyable` returns true like the other compilers)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93510