Skip to content

Conversation

@zyn0217
Copy link
Contributor

@zyn0217 zyn0217 commented May 23, 2025

We check the accessibility of constructors when initializing a default argument whose type is not an aggregate.

Make sure the check is performed within the correct DeclContext. Otherwise, it will be delayed until the end of the declaration, at which point the context is mismatched.

Fixes #62444
Fixes #83608

We check the accessibility of constructors when initializing a default
argument whose type is not an aggregate.

Make sure the check is performed within the correct DeclContext.
Otherwise, it will be delayed until the end of the declaration,
at which point the context is mismatched.
@zyn0217 zyn0217 requested review from cor3ntin and erichkeane May 23, 2025 07:37
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels May 23, 2025
@llvmbot
Copy link
Member

llvmbot commented May 23, 2025

@llvm/pr-subscribers-clang

Author: Younan Zhang (zyn0217)

Changes

We check the accessibility of constructors when initializing a default argument whose type is not an aggregate.

Make sure the check is performed within the correct DeclContext. Otherwise, it will be delayed until the end of the declaration, at which point the context is mismatched.

Fixes #83608


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

3 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+1)
  • (modified) clang/lib/Sema/SemaTemplateInstantiate.cpp (+7-7)
  • (modified) clang/test/SemaTemplate/default-arguments.cpp (+22)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 573ae97bff710..45c96af809505 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -767,6 +767,7 @@ Bug Fixes to C++ Support
 - Clang could incorrectly instantiate functions in discarded contexts (#GH140449)
 - Fix instantiation of default-initialized variable template specialization. (#GH140632) (#GH140622)
 - Clang modules now allow a module and its user to differ on TrivialAutoVarInit*
+- Fixed a C++20 access checking bug when initializing non-aggregates in default arguments (#GH83608)
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index d028eea4f8f3e..cd20dfb4fe093 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -3340,13 +3340,13 @@ bool Sema::SubstDefaultArgument(
   }
 
   ExprResult Result;
+  // C++ [dcl.fct.default]p5:
+  //   The names in the [default argument] expression are bound, and
+  //   the semantic constraints are checked, at the point where the
+  //   default argument expression appears.
+  ContextRAII SavedContext(*this, FD);
   {
-    // C++ [dcl.fct.default]p5:
-    //   The names in the [default argument] expression are bound, and
-    //   the semantic constraints are checked, at the point where the
-    //   default argument expression appears.
-    ContextRAII SavedContext(*this, FD);
-    std::unique_ptr<LocalInstantiationScope> LIS;
+    std::optional<LocalInstantiationScope> LIS;
 
     if (ForCallExpr) {
       // When instantiating a default argument due to use in a call expression,
@@ -3354,7 +3354,7 @@ bool Sema::SubstDefaultArgument(
       // required to satisfy references from the default argument. For example:
       //   template<typename T> void f(T a, int = decltype(a)());
       //   void g() { f(0); }
-      LIS = std::make_unique<LocalInstantiationScope>(*this);
+      LIS.emplace(*this);
       FunctionDecl *PatternFD = FD->getTemplateInstantiationPattern(
           /*ForDefinition*/ false);
       if (addInstantiatedParametersToScope(FD, PatternFD, *LIS, TemplateArgs))
diff --git a/clang/test/SemaTemplate/default-arguments.cpp b/clang/test/SemaTemplate/default-arguments.cpp
index 5ea34c0254ec1..e9d781bfe5712 100644
--- a/clang/test/SemaTemplate/default-arguments.cpp
+++ b/clang/test/SemaTemplate/default-arguments.cpp
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++98 %s
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 %s
 template<typename T, int N = 2> struct X; // expected-note{{template is declared here}}
 
 X<int, 1> *x1;
@@ -282,4 +283,25 @@ static_assert(S<short *>().SizeOfT<char>() == sizeof(short *), "");
 
 } // namespace GH68490
 
+namespace GH83608 {
+
+class single;
+
+class check_constructible {
+  // This makes it a non-aggregate in C++20+.
+  check_constructible() = default;
+
+  friend class single;
+};
+
+struct single {
+  template <class T> single(T u, check_constructible = {}) {}
+};
+
+// We perform access checking when substituting into the default argument.
+// Make sure it runs within class single.
+single x(0);
+
+}
+
 #endif

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.

Thanks

@cor3ntin
Copy link
Contributor

Does this fixes #62444 ?

@zyn0217
Copy link
Contributor Author

zyn0217 commented May 23, 2025

Does this fixes #62444 ?

Yes 😎

@zyn0217 zyn0217 merged commit d4f0f43 into llvm:main May 23, 2025
10 of 12 checks passed
@zyn0217 zyn0217 deleted the 83608 branch May 23, 2025 12:11
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

3 participants