Skip to content

[Clang][AST] {CXXDefaultArgExpr, CXXDefaultInitExpr}::hasRewrittenInit return true iff the init expression was really rebuild #99748

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

[Clang][AST] {CXXDefaultArgExpr, CXXDefaultInitExpr}::hasRewrittenInit return true iff the init expression was really rebuild #99748

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions clang/include/clang/AST/ExprCXX.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1277,7 +1277,8 @@ class CXXDefaultArgExpr final
DeclContext *UsedContext;

CXXDefaultArgExpr(StmtClass SC, SourceLocation Loc, ParmVarDecl *Param,
Expr *RewrittenExpr, DeclContext *UsedContext)
DeclContext *UsedContext, Expr *RewrittenExpr,
bool HasRewrittenInit)
: Expr(SC,
Param->hasUnparsedDefaultArg()
? Param->getType().getNonReferenceType()
Expand All @@ -1286,7 +1287,7 @@ class CXXDefaultArgExpr final
Param->getDefaultArg()->getObjectKind()),
Param(Param), UsedContext(UsedContext) {
CXXDefaultArgExprBits.Loc = Loc;
CXXDefaultArgExprBits.HasRewrittenInit = RewrittenExpr != nullptr;
CXXDefaultArgExprBits.HasRewrittenInit = HasRewrittenInit;
if (RewrittenExpr)
*getTrailingObjects<Expr *>() = RewrittenExpr;
setDependence(computeDependence(this));
Expand All @@ -1304,8 +1305,8 @@ class CXXDefaultArgExpr final
// \p Param is the parameter whose default argument is used by this
// expression.
static CXXDefaultArgExpr *Create(const ASTContext &C, SourceLocation Loc,
ParmVarDecl *Param, Expr *RewrittenExpr,
DeclContext *UsedContext);
ParmVarDecl *Param, DeclContext *UsedContext,
Expr *InitExpr, bool HasRewrittenInit);
// Retrieve the parameter that the argument was created from.
const ParmVarDecl *getParam() const { return Param; }
ParmVarDecl *getParam() { return Param; }
Expand Down Expand Up @@ -1385,7 +1386,7 @@ class CXXDefaultInitExpr final

CXXDefaultInitExpr(const ASTContext &Ctx, SourceLocation Loc,
FieldDecl *Field, QualType Ty, DeclContext *UsedContext,
Expr *RewrittenInitExpr);
Expr *InitExpr, bool HasRewrittenInit);

CXXDefaultInitExpr(EmptyShell Empty, bool HasRewrittenInit)
: Expr(CXXDefaultInitExprClass, Empty) {
Expand All @@ -1399,7 +1400,7 @@ class CXXDefaultInitExpr final
/// by this expression.
static CXXDefaultInitExpr *Create(const ASTContext &Ctx, SourceLocation Loc,
FieldDecl *Field, DeclContext *UsedContext,
Expr *RewrittenInitExpr);
Expr *InitExpr, bool HasRewrittenInit);

bool hasRewrittenInit() const {
return CXXDefaultInitExprBits.HasRewrittenInit;
Expand Down
7 changes: 4 additions & 3 deletions clang/lib/AST/ASTImporter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8107,8 +8107,8 @@ ExpectedStmt ASTNodeImporter::VisitCXXDefaultArgExpr(CXXDefaultArgExpr *E) {
RewrittenInit = ExprOrErr.get();
}
return CXXDefaultArgExpr::Create(Importer.getToContext(), *ToUsedLocOrErr,
*ToParamOrErr, RewrittenInit,
*UsedContextOrErr);
*ToParamOrErr, *UsedContextOrErr,
RewrittenInit, E->hasRewrittenInit());
}

ExpectedStmt
Expand Down Expand Up @@ -8814,7 +8814,8 @@ ExpectedStmt ASTNodeImporter::VisitCXXDefaultInitExpr(CXXDefaultInitExpr *E) {
}

return CXXDefaultInitExpr::Create(Importer.getToContext(), *ToBeginLocOrErr,
ToField, *UsedContextOrErr, RewrittenInit);
ToField, *UsedContextOrErr, RewrittenInit,
E->hasRewrittenInit());
}

ExpectedStmt ASTNodeImporter::VisitCXXNamedCastExpr(CXXNamedCastExpr *E) {
Expand Down
33 changes: 16 additions & 17 deletions clang/lib/AST/ExprCXX.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1013,15 +1013,15 @@ CXXDefaultArgExpr *CXXDefaultArgExpr::CreateEmpty(const ASTContext &C,
return new (Mem) CXXDefaultArgExpr(EmptyShell(), HasRewrittenInit);
}

CXXDefaultArgExpr *CXXDefaultArgExpr::Create(const ASTContext &C,
SourceLocation Loc,
ParmVarDecl *Param,
Expr *RewrittenExpr,
DeclContext *UsedContext) {
CXXDefaultArgExpr *
CXXDefaultArgExpr::Create(const ASTContext &C, SourceLocation Loc,
ParmVarDecl *Param, DeclContext *UsedContext,
Expr *RewrittenExpr, bool HasRewrittenInit) {
size_t Size = totalSizeToAlloc<Expr *>(RewrittenExpr != nullptr);
Comment on lines +1019 to 1020
Copy link
Member

@Sirraide Sirraide Jul 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Maybe the naming here is just weird, but is there ever a case where RewrittenExpr != nullptr and HasRewrittenInit don’t evaluate to the same value? Because in that case, either HasRewrittenInit seems superfluous or RewrittenExpr is badly named, because a ‘non-rewritten RewrittenExpr’ sounds a bit nonsensical...

Copy link
Contributor

Choose a reason for hiding this comment

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

I have the same question!
In particular in the test change, I think the expression is rewritten.

auto *Mem = C.Allocate(Size, alignof(CXXDefaultArgExpr));
return new (Mem) CXXDefaultArgExpr(CXXDefaultArgExprClass, Loc, Param,
RewrittenExpr, UsedContext);
return new (Mem)
CXXDefaultArgExpr(CXXDefaultArgExprClass, Loc, Param, UsedContext,
RewrittenExpr, HasRewrittenInit);
}

Expr *CXXDefaultArgExpr::getExpr() {
Expand All @@ -1042,18 +1042,18 @@ Expr *CXXDefaultArgExpr::getAdjustedRewrittenExpr() {
CXXDefaultInitExpr::CXXDefaultInitExpr(const ASTContext &Ctx,
SourceLocation Loc, FieldDecl *Field,
QualType Ty, DeclContext *UsedContext,
Expr *RewrittenInitExpr)
Expr *InitExpr, bool HasRewrittenInit)
: Expr(CXXDefaultInitExprClass, Ty.getNonLValueExprType(Ctx),
Ty->isLValueReferenceType() ? VK_LValue
: Ty->isRValueReferenceType() ? VK_XValue
: VK_PRValue,
/*FIXME*/ OK_Ordinary),
Field(Field), UsedContext(UsedContext) {
CXXDefaultInitExprBits.Loc = Loc;
CXXDefaultInitExprBits.HasRewrittenInit = RewrittenInitExpr != nullptr;
CXXDefaultInitExprBits.HasRewrittenInit = HasRewrittenInit;

if (CXXDefaultInitExprBits.HasRewrittenInit)
*getTrailingObjects<Expr *>() = RewrittenInitExpr;
*getTrailingObjects<Expr *>() = InitExpr;

assert(Field->hasInClassInitializer());

Expand All @@ -1067,16 +1067,15 @@ CXXDefaultInitExpr *CXXDefaultInitExpr::CreateEmpty(const ASTContext &C,
return new (Mem) CXXDefaultInitExpr(EmptyShell(), HasRewrittenInit);
}

CXXDefaultInitExpr *CXXDefaultInitExpr::Create(const ASTContext &Ctx,
SourceLocation Loc,
FieldDecl *Field,
DeclContext *UsedContext,
Expr *RewrittenInitExpr) {
CXXDefaultInitExpr *
CXXDefaultInitExpr::Create(const ASTContext &Ctx, SourceLocation Loc,
FieldDecl *Field, DeclContext *UsedContext,
Expr *InitExpr, bool HasRewrittenInit) {

size_t Size = totalSizeToAlloc<Expr *>(RewrittenInitExpr != nullptr);
size_t Size = totalSizeToAlloc<Expr *>(InitExpr != nullptr);
auto *Mem = Ctx.Allocate(Size, alignof(CXXDefaultInitExpr));
return new (Mem) CXXDefaultInitExpr(Ctx, Loc, Field, Field->getType(),
UsedContext, RewrittenInitExpr);
UsedContext, InitExpr, HasRewrittenInit);
}

Expr *CXXDefaultInitExpr::getExpr() {
Expand Down
4 changes: 2 additions & 2 deletions clang/lib/AST/JSONNodeDumper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1579,11 +1579,11 @@ void JSONNodeDumper::VisitMaterializeTemporaryExpr(
}

void JSONNodeDumper::VisitCXXDefaultArgExpr(const CXXDefaultArgExpr *Node) {
attributeOnlyIfTrue("hasRewrittenInit", Node->hasRewrittenInit());
JOS.attribute("hasRewrittenInit", Node->hasRewrittenInit());
Copy link
Collaborator

Choose a reason for hiding this comment

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

So if IIUC this used to only write the attribute if the value was true, so we should have a test for both cases true and false to verify this is working as intended.

}

void JSONNodeDumper::VisitCXXDefaultInitExpr(const CXXDefaultInitExpr *Node) {
attributeOnlyIfTrue("hasRewrittenInit", Node->hasRewrittenInit());
JOS.attribute("hasRewrittenInit", Node->hasRewrittenInit());
}

void JSONNodeDumper::VisitCXXDependentScopeMemberExpr(
Expand Down
14 changes: 9 additions & 5 deletions clang/lib/Sema/SemaExpr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5426,6 +5426,7 @@ ExprResult Sema::BuildCXXDefaultArgExpr(SourceLocation CallLoc,

bool NestedDefaultChecking = isCheckingDefaultArgumentOrInitializer();
bool InLifetimeExtendingContext = isInLifetimeExtendingContext();
bool HasRewrittenInit = false;
std::optional<ExpressionEvaluationContextRecord::InitializationContext>
InitializationContext =
OutermostDeclarationWithDelayedImmediateInvocations();
Expand Down Expand Up @@ -5458,10 +5459,10 @@ ExprResult Sema::BuildCXXDefaultArgExpr(SourceLocation CallLoc,
ImmediateCallVisitor V(getASTContext());
if (!NestedDefaultChecking)
V.TraverseDecl(Param);

HasRewrittenInit = V.HasImmediateCalls || InLifetimeExtendingContext;
// Rewrite the call argument that was created from the corresponding
// parameter's default argument.
if (V.HasImmediateCalls || InLifetimeExtendingContext) {
if (HasRewrittenInit) {
if (V.HasImmediateCalls)
ExprEvalContexts.back().DelayedDefaultInitializationContext = {
CallLoc, Param, CurContext};
Expand Down Expand Up @@ -5490,7 +5491,8 @@ ExprResult Sema::BuildCXXDefaultArgExpr(SourceLocation CallLoc,
return ExprError();

return CXXDefaultArgExpr::Create(Context, InitializationContext->Loc, Param,
Init, InitializationContext->Context);
InitializationContext->Context, Init,
HasRewrittenInit);
}

ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) {
Expand Down Expand Up @@ -5548,7 +5550,9 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) {
ImmediateCallVisitor V(getASTContext());
if (!NestedDefaultChecking)
V.TraverseDecl(Field);
if (V.HasImmediateCalls) {

bool HasRewrittenInit = V.HasImmediateCalls;
if (HasRewrittenInit) {
ExprEvalContexts.back().DelayedDefaultInitializationContext = {Loc, Field,
CurContext};
ExprEvalContexts.back().IsCurrentlyCheckingDefaultArgumentOrInitializer =
Expand Down Expand Up @@ -5587,7 +5591,7 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) {

return CXXDefaultInitExpr::Create(Context, InitializationContext->Loc,
Field, InitializationContext->Context,
Init);
Init, HasRewrittenInit);
}

// DR1351:
Expand Down
9 changes: 5 additions & 4 deletions clang/lib/Sema/TreeTransform.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3369,9 +3369,10 @@ class TreeTransform {
/// require any semantic analysis. Subclasses may override this routine to
/// provide different behavior.
ExprResult RebuildCXXDefaultArgExpr(SourceLocation Loc, ParmVarDecl *Param,
Expr *RewrittenExpr) {
Expr *InitExpr, bool HasRewrittenExpr) {
return CXXDefaultArgExpr::Create(getSema().Context, Loc, Param,
RewrittenExpr, getSema().CurContext);
getSema().CurContext, InitExpr,
HasRewrittenExpr);
}

/// Build a new C++11 default-initialization expression.
Expand Down Expand Up @@ -13307,8 +13308,8 @@ TreeTransform<Derived>::TransformCXXDefaultArgExpr(CXXDefaultArgExpr *E) {
InitRes.get() == E->getRewrittenExpr())
return E;

return getDerived().RebuildCXXDefaultArgExpr(E->getUsedLocation(), Param,
InitRes.get());
return getDerived().RebuildCXXDefaultArgExpr(
E->getUsedLocation(), Param, InitRes.get(), E->hasRewrittenInit());
}

template<typename Derived>
Expand Down
2 changes: 1 addition & 1 deletion clang/test/AST/ast-dump-default-init-json.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -745,7 +745,7 @@ void test() {
// CHECK-NEXT: "qualType": "const A"
// CHECK-NEXT: },
// CHECK-NEXT: "valueCategory": "lvalue",
// CHECK-NEXT: "hasRewrittenInit": true,
// CHECK-NEXT: "hasRewrittenInit": false,
// CHECK-NEXT: "inner": [
// CHECK-NEXT: {
// CHECK-NEXT: "id": "0x{{.*}}",
Expand Down
2 changes: 1 addition & 1 deletion clang/test/AST/ast-dump-default-init.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ struct B {
void test() {
B b{};
}
// CHECK: -CXXDefaultInitExpr 0x{{[^ ]*}} <{{.*}}> 'const A' lvalue has rewritten init
// CHECK: -CXXDefaultInitExpr 0x{{[^ ]*}} <{{.*}}> 'const A' lvalue
// CHECK-NEXT: `-ExprWithCleanups 0x{{[^ ]*}} <{{.*}}> 'const A' lvalue
// CHECK-NEXT: `-MaterializeTemporaryExpr 0x{{[^ ]*}} <{{.*}}> 'const A' lvalue extended by Field 0x{{[^ ]*}} 'a' 'const A &'
// CHECK-NEXT: `-ImplicitCastExpr 0x{{[^ ]*}} <{{.*}}> 'const A' <NoOp>
Expand Down
Loading