Skip to content

Commit

Permalink
[Clang] Fix consteval propagation for aggregates and defaulted constr…
Browse files Browse the repository at this point in the history
…uctors

This patch does a few things:

* Fix aggregate initialization.
  When an aggregate has an initializer that is immediate-escalating,
  the context in which it is used automatically becomes an immediate function.
  The wording does that by rpretending an aggregate initialization is itself
  an invocation which is not really how clang works, so my previous attempt
  was... wrong.

* Fix initialization of defaulted constructors with immediate escalating
  default member initializers.
  The wording was silent about that case and I did not handled it fully
  https://cplusplus.github.io/CWG/issues/2760.html

* Fix diagnostics
  In some cases clang would produce additional and unhelpful
  diagnostics by listing the invalid references to consteval
  function that appear in immediate escalating functions

Fixes #63742

Reviewed By: aaron.ballman, #clang-language-wg, Fznamznon

Differential Revision: https://reviews.llvm.org/D155175
  • Loading branch information
cor3ntin committed Jul 24, 2023
1 parent 331a54d commit 8698262
Show file tree
Hide file tree
Showing 11 changed files with 299 additions and 66 deletions.
6 changes: 5 additions & 1 deletion clang/include/clang/AST/RecursiveASTVisitor.h
Original file line number Diff line number Diff line change
Expand Up @@ -2727,7 +2727,11 @@ DEF_TRAVERSE_STMT(CXXDefaultArgExpr, {
TRY_TO(TraverseStmt(S->getExpr()));
})

DEF_TRAVERSE_STMT(CXXDefaultInitExpr, {})
DEF_TRAVERSE_STMT(CXXDefaultInitExpr, {
if (getDerived().shouldVisitImplicitCode())
TRY_TO(TraverseStmt(S->getExpr()));
})

DEF_TRAVERSE_STMT(CXXDeleteExpr, {})
DEF_TRAVERSE_STMT(ExprWithCleanups, {})
DEF_TRAVERSE_STMT(CXXInheritedCtorInitExpr, {})
Expand Down
3 changes: 2 additions & 1 deletion clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -2685,7 +2685,8 @@ def err_immediate_function_used_before_definition : Error<
"immediate function %0 used before it is defined">;

def note_immediate_function_reason : Note<
"%0 is an immediate function because its body "
"%0 is an immediate %select{function|constructor}5 because "
"%select{its body|the%select{| default}7 initializer of %8}6 "
"%select{evaluates the address of %select{an immediate|a consteval}2 "
"function %1|contains a call to %select{an immediate|a consteval}2 "
"%select{function|constructor}4 %1 and that call is not a constant "
Expand Down
15 changes: 10 additions & 5 deletions clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -1078,10 +1078,13 @@ class Sema final {
public:
SynthesizedFunctionScope(Sema &S, DeclContext *DC)
: S(S), SavedContext(S, DC) {
auto *FD = dyn_cast<FunctionDecl>(DC);
S.PushFunctionScope();
S.PushExpressionEvaluationContext(
Sema::ExpressionEvaluationContext::PotentiallyEvaluated);
if (auto *FD = dyn_cast<FunctionDecl>(DC)) {
(FD && FD->isConsteval())
? ExpressionEvaluationContext::ImmediateFunctionContext
: ExpressionEvaluationContext::PotentiallyEvaluated);
if (FD) {
FD->setWillHaveBody(true);
S.ExprEvalContexts.back().InImmediateFunctionContext =
FD->isImmediateFunction();
Expand All @@ -1106,8 +1109,10 @@ class Sema final {
~SynthesizedFunctionScope() {
if (PushedCodeSynthesisContext)
S.popCodeSynthesisContext();
if (auto *FD = dyn_cast<FunctionDecl>(S.CurContext))
if (auto *FD = dyn_cast<FunctionDecl>(S.CurContext)) {
FD->setWillHaveBody(false);
S.CheckImmediateEscalatingFunctionDefinition(FD, S.getCurFunction());
}
S.PopExpressionEvaluationContext();
S.PopFunctionScopeInfo();
}
Expand Down Expand Up @@ -6578,11 +6583,11 @@ class Sema final {
ExprResult CheckForImmediateInvocation(ExprResult E, FunctionDecl *Decl);

bool CheckImmediateEscalatingFunctionDefinition(
FunctionDecl *FD, bool HasImmediateEscalatingExpression);
FunctionDecl *FD, const sema::FunctionScopeInfo *FSI);

void MarkExpressionAsImmediateEscalating(Expr *E);

void DiagnoseImmediateEscalatingReason(const clang::FunctionDecl *FD);
void DiagnoseImmediateEscalatingReason(FunctionDecl *FD);

bool CompleteConstructorCall(CXXConstructorDecl *Constructor,
QualType DeclInitType, MultiExprArg ArgsPtr,
Expand Down
4 changes: 4 additions & 0 deletions clang/lib/CodeGen/CGExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5319,6 +5319,10 @@ RValue CodeGenFunction::EmitCall(QualType CalleeType, const CGCallee &OrigCallee
const Decl *TargetDecl =
OrigCallee.getAbstractInfo().getCalleeDecl().getDecl();

assert((!isa_and_present<FunctionDecl>(TargetDecl) ||
!cast<FunctionDecl>(TargetDecl)->isImmediateFunction()) &&
"trying to emit a call to an immediate function");

CalleeType = getContext().getCanonicalType(CalleeType);

auto PointeeType = cast<PointerType>(CalleeType)->getPointeeType();
Expand Down
11 changes: 4 additions & 7 deletions clang/lib/CodeGen/CodeGenModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4438,13 +4438,10 @@ llvm::Constant *CodeGenModule::GetOrCreateLLVMFunction(
/// GetAddrOfFunction - Return the address of the given function. If Ty is
/// non-null, then this function will use the specified type if it has to
/// create it (this occurs when we see a definition of the function).
llvm::Constant *CodeGenModule::GetAddrOfFunction(GlobalDecl GD,
llvm::Type *Ty,
bool ForVTable,
bool DontDefer,
ForDefinition_t IsForDefinition) {
assert(!cast<FunctionDecl>(GD.getDecl())->isImmediateFunction() &&
"an immediate function should never be emitted");
llvm::Constant *
CodeGenModule::GetAddrOfFunction(GlobalDecl GD, llvm::Type *Ty, bool ForVTable,
bool DontDefer,
ForDefinition_t IsForDefinition) {
// If there was no specific requested type, just convert it now.
if (!Ty) {
const auto *FD = cast<FunctionDecl>(GD.getDecl());
Expand Down
3 changes: 1 addition & 2 deletions clang/lib/Sema/SemaDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15575,8 +15575,7 @@ Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt *Body,
if (FD) {
FD->setBody(Body);
FD->setWillHaveBody(false);
CheckImmediateEscalatingFunctionDefinition(
FD, FSI->FoundImmediateEscalatingExpression);
CheckImmediateEscalatingFunctionDefinition(FD, FSI);

if (getLangOpts().CPlusPlus14) {
if (!FD->isInvalidDecl() && Body && !FD->isDependentContext() &&
Expand Down
83 changes: 55 additions & 28 deletions clang/lib/Sema/SemaDeclCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/Support/SaveAndRestore.h"
#include <map>
#include <optional>
#include <set>
Expand Down Expand Up @@ -2438,13 +2439,12 @@ static bool CheckConstexprFunctionBody(Sema &SemaRef, const FunctionDecl *Dcl,
}

bool Sema::CheckImmediateEscalatingFunctionDefinition(
FunctionDecl *FD, bool HasImmediateEscalatingExpression) {
if (!FD->hasBody() || !getLangOpts().CPlusPlus20 ||
!FD->isImmediateEscalating())
FunctionDecl *FD, const sema::FunctionScopeInfo *FSI) {
if (!getLangOpts().CPlusPlus20 || !FD->isImmediateEscalating())
return true;
FD->setBodyContainsImmediateEscalatingExpressions(
HasImmediateEscalatingExpression);
if (HasImmediateEscalatingExpression) {
FSI->FoundImmediateEscalatingExpression);
if (FSI->FoundImmediateEscalatingExpression) {
auto it = UndefinedButUsed.find(FD->getCanonicalDecl());
if (it != UndefinedButUsed.end()) {
Diag(it->second, diag::err_immediate_function_used_before_definition)
Expand All @@ -2458,65 +2458,92 @@ bool Sema::CheckImmediateEscalatingFunctionDefinition(
return true;
}

void Sema::DiagnoseImmediateEscalatingReason(const FunctionDecl *FD) {
void Sema::DiagnoseImmediateEscalatingReason(FunctionDecl *FD) {
assert(FD->isImmediateEscalating() && !FD->isConsteval() &&
"expected an immediate function");
assert(FD->hasBody() && "expected the function to have a body");
struct ImmediateEscalatingExpressionsVisitor
: public RecursiveASTVisitor<ImmediateEscalatingExpressionsVisitor> {

using Base = RecursiveASTVisitor<ImmediateEscalatingExpressionsVisitor>;
Sema &SemaRef;
const FunctionDecl *FD;
ImmediateEscalatingExpressionsVisitor(Sema &SemaRef, const FunctionDecl *FD)
: SemaRef(SemaRef), FD(FD) {}

const FunctionDecl *ImmediateFn;
bool ImmediateFnIsConstructor;
CXXConstructorDecl *CurrentConstructor = nullptr;
CXXCtorInitializer *CurrentInit = nullptr;

ImmediateEscalatingExpressionsVisitor(Sema &SemaRef, FunctionDecl *FD)
: SemaRef(SemaRef), ImmediateFn(FD),
ImmediateFnIsConstructor(isa<CXXConstructorDecl>(FD)) {}

bool shouldVisitImplicitCode() const { return true; }
bool shouldVisitLambdaBody() const { return false; }

void Diag(const Expr *E, const FunctionDecl *Fn, bool IsCall) {
SourceLocation Loc = E->getBeginLoc();
SourceRange Range = E->getSourceRange();
if (CurrentConstructor && CurrentInit) {
Loc = CurrentConstructor->getLocation();
Range = CurrentInit->isWritten() ? CurrentInit->getSourceRange()
: SourceRange();
}
SemaRef.Diag(Loc, diag::note_immediate_function_reason)
<< ImmediateFn << Fn << Fn->isConsteval() << IsCall
<< isa<CXXConstructorDecl>(Fn) << ImmediateFnIsConstructor
<< (CurrentInit != nullptr)
<< (CurrentInit && !CurrentInit->isWritten())
<< (CurrentInit ? CurrentInit->getAnyMember() : nullptr) << Range;
}
bool TraverseCallExpr(CallExpr *E) {
if (const auto *DR =
dyn_cast<DeclRefExpr>(E->getCallee()->IgnoreImplicit());
DR && DR->isImmediateEscalating()) {
SemaRef.Diag(E->getBeginLoc(), diag::note_immediate_function_reason)
<< FD << E->getDirectCallee() << E->getDirectCallee()->isConsteval()
<< /*Call*/ 1 << /*Function*/ 0 << E->getSourceRange();
}
for (auto A : E->arguments()) {
getDerived().TraverseStmt(A);
Diag(E, E->getDirectCallee(), /*IsCall=*/true);
return false;
}

for (Expr *A : E->arguments())
if (!getDerived().TraverseStmt(A))
return false;

return true;
}

bool VisitDeclRefExpr(DeclRefExpr *E) {
if (const auto *ReferencedFn = dyn_cast<FunctionDecl>(E->getDecl());
ReferencedFn && E->isImmediateEscalating()) {
SemaRef.Diag(E->getBeginLoc(), diag::note_immediate_function_reason)
<< FD << ReferencedFn << ReferencedFn->isConsteval()
<< /*Address*/ 0 << /*Function*/ 0 << E->getSourceRange();
Diag(E, ReferencedFn, /*IsCall=*/false);
return false;
}

return true;
}

bool VisitCXXConstructExpr(CXXConstructExpr *E) {
CXXConstructorDecl *D = E->getConstructor();
if (E->isImmediateEscalating()) {
SemaRef.Diag(E->getBeginLoc(), diag::note_immediate_function_reason)
<< FD << D << D->isConsteval() << /*Call*/ 1 << /*Constructor*/ 1
<< E->getSourceRange();
Diag(E, D, /*IsCall=*/true);
return false;
}
return true;
}

// These nodes can never contain an immediate escalating expression,
// we can skip them to avoid unecessary work.
bool TraverseDecl(Decl *D) {
if (isa<FunctionDecl, RecordDecl>(D))
return true;
return Base::TraverseDecl(D);
bool TraverseConstructorInitializer(CXXCtorInitializer *Init) {
llvm::SaveAndRestore RAII(CurrentInit, Init);
return Base::TraverseConstructorInitializer(Init);
}

bool TraverseCXXConstructorDecl(CXXConstructorDecl *Ctr) {
llvm::SaveAndRestore RAII(CurrentConstructor, Ctr);
return Base::TraverseCXXConstructorDecl(Ctr);
}

bool TraverseType(QualType T) { return true; }
bool VisitBlockExpr(BlockExpr *T) { return true; }

} Visitor(*this, FD);
Visitor.TraverseStmt(FD->getBody());
Visitor.TraverseDecl(FD);
}

/// Get the class that is directly named by the current context. This is the
Expand Down
37 changes: 22 additions & 15 deletions clang/lib/Sema/SemaExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6084,17 +6084,11 @@ struct ImmediateCallVisitor : public RecursiveASTVisitor<ImmediateCallVisitor> {
ImmediateCallVisitor(const ASTContext &Ctx) : Context(Ctx) {}

bool HasImmediateCalls = false;
bool IsImmediateInvocation = false;

bool shouldVisitImplicitCode() const { return true; }

bool VisitCallExpr(CallExpr *E) {
if (const FunctionDecl *FD = E->getDirectCallee()) {
if (const FunctionDecl *FD = E->getDirectCallee())
HasImmediateCalls |= FD->isImmediateFunction();
if (FD->isConsteval() && !E->isCXX11ConstantExpr(Context))
IsImmediateInvocation = true;
}

return RecursiveASTVisitor<ImmediateCallVisitor>::VisitStmt(E);
}

Expand Down Expand Up @@ -6224,6 +6218,8 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) {
if (Field->isInvalidDecl())
return ExprError();

CXXThisScopeRAII This(*this, Field->getParent(), Qualifiers());

auto *ParentRD = cast<CXXRecordDecl>(Field->getParent());

std::optional<ExpressionEvaluationContextRecord::InitializationContext>
Expand Down Expand Up @@ -6271,13 +6267,6 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) {
if (!NestedDefaultChecking)
V.TraverseDecl(Field);
if (V.HasImmediateCalls) {
// C++23 [expr.const]/p15
// An aggregate initialization is an immediate invocation
// if it evaluates a default member initializer that has a subexpression
// that is an immediate-escalating expression.
ExprEvalContexts.back().InImmediateFunctionContext |=
V.IsImmediateInvocation;

ExprEvalContexts.back().DelayedDefaultInitializationContext = {Loc, Field,
CurContext};
ExprEvalContexts.back().IsCurrentlyCheckingDefaultArgumentOrInitializer =
Expand Down Expand Up @@ -18445,7 +18434,12 @@ HandleImmediateInvocations(Sema &SemaRef,
if (!CE.getInt())
EvaluateAndDiagnoseImmediateInvocation(SemaRef, CE);
for (auto *DR : Rec.ReferenceToConsteval) {
const auto *FD = cast<FunctionDecl>(DR->getDecl());
// If the expression is immediate escalating, it is not an error;
// The outer context itself becomes immediate and further errors,
// if any, will be handled by DiagnoseImmediateEscalatingReason.
if (DR->isImmediateEscalating())
continue;
auto *FD = cast<FunctionDecl>(DR->getDecl());
const NamedDecl *ND = FD;
if (const auto *MD = dyn_cast<CXXMethodDecl>(ND);
MD && (MD->isLambdaStaticInvoker() || isLambdaCallOperator(MD)))
Expand All @@ -18470,8 +18464,15 @@ HandleImmediateInvocations(Sema &SemaRef,
SemaRef.Diag(DR->getBeginLoc(), diag::err_invalid_consteval_take_address)
<< ND << isa<CXXRecordDecl>(ND) << FD->isConsteval();
SemaRef.Diag(ND->getLocation(), diag::note_declared_at);
if (auto Context =
SemaRef.InnermostDeclarationWithDelayedImmediateInvocations()) {
SemaRef.Diag(Context->Loc, diag::note_invalid_consteval_initializer)
<< Context->Decl;
SemaRef.Diag(Context->Decl->getBeginLoc(), diag::note_declared_at);
}
if (FD->isImmediateEscalating() && !FD->isConsteval())
SemaRef.DiagnoseImmediateEscalatingReason(FD);

} else {
SemaRef.MarkExpressionAsImmediateEscalating(DR);
}
Expand Down Expand Up @@ -18920,6 +18921,12 @@ void Sema::MarkFunctionReferenced(SourceLocation Loc, FunctionDecl *Func,
// or of another default member initializer (ie a PotentiallyEvaluatedIfUsed
// context), its initializers may not be referenced yet.
if (CXXConstructorDecl *Constructor = dyn_cast<CXXConstructorDecl>(Func)) {
EnterExpressionEvaluationContext EvalContext(
*this,
Constructor->isImmediateFunction()
? ExpressionEvaluationContext::ImmediateFunctionContext
: ExpressionEvaluationContext::PotentiallyEvaluated,
Constructor);
for (CXXCtorInitializer *Init : Constructor->inits()) {
if (Init->isInClassMemberInitializer())
runWithSufficientStackSpace(Init->getSourceLocation(), [&]() {
Expand Down
21 changes: 21 additions & 0 deletions clang/test/CodeGenCXX/cxx2c-consteval-propagate.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// RUN: %clang_cc1 -emit-llvm %s -std=c++20 -triple x86_64-unknown-linux-gnu -o - | FileCheck %s

namespace GH63742 {

void side_effect();
consteval int f(int x) {
if (!__builtin_is_constant_evaluated()) side_effect();
return x;
}
struct SS {
int x = f(42);
SS();
};
SS::SS(){}

}

// CHECK-LABEL: @_ZN7GH637422SSC2Ev
// CHECK-NOT: call
// CHECK: store i32 42, ptr {{.*}}
// CHECK: ret void
10 changes: 6 additions & 4 deletions clang/test/SemaCXX/cxx2a-consteval-default-params.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,19 +41,21 @@ struct UnusedInitWithLambda {
}();
};

consteval int ub(int n) { // expected-note {{declared here}}
consteval int ub(int n) {
return 0/n;
}

struct InitWithLambda {
int b = [](int error = undefined()) { // expected-error {{cannot take address of consteval function 'undefined' outside of an immediate invocation}}
struct InitWithLambda { // expected-note {{'InitWithLambda' is an immediate constructor because the default initializer of 'b' contains a call to a consteval function 'undefined' and that call is not a constant expression}}
int b = [](int error = undefined()) { // expected-note {{undefined function 'undefined' cannot be used in a constant expression}}
return error;
}();
int c = [](int error = sizeof(undefined()) + ub(0)) { // expected-error {{cannot take address of consteval function 'ub' outside of an immediate invocation}}
int c = [](int error = sizeof(undefined()) + ub(0)) {

return error;
}();
} i;
// expected-error@-1 {{call to immediate function 'InitWithLambda::InitWithLambda' is not a constant expression}} \
expected-note@-1 {{in call to 'InitWithLambda()'}}

namespace ShouldNotCrash {
template<typename T>
Expand Down

0 comments on commit 8698262

Please sign in to comment.