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

error when define a move assignment = default (if using type alias) #56456

Closed
twinstar6980 opened this issue Jul 9, 2022 · 8 comments
Closed
Assignees
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema"

Comments

@twinstar6980
Copy link

twinstar6980 commented Jul 9, 2022

For some reason, I used a type alias when defining the move constructor, which compiles successfully on MSVC and GCC, but Clang doesn't allow it
I tested other special member functions, copy constructor, move constructor, copy assignment function can all use the = default declaration explicitly in the case of using type alias, only move assignment function does not allow this, but it can use = delete explicitly, so is this a bug?
here is my test code

// constant reference
template <typename T>
using RC = T const &;

// non-constant reference
template <typename T>
using RV = T &;

// r-value reference
template <typename T>
using RM = T&&;

struct A {

	// default constructor
	A () = default;
	// copy constructor
	A (RC<A>) = default;
	// move constructor
	A (RM<A>) = default;
	
	// copy assignment
	auto operator = (RC<A>) -> RV<A> = default;
	// move assignment
	auto operator = (RM<A>) -> RV<A> = default; // error: only special member functions may be defaulted
	//auto operator = (RM<A>) -> RV<A> = delete;  // OK
	
};

int main () {
	return 0;
}
@shafik
Copy link
Collaborator

shafik commented Jul 9, 2022

Looks like this is failing in Sema::SetDeclDefaulted, in particular it looks like Sema::getDefaultedFunctionKind is not doing the right thing because MD->isMoveAssignmentOperator() comes back false.

Looking at CXXMethodDecl::isCopyAssignmentOperator() is does the following:

  QualType ParamType = getParamDecl(0)->getType();
  if (const auto *Ref = ParamType->getAs<LValueReferenceType>())
    ParamType = Ref->getPointeeType();

while CXXMethodDecl::isMoveAssignmentOperator() does:

 QualType ParamType = getParamDecl(0)->getType();
  if (!isa<RValueReferenceType>(ParamType))
    return false;
  ParamType = ParamType->getPointeeType();

If I modify isMoveAssignmentOperator to work like isCopyAssignmentOperator it seems to fix the issue but I need to make sure this passes check-clang.

@shafik shafik added the clang:frontend Language frontend issues, e.g. anything involving "Sema" label Jul 9, 2022
@llvmbot
Copy link
Collaborator

llvmbot commented Jul 9, 2022

@llvm/issue-subscribers-clang-frontend

@shafik shafik removed the new issue label Jul 9, 2022
@shafik shafik self-assigned this Jul 9, 2022
@shafik
Copy link
Collaborator

shafik commented Jul 10, 2022

That did not pass but modifying CXXMethodDecl::isMoveAssignmentOperator() this way does pass 'check-clang` cleanly:

 QualType ParamType = getParamDecl(0)->getType();
  if (!ParamType->isRValueReferenceType())
    return false;
  ParamType = ParamType->getPointeeType();

@twinstar6980
Copy link
Author

That did not pass but modifying CXXMethodDecl::isMoveAssignmentOperator() this way does pass 'check-clang` cleanly:

 QualType ParamType = getParamDecl(0)->getType();
  if (!ParamType->isRValueReferenceType())
    return false;
  ParamType = ParamType->getPointeeType();

ok, as you said it is working now, thanks for your reply

@shafik
Copy link
Collaborator

shafik commented Jul 10, 2022

Apologies, I was not clear. I have not put in a fix yet. I am just explaining the steps I have taken so far in looking into the problem. I still need to write some tests to cover this problem and post a PR for a fix. Once I have the PR I will post it to the bug.

@shafik shafik reopened this Jul 10, 2022
@shafik
Copy link
Collaborator

shafik commented Jul 12, 2022

@twinstar6980
Copy link
Author

PR: https://reviews.llvm.org/D129591

ok❤️

@EugeneZelenko
Copy link
Contributor

@TwinKleS-C: patch is not merged into main yet.

@EugeneZelenko EugeneZelenko reopened this Jul 14, 2022
@shafik shafik closed this as completed in 80dec2e Jul 14, 2022
mem-frob pushed a commit to draperlaboratory/hope-llvm-project that referenced this issue Oct 7, 2022
…ugh type sugar

AcceptedPublic

Currently CXXMethodDecl::isMoveAssignmentOperator() does not look though type
sugar and so if the parameter is a type alias it will not be able to detect
that the method is a move assignment operator. This PR fixes that and adds a set
of tests that covers that we correctly detect special member functions when
defaulting or deleting them.

This fixes: llvm/llvm-project#56456

Differential Revision: https://reviews.llvm.org/D129591
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema"
Projects
None yet
Development

No branches or pull requests

4 participants