[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

Tue Jul 6 11:57:08 PDT 2021

mizvekov added a comment.

In D99005#2860021 <https://reviews.llvm.org/D99005#2860021>, @aaron.ballman wrote:

> I would argue it gives the committee valuable implementation experience feedback: the change breaks at least one popular system header.

But we already knew that :-)
Reverting the change globally would stop us from finding further affected entities.
But it is a given that because of this STL bug, this is affecting our ability to get that feedback on user code in that ecosystem as well.

> I don't think it's reasonable to leave this on by default for MSVC compatibility; it's not like `std::ifstream` is an obscure part of the STL or there's only one version of the STL that's impacted. The suggestions I have for how to move forward are:
>
> - Add a compatibility hack that recognizes using the STL as a system header in a limited way to disable the diagnostics and get the old behavior (we do this on occasion for libstdc++, no reason we couldn't do something similar for MSVC STL). Then old headers still work without needing a flag and new (fixed) headers never see the issue. Someday in the distant future, we can remove the compatibility hack.
> - Or, add a command line flag that defaults off for `-fms-compatibility` and defaults on otherwise. The default can be switched based on the compatibility version being used (perhaps) once the STL has been fixed.

I find interesting the idea of combining these two options. We can suppress the effect when compiling with fms-compatibility but only on the STL headers themselves.

> I'd prefer to keep existing code working without requiring users to enable a flag, if at all possible (esp because I don't expect the flag to be needed forever). That said, I still think it makes sense to temporarily revert this while doing that work.

I'd like to give some time for other stakeholders to give their opinion if this is not too urgent, specially @Quuxplusone and @rsmith.
But thinking of another option, almost as easy as a global revert for this, we could suppress the effect under -fms-compatibility for now, and then implement a more targetted fix where we only suppress this when parsing the STL system headers themselves under fms-compatibility.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99005