This is an archive of the discontinued LLVM Phabricator instance.

Differential D24333

[VLA] Handle VLA size expression in a full-expression context.
ClosedPublic

Authored by timshen on Sep 8 2016, 2:23 AM.

Details

Reviewers
rsmith
Commits
rGdef72aad0bc1: [VLA] Fix the test failure on msvc by specifying the triple.
rGb34d0ef2caa6: [VLA] Handle VLA size expression in a full-expression context.
rC295127: [VLA] Fix the test failure on msvc by specifying the triple.
rC295123: [VLA] Handle VLA size expression in a full-expression context.
rL295127: [VLA] Fix the test failure on msvc by specifying the triple.
rL295123: [VLA] Handle VLA size expression in a full-expression context.
Summary

Previously the cleanups (e.g. dtor calls) are inserted into the
outer scope (e.g. function body scope), instead of it's own scope. After
the fix, the cleanups are inserted right after getting the size value.

Diff Detail

Repository
rL LLVM

Event Timeline

timshen updated this revision to Diff 70665.Sep 8 2016, 2:23 AM
timshen retitled this revision from to [CleanupInfo] Use cleanupsHaveSideEffects instead of exprNeedsCleanups in assertions.
timshen updated this object.
timshen added a reviewer: rsmith.
timshen added a subscriber: cfe-commits.
timshen updated this revision to Diff 70667.Sep 8 2016, 2:28 AM

Update the test file name and remove unused code.

rsmith added inline comments.Jan 30 2017, 1:26 PM
clang/test/SemaCXX/pr30306.cpp
5 ↗(On Diff #70667)

Shouldn't we be (somehow) handling the cleanups from the array bound expression here -- either discarding them or attaching them to the array bound? Looks like TreeTransform::TransformVariableArrayType is missing a call to ActOnFinishFullExpr.

If we modify the test to:

struct A { A(int); ~A(); };
int f(const A &);
template<typename T> void g() { int a[f(3)]; }
int main() { g<int>(); }

... we need to register the ~A() destructor to actually be run at some point.

timshen updated this revision to Diff 86375.Jan 30 2017, 5:46 PM

Fix in the right way as rsmith pointed out.

timshen updated this revision to Diff 86377.Jan 30 2017, 5:58 PM

ActOnFinishFullExpr after exiting the expression evaluation context.

timshen retitled this revision from [CleanupInfo] Use cleanupsHaveSideEffects instead of exprNeedsCleanups in assertions to [VLA] Handle VLA size expression in a full-expression context..Jan 30 2017, 5:59 PM
timshen edited the summary of this revision. (Show Details)
rsmith accepted this revision.Feb 14 2017, 2:51 PM
This revision is now accepted and ready to land.Feb 14 2017, 2:51 PM
Closed by commit rL295123: [VLA] Handle VLA size expression in a full-expression context. (authored by timshen). · Explain WhyFeb 14 2017, 3:58 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathSize
cfe/
trunk/
lib/
Sema/
2 lines
11 lines
test/
Sema/
15 lines

Diff 88464

cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp

Show First 20 Lines • Show All 3,858 Lines • ▼ Show 20 Lines else {
} }
// Instantiate the function body. // Instantiate the function body.
StmtResult Body = SubstStmt(Pattern, TemplateArgs); StmtResult Body = SubstStmt(Pattern, TemplateArgs);
if (Body.isInvalid()) if (Body.isInvalid())
Function->setInvalidDecl(); Function->setInvalidDecl();
// FIXME: finishing the function body while in an expression evaluation
// context seems wrong. Investigate more.
ActOnFinishFunctionBody(Function, Body.get(), ActOnFinishFunctionBody(Function, Body.get(),
/*IsInstantiation=*/true); /*IsInstantiation=*/true);
PerformDependentDiagnostics(PatternDecl, TemplateArgs); PerformDependentDiagnostics(PatternDecl, TemplateArgs);
if (auto *Listener = getASTMutationListener()) if (auto *Listener = getASTMutationListener())
Listener->FunctionDefinitionInstantiated(Function); Listener->FunctionDefinitionInstantiated(Function);
▲ Show 20 Lines • Show All 1,278 Lines • Show Last 20 Lines

cfe/trunk/lib/Sema/TreeTransform.h

  • This file is larger than 256 KB, so syntax highlighting is disabled by default.
Show First 20 Lines • Show All 4,597 Lines • ▼ Show 20 Lines
QualType QualType
TreeTransform<Derived>::TransformVariableArrayType(TypeLocBuilder &TLB, TreeTransform<Derived>::TransformVariableArrayType(TypeLocBuilder &TLB,
VariableArrayTypeLoc TL) { VariableArrayTypeLoc TL) {
const VariableArrayType *T = TL.getTypePtr(); const VariableArrayType *T = TL.getTypePtr();
QualType ElementType = getDerived().TransformType(TLB, TL.getElementLoc()); QualType ElementType = getDerived().TransformType(TLB, TL.getElementLoc());
if (ElementType.isNull()) if (ElementType.isNull())
return QualType(); return QualType();
ExprResult SizeResult ExprResult SizeResult;
= getDerived().TransformExpr(T->getSizeExpr()); {
EnterExpressionEvaluationContext Context(SemaRef,
Sema::PotentiallyEvaluated);
SizeResult = getDerived().TransformExpr(T->getSizeExpr());
}
if (SizeResult.isInvalid())
return QualType();
SizeResult = SemaRef.ActOnFinishFullExpr(SizeResult.get());
if (SizeResult.isInvalid()) if (SizeResult.isInvalid())
return QualType(); return QualType();
Expr *Size = SizeResult.get(); Expr *Size = SizeResult.get();
QualType Result = TL.getType(); QualType Result = TL.getType();
if (getDerived().AlwaysRebuild() || if (getDerived().AlwaysRebuild() ||
ElementType != T->getElementType() || ElementType != T->getElementType() ||
▲ Show 20 Lines • Show All 7,810 Lines • Show Last 20 Lines

cfe/trunk/test/Sema/pr30306.cpp

// RUN: %clang_cc1 -x c++ -emit-llvm < %s | FileCheck %s
struct A { A(int); ~A(); };
int f(const A &);
// CHECK: call void @_ZN1AC1Ei
// CHECK-NEXT: call i32 @_Z1fRK1A
// CHECK-NEXT: call void @_ZN1AD1Ev
// CHECK: call void @_ZN1AC1Ei
// CHECK-NEXT: call i32 @_Z1fRK1A
// CHECK-NEXT: call void @_ZN1AD1Ev
template<typename T> void g() {
int a[f(3)];
int b[f(3)];
}
int main() { g<int>(); }