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

[clang-tidy] Using an aliased name of a class for its copy/move/assignment constructors/operators produces a false positive cppcoreguidelines-special-member-functions warning. #116055

Open
crud89 opened this issue Nov 13, 2024 · 4 comments
Labels
clang-tidy false-positive Warning fires when it should not

Comments

@crud89
Copy link

crud89 commented Nov 13, 2024

The following class produces a false positive Wcppcoreguidelines-special-member-functions with LLVM 19.1.3:

template <typename T, unsigned ROWS, unsigned COLS> requires (ROWS >= 2 && COLS >= 2)
class Mat {
public:
	static constexpr size_t mat_rows = ROWS;
	static constexpr size_t mat_cols = COLS;

	using scalar_type = T;
	using mat_type = Mat<scalar_type, mat_rows, mat_cols>;

protected:
	using array_type = std::array<scalar_type, mat_rows* mat_cols>;

private:
	array_type m_elements = { };

public:
	constexpr Mat() noexcept = default;

	constexpr Mat(const mat_type& _other) noexcept {
		std::ranges::copy(_other.m_elements, std::begin(m_elements));
	}

	constexpr Mat(mat_type&& _other) noexcept :
		m_elements{ std::move(_other.m_elements) } {
	}

	virtual ~Mat() noexcept = default;

	constexpr auto& operator=(const mat_type& _other) noexcept {
		std::ranges::copy(_other.m_elements, std::begin(m_elements));
		return *this;
	}

	constexpr auto& operator=(mat_type&& _other) noexcept {
		m_elements = std::move(_other.m_elements);
		return *this;
	}
};

This happens due to the alias using mat_type = Mat<scalar_type, mat_rows, mat_cols>;. If I replace the alias mat_type in the constructors and assignment operators with the full class name, the warning goes away.

@EugeneZelenko EugeneZelenko added the false-positive Warning fires when it should not label Nov 13, 2024
@chrchr-github
Copy link

Reduced:

template<int i>
struct S {
    static constexpr int n = i;
    using U = S<n>;
    S(const U&) = default;
    S(U&&) = default;
    U& operator=(const U& u) = default;
    U& operator=(U&& u) = default;
    virtual ~S() = default;
};

https://godbolt.org/z/3EP3Td9jT
There is an additional false positive hicpp-explicit-conversions.

@nicovank
Copy link
Contributor

Note that this code does not compile on GCC.

<source>:5:19: error: 'S<i>::S(const U&)' cannot be defaulted [-Wtemplate-body]
    5 |     S(const U&) = default;
      |                   ^~~~~~~
<source>:6:14: error: 'S<i>::S(U&&)' cannot be defaulted [-Wtemplate-body]
    6 |     S(U&&) = default;
      |              ^~~~~~~
<source>:7:32: error: 'S<i>::U& S<i>::operator=(const U&)' cannot be defaulted [-Wtemplate-body]
    7 |     U& operator=(const U& u) = default;
      |                                ^~~~~~~
<source>:8:27: error: 'S<i>::U& S<i>::operator=(U&&)' cannot be defaulted [-Wtemplate-body]
    8 |     U& operator=(U&& u) = default;
      |                           ^~~~~~~

@chrchr-github
Copy link

MSVC rejects also. But the false positive still occurs when implementations are provided:
https://godbolt.org/z/Thq7bar6d

@nicovank
Copy link
Contributor

Correct, but I think something fishy is going here where the check might technically be right in that no "move constructor" as defined in standard is provided. For example, in the example with implementations provided, you can add another overload S(const S&) {}, which will compile just fine until you use it. In practice everything should work fine because the correct overload is selected anyway.

I'm curious why Clang lets you = default it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang-tidy false-positive Warning fires when it should not
Projects
None yet
Development

No branches or pull requests

4 participants