Do we still need the self move protection in APInt's move assignment operator?

Project Infrastructure LLVM Dev List Archives
1

The comment says it was added due to a bug in MSVC 2013 implementation of stable_sort. Newer versions of MSVC have fixed this bug in stable_sort according to this webpage record https://connect.microsoft.com/VisualStudio/feedback/details/886652

Can we remove the check now?

2

I think so. Besides, recommended VS version is VS 2015 [1], the bug
should be long gone.

[1] http://llvm.org/docs/GettingStarted.html#host-c-toolchain-both-compiler-and-standard-library

Regards,
chenwj

3

The way the code is currently written we only do the self-move check on the multiword case. But on if we do a self move on a single word, we’ll end up calling a memcpy with src and dest the same which is UB. So either way this code needs to change. We need to either move it up to cover single word too or remove it entirely.

4

Then I prefer removing to entirely. It’s weird to write single/multi word self move protection to workaround a bug of the host compiler whose version we don’t support. Note on [1], it says:

For the most popular host toolchains we check for specific minimum versions in our build systems

I don’t play LLVM on Windows. But if the build system does check the minimum version, I think we should remove
the protection entirely.

[1] http://llvm.org/docs/GettingStarted.html#host-c-toolchain-both-compiler-and-standard-library

Regards,
chenwj