cert-dcl58-cpp flags legitimate specializations (false positive)

[LegalizeAdulthood]

Bugzilla Link	46109
Version	unspecified
OS	Windows NT
CC	@EugeneZelenko,@johnmcfarlane,@Xazax-hun

Extended Description

You're allowed to inject specializations of std::hash into namespace std for your own types.

https://en.cppreference.com/w/cpp/utility/hash

[johnmcfarlane]

You're also allowed to specialize std::numbers math constants but currently cannot: https://godbolt.org/z/4dK1zo

Here's the clause: https://eel.is/c++draft/numbers#math.constants-2
More details here: wg21.link/p0631

[llvmbot]

You're allowed to inject specializations of std::hash into namespace std for
your own types.

https://en.cppreference.com/w/cpp/utility/hash
That specialization doesn't raise a warning for this check
https://godbolt.org/z/sW7Exd

You're also allowed to specialize std::numbers math constants but
currently cannot: https://godbolt.org/z/4dK1zo

Here's the clause: https://eel.is/c++draft/numbers#math.constants-2
More details here: wg21.link/p0631

Probably should extend the check to work with constexpr variable templates

[LegalizeAdulthood]

You're allowed to inject specializations of std::hash into namespace std for
your own types.

https://en.cppreference.com/w/cpp/utility/hash
That specialization doesn't raise a warning for this check
https://godbolt.org/z/sW7Exd

It could be that the tool has been updated and fixed this compared to the version I had on my machine. I'll see if I can reproduce with more recent clang-tidy.

[LegalizeAdulthood]

The issue no longer reproduces in clang-tidy 14, so I'm closing.

[Febbe]

Please reopen, it still fails (15.0-rc2) for templated objects:

template <typename T>
struct MyStruct{};

namespace std {
template <typename T>
struct hash<MyStruct<T>> {
    auto operator()(const MyStruct<T>& k) const noexcept -> std::size_t {
        return 1U;
    }
};
}    // namespace std

[captaincrutches]

I can confirm @Febbe's case, I'm also getting that warning on a partial specialization as of 15.0.0.

It seems to have started in 15 actually, I wasn't getting it in 14.

There were changes made to this warning in 15, seemingly with the goal of allowing full specializations, which do work - but partial ones are throwing out the false positive now.

[jbytheway]

Just giving this a bump because I've run into the same issue of false positives on partial specializations on version 16.0.4.

[N-Dekker]

Just ran into the same issue (I think), trying to add structured binding support to a class template, itk::ImageRegion<unsigned int>, by specializing std::tuple_size and std::tuple_element for itk::ImageRegion: InsightSoftwareConsortium/ITK#4367

Affected code:

namespace std
{
template <unsigned int VImageDimension>
struct tuple_size<itk::ImageRegion<VImageDimension>> : std::integral_constant<size_t, 2>
{};

template <size_t VTupleIndex, unsigned int VImageDimension>
struct tuple_element<VTupleIndex, itk::ImageRegion<VImageDimension>>
  : conditional<VTupleIndex == 0, itk::Index<VImageDimension>, itk::Size<VImageDimension>>
{};

}

Clang Tidy output from Visual Studio 2019, Clang Power Tools 2023.9.0, LLVM 17.0.1:

warning: modification of 'std' namespace can result in undefined behavior [cert-dcl58-cpp]
     | struct tuple_size<itk::ImageRegion<VImageDimension>> : std::integral_constant<size_t, 2>
     |        ^

...

warning: modification of 'std' namespace can result in undefined behavior [cert-dcl58-cpp]
     | struct tuple_element<VTupleIndex, itk::ImageRegion<VImageDimension>>
     |        ^

Hope it can still be fixed. Otherwise, do you have a suggestion how to work around this warning, in user code?

[Febbe]

Hope it can still be fixed. Otherwise, do you have a suggestion how to work around this warning, in user code?

It's a bug in the checkers code. So if someone has time, he could always write a fix and open a PR.
There is no work around / other way to do things like that. The code is completely correct. I would just disable the warning either line by line, or in the .clang-tidy config.

[PiotrZSL]

Currently //NOLINT(cert-dcl58-cpp) or //NOLINTNEXTLINE(cert-dcl58-cpp) as an workaround