Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[intro.races] Clarify 'synchronization operation'. #1612

Closed
wants to merge 1 commit into from

Conversation

jensmaurer
Copy link
Member

Fixes #1611 and CWG 2297.

source/intro.tex Outdated
The library identifies certain operations as
\defnx{synchronization operations}{synchronization operation}
(\ref{support.signal}, \ref{util.smartptr.shared.atomic}, \ref{atomics.order},
\ref{atomics.fences}, Clause~\ref{thread}).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably mention the Synchronization: element used in library descriptions here.

[support.signal] doesn't seem to define any synchronization operations.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

regarding [support.signal]: "A call to the function signal synchronizes with any resulting invocation of the signal handler so installed."

@jensmaurer
Copy link
Member Author

Rebased and addressed the review comment, although we do say "synchronizes with" outside of that library element, too.

@zygoloid zygoloid added the needs rebase The pull request needs a git rebase to resolve merge conflicts. label Nov 12, 2017
@cplusplus cplusplus deleted a comment from jensmaurer Nov 12, 2017
@cplusplus cplusplus deleted a comment from tkoeppe Nov 12, 2017
@jensmaurer jensmaurer added lwg Issue must be reviewed by LWG. and removed needs rebase The pull request needs a git rebase to resolve merge conflicts. labels Nov 12, 2017
@jensmaurer
Copy link
Member Author

LWG should decide how they tag synchronization operations: via a "Synchronizes" element and/or by uttering "synchronizes with" and/or via something else.

source/basic.tex Outdated
\pnum
Certain library calls \defn{synchronize with} other library calls performed by
another thread
(\ref{support.signal}, \ref{util.smartptr.atomic}, \ref{atomics.order},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer the list of sections to be more clearly non-normative.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm... I could move it down into the note after "The specifications of the synchronization operations", but I'd actually prefer to keep the list here, right at the top. A missed cross-reference is at most an editorial bug.

(Also, the library clauses should make sure they have a cross-reference to this section whenever they mention "synchronize with". This would then be a clear indicator that we mean the normative cross-thread meaning of "synchronize with". Similar to what we did with odr-use in the core section.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there other places where the standard, at the point where it defines a term, provides cross-references to uses of the term? I can't recall any, and it doesn't seem like a good practice- it would be hard to maintain, and the existence of the list would create the impression that the list is exhaustive (and thereby creating confusion around any use of "synchronize with" that's omitted from the list).

I'm fine with having uses of this term cross-reference the definition, of course, just not vice-versa.

I'd also be OK with making this list explicitly non-exhaustive, e.g. by starting it with "such as", although I still wouldn't understand why it's there.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@geoffromer: The SFINAE rule lists the cases where it applies in a non-normative note [temp.deduct] p11.
I've moved the list of cross-references to the note; it still seems useful to the reader to have some cross-references to give an impression what those synchronization operations might be.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@jensmaurer
Copy link
Member Author

@zygoloid: I think we can move forward here now.

@zygoloid
Copy link
Member

zygoloid commented Apr 1, 2018

I'm sorry, after staring at this wording change for a while and trying to think through the consequences, I can't convince myself that it preserves the intention -- in particular, that the initial struck text does not need to be retained, and that identifying "synchronization operations" with operations that "synchronize with" is correct (also, what happens if there is nothing for an operation to synchronize with? is that a synchronization operation still, or not?).

So while I think this may well be correct, and certainly seems like an improvement, I don't see a path to making this change editorially. (Perhaps while you're there, you could also suggest a fix for the "For atomic objects, the definition is clear." sentence, which serves no purpose other than to infuriate the readers for whom the definition is /not/ clear?)

@jensmaurer
Copy link
Member Author

@zygoloid: Synchronization operations always "synchronize with" something else, otherwise they couldn't contribute to the happens-before relationship [intro.races] p9.
So, is this due for a paper for SG1 and CWG? Or just a core issue where SG1 gives a nod?

@jensmaurer jensmaurer added cwg Issue must be reviewed by CWG. not-editorial Issue is not deemed editorial; the editorial issue is kept open for tracking. labels Apr 3, 2018
@jensmaurer
Copy link
Member Author

This is also covered by LWG 2506.

@jensmaurer jensmaurer added the lwg Issue must be reviewed by LWG. label Apr 13, 2018
@jensmaurer
Copy link
Member Author

Discussion on LWG 2506 in SG1 asked for a paper, which is well beyond the scope of an editorial pull request.

@jensmaurer jensmaurer closed this Apr 13, 2018
@jensmaurer jensmaurer deleted the b13 branch October 9, 2018 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cwg Issue must be reviewed by CWG. lwg Issue must be reviewed by LWG. not-editorial Issue is not deemed editorial; the editorial issue is kept open for tracking.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants