[PATCH] D80433: [clang-tblgen][CommandLine][ManagedStatic] Fix build errors with clang-tblgen in Debug mode using MSVC 2019 v16.6

Reid "Away June-Sep" Kleckner via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat May 23 10:03:29 PDT 2020


rnk added a comment.

To restate what I know, here is what I think is going on:

- VC++ used to have a constexpr constructor compiler bug.
- The MSVC STL worked around it in std::atomic, this was tracked by the internal number VSO-406237
- C1XX fixed the constexpr bug
- The MSVC STL removed the work around in this github pull request: https://github.com/microsoft/STL/issues/336 / diff https://github.com/microsoft/STL/pull/390/commits/92d1e1f4d9b2c0c52b145adb598f0e27ff0203d2

Once that is done, the default constructor of `std::atomic<void*>` becomes constexpr, but it is no longer trivial (it does zero init). ManagedStaticBase's constructor, however, cannot be constexpr unless it initializes all fields. So, if it is not constexpr, and it is not trivial, then it requires dynamic initialization, which was never desired. :( 💥

This annoyingly breaks our code, but strictly speaking, our workaround wasn't ever supposed to work (std::atomic wasn't supposed to be trivially default constructible). I suspect we are not alone, but such is life, the fix looks good to me.



================
Comment at: llvm/include/llvm/Support/ManagedStatic.h:46
+// Check MSVC version here
+// (https://docs.microsoft.com/en-us/cpp/preprocessor/predefined-macros?redirectedfrom=MSDN&view=vs-2019).
+#if !defined(_MSC_VER) || (_MSC_VER >= 1925) || defined(__clang__)
----------------
RKSimon wrote:
> I'm not sure what the style rules are about exceeding the column-80 limit for URLs. Maybe just refer people to http://llvm.org/PR41367 ?
I agree with @RKSimon, this comment seems like too much detail. The original comment already explains why the code is written the way it is. The only new information is that "affected versions" are now known to be `_MSC_VER < 1925`, which is clearly expressed by the code. I think you can remove the comment edits. Whenever we drop support for pre-1925 versions of MSVC, someone will come back here and remove all this. The VS dev community links are also probably not super stable.

Re: URLs in comments, yes, they generally are the most common exception to the line limit style rule, but I don't think they add much value in this case.


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

https://reviews.llvm.org/D80433





More information about the llvm-commits mailing list