Skip to content

<thread>: implement LWG-3788#3130

Merged
StephanTLavavej merged 2 commits into
microsoft:mainfrom
strega-nil:lwg-3788
Oct 12, 2022
Merged

<thread>: implement LWG-3788#3130
StephanTLavavej merged 2 commits into
microsoft:mainfrom
strega-nil:lwg-3788

Conversation

@strega-nil-ms
Copy link
Copy Markdown
Contributor

We implemented self-assign by stopping and joining; this was a bad idea, and there is now an LWG issue to say that we shouldn't do that.

See LWG-3788.

We implemented self-assign by stopping and joining; this was a bad idea,
and there is now an LWG issue to say that we shouldn't do that.
@strega-nil-ms strega-nil-ms requested a review from a team as a code owner September 26, 2022 15:58
@strega-nil-ms strega-nil-ms added the bug Something isn't working label Sep 26, 2022
@strega-nil-ms strega-nil-ms changed the title <thread>: fix LWG-3788 <thread>: implement LWG-3788 Sep 26, 2022
@StephanTLavavej
Copy link
Copy Markdown
Member

@strega-nil-ms If possible, I strongly recommend writing a Proposed Resolution for LWG-3788 if you want it to be accepted this decade - issues without Proposed Resolutions tend to linger for years, while those with simple and uncontroversial Proposed Resolutions can be quickly accepted within a meeting or two.

@StephanTLavavej
Copy link
Copy Markdown
Member

This seems to be somewhat more daring than we usually are for not-yet-accepted LWG issues (that change the Standardese 180 degrees, instead of merely clarifying something ambiguous). In the past, we've been burned by speculatively implementing "obviously correct" fixes that later turned out to be controversial (e.g. future's blocking destructor, IIRC).

That said, I think it's reasonable to be slightly daring here, as the existing self-move-assign behavior is so wacky, being a no-op is much more intuitive, and (importantly) this is a hopefully obscure case in relatively new machinery, so the possible impact is small.

✅ No modules impact.

@strega-nil-ms
Copy link
Copy Markdown
Contributor Author

strega-nil-ms commented Sep 29, 2022

Jonathan Wakely has put forward a resolution to this issue that the LWG mailing list seems happy about, which is in line with this implementation.

@StephanTLavavej StephanTLavavej self-assigned this Oct 11, 2022
@StephanTLavavej
Copy link
Copy Markdown
Member

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej
Copy link
Copy Markdown
Member

I've pushed a merge with main to resolve a simple merge conflict with an updated WP citation in <thread> - this PR's change supersedes that.

@StephanTLavavej StephanTLavavej merged commit b29a16b into microsoft:main Oct 12, 2022
@StephanTLavavej
Copy link
Copy Markdown
Member

Thanks for fixing this wacky behavior in both the Standard and our implementation! 😹 😸 🎉

@strega-nil-ms strega-nil-ms deleted the lwg-3788 branch December 19, 2022 23:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants