Skip to content

[simd.permute.*] Fix wording that referred to V after renaming V to M for some overloads#8132

Merged
tkoeppe merged 1 commit intocplusplus:mainfrom
mattkretz:fixup_wording_after_rename_to_template_parameter_M
Mar 27, 2026
Merged

[simd.permute.*] Fix wording that referred to V after renaming V to M for some overloads#8132
tkoeppe merged 1 commit intocplusplus:mainfrom
mattkretz:fixup_wording_after_rename_to_template_parameter_M

Conversation

@mattkretz
Copy link
Copy Markdown
Member

@mattkretz mattkretz commented Jul 29, 2025

[simd.permute.static] Now says 'Let V be M …' which is the shortest solution here. Alternatively, we could introduce T to be V::value_type/bool for the two overloads and instead of V::size() write v.size().

Also, maybe we want to rename the function parameter from v to x for the [simd.permute.*] functions?

Fixes https://github.com/cplusplus/nbballot/issues/870.

@tkoeppe
Copy link
Copy Markdown
Contributor

tkoeppe commented Jul 29, 2025

@Dani-Hub Could you take a look please?

@Dani-Hub
Copy link
Copy Markdown
Member

I'm not sure whether I get the full idea. Could you please write it down for one example, @mattkretz ?

@mattkretz
Copy link
Copy Markdown
Member Author

The alternative wording idea is only for [simd.permute.static]. I'm currently compiling a new std.pdf and will attach a screenshot ASAP.

The renaming from v to x is clear, no?

@mattkretz
Copy link
Copy Markdown
Member Author

mattkretz commented Jul 29, 2025

image See (1.1), (1.3) and the changes from `V::size()` to `v.size()`.

edit: Oops. Stray typename still in (1.3)

@Dani-Hub
Copy link
Copy Markdown
Member

Yeah the x renaming is obvious ;-) What I'm wondering is that you replace the previous static member access (V::size()) by a potentially non-static access (v.size()). This looks a design change (extension) to me. Also: Why using auto, when the return type is T? That seems less clear, especially in the presence of the unspecified value. In other words:

  • Replacing v by x looks harmless
  • I would suggest to perform the T introduction only as long we still keep with the static access, but that doesn't seem to work without the V introduction as currently suggested.
  • The usage of auto makes it less clear

@mattkretz
Copy link
Copy Markdown
Member Author

v.size() is still a static member access. Yes, Clang refuses to believe that v.size(), where v is a const-ref, is a constant expression. But that's been clarified for C++26 with P2280R4. So v.size() is equivalent to remove_reference_t<decltype(v)>::size(). (FWIW, calling .size() is a common thing to do for std::simd users.)
Or did you mean to say something else with "potentially non-static"?

Why using auto, when the return type is T?

The return type is T. The src_index variable uses auto.

[…] as long we still keep with the static access, […]

Note that I changed the other [simd.permute.*] subclauses to use v.size() instead of V::size(). So doing it here as well would be consistent.

@Dani-Hub
Copy link
Copy Markdown
Member

What I'm trying to say is that it could be a non-static access depending on the actual template argument. Personally I feel that this goes beyond an editorial change, but maybe @jwakely has a different opinion.

@mattkretz
Copy link
Copy Markdown
Member Author

The template argument is constrained to enabled specializations of std::simd::basic_vec and std::simd::basic_mask. So you are saying we might want to add partial specializations in the future that change size to be a non-static member?

@Dani-Hub
Copy link
Copy Markdown
Member

I don't want to speculate on that. This is a subtlety to me and I really don't see the editorial advantage.

@Dani-Hub
Copy link
Copy Markdown
Member

Just as an example: Unless I'm mistaken, user code could potentially specialize std::simd::basic_vec or std::simd::basic_mask for a user-provided type and this specialization could try to provide a non-static member function of that name which does funny things. It will likely not work as intended, but just for an editorial refactoring it seems not such a good idea to make things look as if they would begin an extended design supporting non-static functions.

@mattkretz
Copy link
Copy Markdown
Member Author

mattkretz commented Jul 30, 2025

Technically, a user can specialize basic_vec/basic_mask. https://eel.is/c++draft/constraints#namespace.std-2.2 requires "the specialization meets the standard library requirements for the original template". So I don't see a problem there. (There are many other problems with users specializing std::simd types, though.)

But more important to me here is the design intent behind size(). It was the intent to match the array (container) interface while additionally allowing a simple syntax for retrieving the value without an object. (After the TS I found a simple way to also pass the size as constant expressions via function arguments.) The point is that .size() was the primary motivation / interface. To me this is just like static operator[], which doesn't suddenly make it so that you'd have to call it without an object in the specification.

(edit: clarify array interface)

@mattkretz
Copy link
Copy Markdown
Member Author

mattkretz commented Jul 30, 2025

Oh, the editorial advantage is that we don't need an "Let V be M" paragraph.

To me, if I have to choose between "Let V be M. […] V::size()" and "v.size()" the latter is 100% clear whereas the former just needs an extra indirection on reading the spec.

(edit: remove useless speculation)

@mattkretz mattkretz force-pushed the fixup_wording_after_rename_to_template_parameter_M branch from ddd89a7 to 586f74c Compare November 4, 2025 15:18
@mattkretz
Copy link
Copy Markdown
Member Author

As discussed in LWG on https://github.com/cplusplus/nbballot/issues/870, the PR now simply reverts the renaming of V -> M.

@tkoeppe
Copy link
Copy Markdown
Contributor

tkoeppe commented Nov 4, 2025

Thanks, everyone! @mattkretz, could you maybe add a subsequent paragraph to the commit message pointing to the previous commit and/or pull request that changed this, so that we have a bit of rationale why this is "changing back" and why the original change didn't work out?

@tkoeppe tkoeppe requested a review from Dani-Hub November 4, 2025 16:35
@tkoeppe
Copy link
Copy Markdown
Contributor

tkoeppe commented Nov 4, 2025

Does this PR fully address https://github.com/cplusplus/nbballot/issues/870? If so, could you please add a corresponding "Fixes:" line to the first comment?

@mattkretz
Copy link
Copy Markdown
Member Author

Will do. b6e5010 is the commit I'm going to reference

@eisenwave eisenwave added the ballot-comment Response to an NB or ISO comment on a ballot label Nov 6, 2025
@tkoeppe
Copy link
Copy Markdown
Contributor

tkoeppe commented Mar 26, 2026

@mattkretz friendly ping

This was changed editorially by b6e5010
as part of "2025-06 LWG Motion 13: P3691R1 Reconsider naming of the
namespace for std::simd", but turned out not to be a helpful change,
because it makes other wording more complex.

Fixes NB US 180-295 (C++26 CD).
@tkoeppe tkoeppe force-pushed the fixup_wording_after_rename_to_template_parameter_M branch from 586f74c to 9a42b30 Compare March 27, 2026 11:10
@tkoeppe
Copy link
Copy Markdown
Contributor

tkoeppe commented Mar 27, 2026

I've made the changes.

@tkoeppe tkoeppe merged commit 731cda1 into cplusplus:main Mar 27, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ballot-comment Response to an NB or ISO comment on a ballot

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants