Allow immediate invocation of constructors

[wchilders]

Summary

Hi all,

This is an initial patch submission falling out of Lock3's reflection and metaprogramming work. This is a rather small patch to allow immediate invocation of constructors.

We have additional larger patches for consteval which will likely be a bit trickier, so I wanted to start here to help familiarize myself with the process.

For context, we have forthcoming patches that:

• Provide improvements which allow the cached values of ConstantExpr to be used by both the constant evaluator and code gen
• Update the application of evaluation contexts, using the stronger guarantees of manifest constant evaluation to apply the ConstantEvaluated evaluation context in more places

These are both larger, and while we depend on them in observable ways for our work, these are not currently observable changes from the perspective of the test suite.

[rsmith]

clang/lib/Sema/SemaInit.cpp

6466

It looks like the other callers to BuildCXXConstructExpr are also missing this handling. Can we put the call to CheckForImmediateInvocation in BuildCXXConstructExpr to handle all those cases at once, or do we need to defer it until after the other stuff below?

[wchilders]

clang/lib/Sema/SemaInit.cpp

6466

Those changes shouldn't have impact here.

I choose to apply this here as when looking at other uses of CheckForImmediateInvocation, MaybeBindToTemporary was performed on the expr, prior to the expr being passed to CheckForImmediateInvocation.

Upon changing this over, all tests (upstream and ours) still pass, so if you don't see any issues, I'm fine with moving this into BuildCXXConstructExpr. Additionally, upon closer inspection, there is low coupling between MaybeBindToTemporary and CXXConstructExpr, where as there is significant coupling for ObjC and CallExprs. Unless something I've said here gives you some doubts, I'm fine with moving this to BuildCXXConstructExpr, there's nothing that immediately jumps out to me as a problem.

[wchilders]

clang/lib/Sema/SemaInit.cpp

6466

Upon changing this over, all tests (upstream and ours) still pass, so if you don't see any issues, I'm fine with moving this into BuildCXXConstructExpr. (Whoops, rearranged some sentences last second heh)

[Tyker]

I have already a patch aiming to do the same thing. D74007

Provide improvements which allow the cached values of ConstantExpr to be used by both the constant evaluator and code gen

this is definitely something we should do.

Update the application of evaluation contexts, using the stronger guarantees of manifest constant evaluation to apply the ConstantEvaluated evaluation context in more places

the tracking of evaluation context isn't as good as it could/should.

[wchilders]

In D76396#1930773, @Tyker wrote:

I have already a patch aiming to do the same thing. D74007

Oof, okay, sounds good. I was not aware of this patch, it looks to be much further along :)

Provide improvements which allow the cached values of ConstantExpr to be used by both the constant evaluator and code gen

this is definitely something we should do.

Update the application of evaluation contexts, using the stronger guarantees of manifest constant evaluation to apply the ConstantEvaluated evaluation context in more places

the tracking of evaluation context isn't as good as it could/should.

With this being closed out in favor of Tyker's patch, I'll post a patch for reusing cached values of ConstantExpr shortly.

[rsmith]

In D76396#1930773, @Tyker wrote:

I have already a patch aiming to do the same thing. D74007

Thanks. Sorry I dropped the ball on that one. =/

clang/lib/Sema/SemaInit.cpp

if (isExplicitTemporary(Entity, Kind, NumArgs)) {
     if (IsListInitialization)
       ParenOrBraceRange = SourceRange(LBraceLoc, RBraceLoc);
     else if (Kind.getKind() == InitializationKind::IK_Direct)
       ParenOrBraceRange = Kind.getParenOrBraceRange();
 
     // If the entity allows NRVO, mark the construction as elidable
     // unconditionally.
     if (Entity.allowsNRVO())
       CurInit = S.BuildCXXConstructExpr(Loc, Step.Type,
                                         Step.Function.FoundDecl,
                                         Constructor, /*Elidable=*/true,
                                         ConstructorArgs,
                                         HadMultipleCandidates,
                                         IsListInitialization,
                                         IsStdInitListInitialization,
                                         ConstructorInitRequiresZeroInit,
                                         ConstructKind,
PerformConstructorInitialization(Sema &S,
 
   if (const ArrayType *AT = S.Context.getAsArrayType(Entity.getType()))
     if (checkDestructorReference(S.Context.getBaseElementType(AT), Loc, S))
       return ExprError();
 
   if (shouldBindAsTemporary(Entity))
     CurInit = S.MaybeBindToTemporary(CurInit.get());
 
-  return CurInit;
+  return S.CheckForImmediateInvocation(CurInit, Constructor);
 }
 
 namespace {
 enum LifetimeKind {
   /// The lifetime of a temporary bound to this entity ends at the end of the
   /// full-expression, and that's (probably) fine.
   LK_FullExpression,
 

clang/test/SemaCXX/cxx2a-consteval.cpp

struct X {
       virtual bool operator==(const Y&) const; // expected-note {{here}}
     };
     struct Y : X {
       consteval std::strong_ordering operator<=>(const Y&) const = default;
       // expected-error@-1 {{consteval function 'operator==' cannot override a non-consteval function}}
     };
   }
 }
+
+namespace constructor {
+  struct A {
+    A* ptr = nullptr;
+    consteval A() : ptr(this) { }
+  };
+
+  A a = A(); // expected-error {{is not a constant expression}}
+  // expected-note@-1 {{is not a constant expression}} expected-note@-1 {{temporary created here}}
+}
+