-
Notifications
You must be signed in to change notification settings - Fork 769
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
Conversation
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}). |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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."
Rebased and addressed the review comment, although we do say "synchronizes with" outside of that library element, too. |
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}, |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.)
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
@zygoloid: I think we can move forward here now. |
Fixes CWG2297.
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?) |
@zygoloid: Synchronization operations always "synchronize with" something else, otherwise they couldn't contribute to the happens-before relationship [intro.races] p9. |
This is also covered by LWG 2506. |
Discussion on LWG 2506 in SG1 asked for a paper, which is well beyond the scope of an editorial pull request. |
Fixes #1611 and CWG 2297.