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] Fix CTAD not work for function-type and array-type arguments. #78159

Merged
merged 3 commits into from
Jan 16, 2024

Conversation

hokein
Copy link
Collaborator

@hokein hokein commented Jan 15, 2024

Fixes #51710.

When transforming a constructor into a corresponding deduction guide, the decayed types (function/array type) were not handled properly which made clang fail to compile valid code. The patch teaches clang handle these decayed type in the transformation.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jan 15, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 15, 2024

@llvm/pr-subscribers-clang

Author: Haojian Wu (hokein)

Changes

Fixes #51710


Full diff: https://github.com/llvm/llvm-project/pull/78159.diff

3 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+2)
  • (modified) clang/lib/Sema/SemaTemplate.cpp (+7-1)
  • (added) clang/test/SemaCXX/ctad-decay.cpp (+26)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 4aba054e252af24..82302bae6d6dc28 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -726,6 +726,8 @@ Bug Fixes in This Version
 - Fix an issue where clang cannot find conversion function with template
   parameter when instantiation of template class.
   Fixes (`#77583 <https://github.com/llvm/llvm-project/issues/77583>`_)
+- Fix an issue where CTAD fails for function-type/array-type arguments.
+  Fixes (`#51710 <https://github.com/llvm/llvm-project/issues/51710>`_)
 
 Bug Fixes to Compiler Builtins
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 5fcc39ec7005228..3052b01f7f12ac3 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -2587,12 +2587,18 @@ struct ConvertConstructorToDeductionGuideTransform {
                           : ParamTy->isRValueReferenceType() ? VK_XValue
                                                              : VK_PRValue);
     }
+    // Handle arrays and functions decay.
+    auto NewType = NewDI->getType();
+    if (NewType->isArrayType())
+      NewType = SemaRef.Context.getArrayDecayedType(NewType);
+    else if (NewType->isFunctionType())
+      NewType = SemaRef.Context.getPointerType(NewType);
 
     ParmVarDecl *NewParam = ParmVarDecl::Create(SemaRef.Context, DC,
                                                 OldParam->getInnerLocStart(),
                                                 OldParam->getLocation(),
                                                 OldParam->getIdentifier(),
-                                                NewDI->getType(),
+                                                NewType,
                                                 NewDI,
                                                 OldParam->getStorageClass(),
                                                 NewDefArg.get());
diff --git a/clang/test/SemaCXX/ctad-decay.cpp b/clang/test/SemaCXX/ctad-decay.cpp
new file mode 100644
index 000000000000000..97063e8fb75659f
--- /dev/null
+++ b/clang/test/SemaCXX/ctad-decay.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wno-unused-value -std=c++20 %s
+// expected-no-diagnostics
+
+namespace GH51710 {
+
+template<typename T>
+struct A{
+  A(T f());
+  A(int f(), T);
+
+  A(T array[10]);
+  A(int array[10], T);
+};
+
+int foo();
+
+void bar() {
+  A test1(foo);
+  A test2(foo, 1);
+
+  int array[10];
+  A test3(array);
+  A test4(array, 1);
+}
+
+} // namespace GH51710

Copy link

github-actions bot commented Jan 15, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@@ -0,0 +1,26 @@
// RUN: %clang_cc1 -fsyntax-only -verify -Wno-unused-value -std=c++20 %s
// expected-no-diagnostics
Copy link
Contributor

Choose a reason for hiding this comment

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

can you move that test to clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure, done.

A(int f(), T) {}

A(T array[10]) {}
A(int array[10], T) {}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add tests for incomplete array (in another struct ie

template<typename T>
struct B {
   B(T array[]) {}
   B(int array[], T) {}
};

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

Comment on lines 2591 to 2595
auto NewType = NewDI->getType();
if (NewType->isArrayType())
NewType = SemaRef.Context.getArrayDecayedType(NewType);
else if (NewType->isFunctionType())
NewType = SemaRef.Context.getPointerType(NewType);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can just use SemaRef.getDecayedType(NewDI->getType()), it covers both case

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point, thanks. I was not aware of this API.

Copy link
Contributor

@cor3ntin cor3ntin left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

@shafik
Copy link
Collaborator

shafik commented Jan 16, 2024

Can you please provide a more detailed summary, since these are usually what goes in the git log. It should describe the cause and the approach of the fix. It is also helpful for reviewers as well.

Copy link
Collaborator

@shafik shafik left a comment

Choose a reason for hiding this comment

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

LGTM after more detailed summary provided.

@hokein
Copy link
Collaborator Author

hokein commented Jan 16, 2024

Can you please provide a more detailed summary, since these are usually what goes in the git log. It should describe the cause and the approach of the fix. It is also helpful for reviewers as well.

Done, added details in the description.

@hokein hokein merged commit f725bb9 into llvm:main Jan 16, 2024
5 checks passed
@hokein hokein deleted the ctad-decay branch January 16, 2024 08:54
@shafik
Copy link
Collaborator

shafik commented Jan 16, 2024

Can you please provide a more detailed summary, since these are usually what goes in the git log. It should describe the cause and the approach of the fix. It is also helpful for reviewers as well.

Done, added details in the description.

Thank you! Much appreciated :-)

justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
…llvm#78159)

Fixes llvm#51710.

When transforming a constructor into a corresponding deduction guide,
the decayed types (function/array type) were not handled properly which
made clang fail to compile valid code. The patch teaches clang handle
these decayed type in the transformation.
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" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Class template argument deduction fails in case of function type argument
4 participants