Skip to content

Mark templates users shouldn't specialize with _NO_SPECIALIZATIONS #5536

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

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

localspook
Copy link

@localspook localspook commented May 24, 2025

This PR marks templates users shouldn't specialize with [[msvc::no_specializations]] or [[clang::no_specializations]], depending on the compiler.

Resolves #5179.

The rule goes [namespace.std]:

  • Specializing class templates is allowed, with the exception of:

    • initializer_list
    • allocator_traits
    • coroutine_handle
    • compare_three_way_result
    • is_execution_policy
    • basic_format_arg
    • is_clock
    • generator
    • range_adaptor_closure
    • variant
    • tuple
    • unary_function
    • binary_function
  • Specializing variable templates is forbidden, with the exception of:

    • format_kind
    • disable_sized_sentinel_for
    • enable_borrowed_range
    • disable_sized_range
    • enable_view
    • enable_nonlocking_formatter_optimization
    • e_v
    • log2e_v
    • log10e_v
    • pi_v
    • inv_pi_v
    • inv_sqrtpi_v
    • ln2_v
    • ln10_v
    • sqrt2_v
    • sqrt3_v
    • inv_sqrt3_v
    • egamma_v
    • phi_v
  • Specializing any template in <type_traits> is forbidden, with the exception of:

    • common_type
    • basic_common_reference
  • Specializing function templates is forbidden, but, following the decision made in P0551R3 Thou Shalt Not Specialize std Function Templates! #24, this PR doesn't touch any of them (except for is_pointer_interconvertible_with_class and is_corresponding_member in <type_traits>, as specializing those has always been forbidden).

Relevant: PR implementing this warning in libc++

@localspook localspook requested a review from a team as a code owner May 24, 2025 00:57
@github-project-automation github-project-automation bot moved this to Initial Review in STL Code Reviews May 24, 2025
@localspook

This comment was marked as off-topic.

# AttributeMacros:
# - __capability
AttributeMacros:
- _NO_SPECIALIZATIONS
Copy link
Author

Choose a reason for hiding this comment

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

Adding the macro here is needed to avoid incorrect formatting like:

-struct _NO_SPECIALIZATIONS enable_if {};
+struct _NO_SPECIALIZATIONS enable_if{};

@localspook
Copy link
Author

This PR doesn't include tests because I'm not sure how to implement them; unless I missed something, the std testing suite has no examples of tests verifying that a warning is triggered or compilation fails (only that a warning is not triggered, or compilation succeeds).

@localspook localspook changed the title Mark templates users shouldn't specialize with _NO_SPECIALIZATIONS Mark templates users shouldn't specialize with _NO_SPECIALIZATIONS May 24, 2025
@localspook
Copy link
Author

CI is failing because libcxx/test/std/iterators/iterator.requirements/iterator.concepts/iterator.concept.readable/indirectly_readable.compile.pass.cpp triggers UB by specializing common_reference (they're aware of this):
https://github.com/llvm/llvm-project/blob/c8f29e3b319f2324d79d350f4c0aa86c178119e6/libcxx/test/std/iterators/iterator.requirements/iterator.concepts/iterator.concept.readable/indirectly_readable.compile.pass.cpp#L143-L147
I think the solution is to specialize basic_common_reference instead, but I'll look into it and open a PR against libc++ tomorrow.

@AlexGuteniev
Copy link
Contributor

CI is failing because libcxx/test/std/iterators/iterator.requirements/iterator.concepts/iterator.concept.readable/indirectly_readable.compile.pass.cpp triggers UB by specializing common_reference (they're aware of this):

I'd suggest adding to /tests/libcxx/expected_results.txt for now, so that the PR would otherwise pass CI

@@ -756,8 +758,9 @@ struct _Default_allocator_traits { // traits for std::allocator
};

_EXPORT_STD template <class _Alloc>
struct allocator_traits : conditional_t<_Is_default_allocator<_Alloc>::value, _Default_allocator_traits<_Alloc>,
_Normal_allocator_traits<_Alloc>> {};
struct _NO_SPECIALIZATIONS allocator_traits : conditional_t<_Is_default_allocator<_Alloc>::value,
Copy link
Contributor

Choose a reason for hiding this comment

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

Pedantically speaking, program-defined specialization of allocator_traits is only forbidden since C++23. But it's possibly a good thing to "extend" the restriction to old modes.

Copy link
Author

Choose a reason for hiding this comment

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

There's a clear migration path that's valid in all modes (define the relevant members in the allocator class), which could be an argument for extending the restriction.

_STD_BEGIN
_EXPORT_STD template <class _Elem>
class initializer_list {
class _NO_SPECIALIZATIONS initializer_list {
Copy link
Contributor

@frederick-vs-ja frederick-vs-ja May 24, 2025

Choose a reason for hiding this comment

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

This is only necessary for Clang. MSVC and EDG can forbid program-defined initializer_list specializations themselves.

Copy link
Author

Choose a reason for hiding this comment

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

Done

@localspook
Copy link
Author

I'd suggest adding to /tests/libcxx/expected_results.txt for now, so that the PR would otherwise pass CI

Done

@StephanTLavavej StephanTLavavej self-assigned this May 27, 2025
@StephanTLavavej StephanTLavavej added the enhancement Something can be improved label May 27, 2025
@localspook
Copy link
Author

I discovered that specializing unary_function and binary_function (before they were removed) is also forbidden, so I’ve marked those and added them to the PR message.

@localspook

This comment was marked as off-topic.

@StephanTLavavej
Copy link
Member

/azp run STL-CI

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Something can be improved
Projects
Status: Initial Review
Development

Successfully merging this pull request may close these issues.

Use [[clang::no_specializations]] to indicate templates which users are forbidden to specialize
4 participants