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

[atomic.ref.generic] Issues regarding P3323R1 #7556

Open
xmcgcg opened this issue Jan 10, 2025 · 3 comments
Open

[atomic.ref.generic] Issues regarding P3323R1 #7556

xmcgcg opened this issue Jan 10, 2025 · 3 comments
Labels
lwg Issue must be reviewed by LWG. not-editorial Issue is not deemed editorial; the editorial issue is kept open for tracking.

Comments

@xmcgcg
Copy link
Contributor

xmcgcg commented Jan 10, 2025

Issue 1: The paper adds this paragraph to multiple sections:

The program is ill-formed if is_always_lock_free is false and is_volatile_v<T> is true.

For the main template, it is OK, but there are issues with the (partial) specializations:

  • For the integral and floating-point specializations, there is no template parameter T, so is_volatile_v<T> cannot be well-formed.
  • For the pointer partial specialization, T refers to the pointed-to type, testing whether it is volatile is definitely not the intention of the author.

Issue 2: The paper adds this paragraph to [atomics.ref.pointer]:

There are specializations of the atomic_ref class template for all pointer-to-object types. For each such
type pointer-type, the specialization atomic_ref<pointer-type> provides additional atomic operations
appropriate to pointer types.

This paragraph seems to identify the pointer specializations as explicit (full) specializations, but it is impossible to enumerate all pointer(-to-object) types. The wording also implies that pointer-to-function types should match the main template, which is currently not the case. Currently, pointer-to-function types match the partial specialization, and such specializations can invoke the member functions that are also provided in the main template (the additional operations are guarded by [atomics.ref.pointer] p6). We should not change this.

We should keep the partial specialization, and we need to redefine atomic_ref<pointer-type> to make use of the template parameter T.

Suggested resolution (I can file a PR if CWG agrees with this direction):

  • [atomics.ref.int] p2: Replace is_volatile_v<T> with is_volatile_v<integral-type>.
  • [atomics.ref.float] p2: Replace is_volatile_v<T> with is_volatile_v<floating-point-type>.
  • [atomics.ref.pointer] p1: Replace paragraph with:

There are partial specializations of the atomic_ref class template for all pointer types. For each type-id pointer-type among T*, T* const, T* volatile, and T* const volatile, the partial specialization atomic_ref<pointer-type> provides additional atomic operations appropriate to pointer-to-object types.

  • [atomics.ref.pointer] p2: Replace is_volatile_v<T> with is_volatile_v<pointer-type>.
@jwakely
Copy link
Member

jwakely commented Jan 10, 2025

This feels non-editorial.

The suggestion for integral and floating point specializations seems fine.

The suggested wording for pointers seems wrong. It could be read as saying that for all pointer types (including function pointers and void pointers) there are operations appropriate to object pointers, i.e. arithmetic would be possible on all pointer types. That is not the intention (as was classified by an old lwg issue).

@jensmaurer
Copy link
Member

The general concerns here seem well-founded, but this is not something that can be fixed editorially.

Please submit an LWG issue; see here: https://isocpp.org/std/submit-issue

@jensmaurer jensmaurer added lwg Issue must be reviewed by LWG. not-editorial Issue is not deemed editorial; the editorial issue is kept open for tracking. labels Jan 10, 2025
@frederick-vs-ja
Copy link
Contributor

This paragraph seems to identify the pointer specializations as explicit (full) specializations, but it is impossible to enumerate all pointer(-to-object) types.

I don't agree on this... But the currently wording in [atomics.ref.pointer] says

template<class T> struct atomic_ref<@\placeholder{pointer-type}@> {

which is clearly wrong.

Also, the synopsis in [atomics.syn] was mistakenly left unchanged.

draft/source/threads.tex

Lines 2423 to 2424 in 9401d5f

// \ref{atomics.ref.pointer}, partial specialization for pointers
template<class T> struct atomic_ref<T*>; // freestanding

The wording also implies that pointer-to-function types should match the main template, which is currently not the case. Currently, pointer-to-function types match the partial specialization, and such specializations can invoke the member functions that are also provided in the main template (the additional operations are guarded by [atomics.ref.pointer] p6). We should not change this.

I don't see why we shouldn't change this. It's probably a deliberate design change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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

No branches or pull requests

4 participants