[Clang] Fix handling of immediate escalation for inherited constructors

[cor3ntin]

Fixes #112677

[llvmbot]

@llvm/pr-subscribers-clang

Author: cor3ntin (cor3ntin)

Changes

Fixes #112677

---

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

3 Files Affected:

• (modified) clang/docs/ReleaseNotes.rst (+2-1)
• (modified) clang/lib/AST/Decl.cpp (+13)
• (modified) clang/test/SemaCXX/cxx2b-consteval-propagate.cpp (+21)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index a65bd6f382901b..8846ee59a6e241 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -418,7 +418,7 @@ Improvements to Clang's diagnostics
 - The warning for an unsupported type for a named register variable is now phrased ``unsupported type for named register variable``,
   instead of ``bad type for named register variable``. This makes it clear that the type is not supported at all, rather than being
   suboptimal in some way the error fails to mention (#GH111550).
-  
+
 - Clang now emits a ``-Wdepredcated-literal-operator`` diagnostic, even if the
   name was a reserved name, which we improperly allowed to suppress the
   diagnostic.
@@ -537,6 +537,7 @@ Bug Fixes to C++ Support
   certain situations. (#GH47400), (#GH90896)
 - Fix erroneous templated array size calculation leading to crashes in generated code. (#GH41441)
 - During the lookup for a base class name, non-type names are ignored. (#GH16855)
+- Fix immediate escalation not propagating through inherited constructors.  (#GH112677)
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 8321cee0e0bc94..5314d01ed4fb6c 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -3284,6 +3284,13 @@ bool FunctionDecl::isImmediateEscalating() const {
   // consteval specifier,
   if (isDefaulted() && !isConsteval())
     return true;
+
+  if (auto *CD = dyn_cast<CXXConstructorDecl>(this);
+      CD && CD->isInheritingConstructor())
+    return CD->getInheritedConstructor()
+        .getConstructor()
+        ->isImmediateEscalating();
+
   // - a function that results from the instantiation of a templated entity
   // defined with the constexpr specifier.
   TemplatedKind TK = getTemplatedKind();
@@ -3304,6 +3311,12 @@ bool FunctionDecl::isImmediateFunction() const {
   if (isImmediateEscalating() && BodyContainsImmediateEscalatingExpressions())
     return true;
 
+  if (auto *CD = dyn_cast<CXXConstructorDecl>(this);
+      CD && CD->isInheritingConstructor())
+    return CD->getInheritedConstructor()
+        .getConstructor()
+        ->isImmediateFunction();
+
   if (const auto *MD = dyn_cast<CXXMethodDecl>(this);
       MD && MD->isLambdaStaticInvoker())
     return MD->getParent()->getLambdaCallOperator()->isImmediateFunction();
diff --git a/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp b/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp
index 378414f1361729..187a4958a2ee62 100644
--- a/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp
+++ b/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp
@@ -496,3 +496,24 @@ struct Y {
 template void g<Y>();
 
 }
+
+namespace GH112677 {
+
+class ConstEval {
+ public:
+  consteval ConstEval(int); // expected-note {{declared here}}
+};
+
+struct B {
+    ConstEval val;
+    template <class Anything = int> constexpr
+    B(int arg) : val(arg) {} // expected-note {{undefined constructor 'ConstEval'}}
+};
+struct C : B {
+    using B::B; // expected-note {{in call to 'B<int>(0)'}}
+};
+
+C c(0); // expected-note{{in implicit initialization for inherited constructor of 'C'}}
+// expected-error@-1 {{call to immediate function 'GH112677::C::B' is not a constant expression}}
+
+}

[efriedma-quic]

Consider the following testcase:

struct ConstEval {
  consteval ConstEval(int) {}
};
struct SimpleCtor { constexpr SimpleCtor(int) {}};
struct TemplateCtor {
  template <class Anything = int> constexpr
  TemplateCtor (int arg) {}
};
struct ConstEvalMember1 : SimpleCtor {
    int y = 10;
    ConstEval x = y;
    using SimpleCtor::SimpleCtor;
};
struct ConstEvalMember2 : TemplateCtor {
    int y = 10;
    ConstEval x = y;
    using TemplateCtor::TemplateCtor;
};
void f() {
  ConstEvalMember1 i(0);
  ConstEvalMember2 i2(0);
}

With this patch, the first one produces an error, the second doesn't. Which... seems dubious? Not sure. Inheriting the "immediate-escalating" property seems surprising to me.

[rkjnsn]

With this patch, the first one produces an error, the second doesn't. Which... seems dubious?

I am far from a standards expert, but I would expect both to compile? When calling an inherited constructor, "all other bases and members of Derived are initialized as if by the defaulted default constructor", and a "a defaulted special member function that is not declared with the consteval specifier" is an immediate-escalating function.

So, my thought is that there is a compiler-generated constructor that is immediate-escalating, which in turn calls the base class constructor, which may or may not be immediate or immediate-escalating. So, in the reduced test case from the original bug #112677, the FakeOptionalBase constructor is immediate-escalating, as is the compiler generated FakeOptional constructor. Since the ConstEval constructor is immediate, the FakeOptionalBase constructor escalates to being immediate, which in turn causes the FakeOptional constructor to escalate to being immediate.

In the testcase here, the TemplateCtor constructor is immedate-escalating and SimpleCtor constructor is not, but in neither case is the resulting base constructor immediate. However, I would expect the generated constructor for both ConstEvalMember1 and CostEvalMember2 to be immediate-escalating, and both to then escalate to being immediate due to the call to the immediate ConstEval constructor, as I believe default member initializers used to initialize a member subobject have been clarified to be part of the body of the constructor.

[rkjnsn]

I tried to poke around at this. I believe the simplest change to the PR to match my intuition would be to change

  if (const auto *CD = dyn_cast<CXXConstructorDecl>(this);
      CD && CD->isInheritingConstructor())
    return CD->getInheritedConstructor()
        .getConstructor()
        ->isImmediateEscalating();

to

  if (const auto *CD = dyn_cast<CXXConstructorDecl>(this);
      CD && CD->isInheritingConstructor())
    return true;

in FunctionDecl::isImmediateEscalating().

However, it might be additionally be desirable to have BodyContainsImmediateEscalatingExpressions() return true if the call to the inherited constructor in the base class is an immediately-escalating expression, similar to other field and base-class initialization. That would allow removing

  if (const auto *CD = dyn_cast<CXXConstructorDecl>(this);
      CD && CD->isInheritingConstructor())
    return CD->getInheritedConstructor()
        .getConstructor()
        ->isImmediateFunction();

from FunctionDecl::isImmediateFunction() and just relying on

  if (isImmediateEscalating() && BodyContainsImmediateEscalatingExpressions())
    return true;

to handle the immediate-inherited-constructor case.

However, I am not familiar enough with the code to know the proper way to make that happen.

[github-actions]

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

[cor3ntin]

@erichkeane mind re-reviewing that?

[erichkeane]

I have no concerns.