Skip to content

Commit 3a84303

Browse files
zygoloidzmodem
authored andcommitted
PR45083: Mark statement expressions as being dependent if they appear in
dependent contexts. We previously assumed they were neither value- nor instantiation-dependent under any circumstances, which would lead to crashes and other misbehavior. (cherry picked from commit bdad0a1)
1 parent bca373f commit 3a84303

File tree

8 files changed

+51
-21
lines changed

8 files changed

+51
-21
lines changed

clang/include/clang/AST/Expr.h

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3955,14 +3955,14 @@ class StmtExpr : public Expr {
39553955
Stmt *SubStmt;
39563956
SourceLocation LParenLoc, RParenLoc;
39573957
public:
3958-
// FIXME: Does type-dependence need to be computed differently?
3959-
// FIXME: Do we need to compute instantiation instantiation-dependence for
3960-
// statements? (ugh!)
39613958
StmtExpr(CompoundStmt *substmt, QualType T,
3962-
SourceLocation lp, SourceLocation rp) :
3959+
SourceLocation lp, SourceLocation rp, bool InDependentContext) :
3960+
// Note: we treat a statement-expression in a dependent context as always
3961+
// being value- and instantiation-dependent. This matches the behavior of
3962+
// lambda-expressions and GCC.
39633963
Expr(StmtExprClass, T, VK_RValue, OK_Ordinary,
3964-
T->isDependentType(), false, false, false),
3965-
SubStmt(substmt), LParenLoc(lp), RParenLoc(rp) { }
3964+
T->isDependentType(), InDependentContext, InDependentContext, false),
3965+
SubStmt(substmt), LParenLoc(lp), RParenLoc(rp) {}
39663966

39673967
/// Build an empty statement expression.
39683968
explicit StmtExpr(EmptyShell Empty) : Expr(StmtExprClass, Empty) { }

clang/include/clang/Sema/Sema.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4923,7 +4923,7 @@ class Sema final {
49234923
LabelDecl *TheDecl);
49244924

49254925
void ActOnStartStmtExpr();
4926-
ExprResult ActOnStmtExpr(SourceLocation LPLoc, Stmt *SubStmt,
4926+
ExprResult ActOnStmtExpr(Scope *S, SourceLocation LPLoc, Stmt *SubStmt,
49274927
SourceLocation RPLoc); // "({..})"
49284928
// Handle the final expression in a statement expression.
49294929
ExprResult ActOnStmtExprResult(ExprResult E);

clang/lib/AST/ASTImporter.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6724,8 +6724,9 @@ ExpectedStmt ASTNodeImporter::VisitStmtExpr(StmtExpr *E) {
67246724
SourceLocation ToLParenLoc, ToRParenLoc;
67256725
std::tie(ToSubStmt, ToType, ToLParenLoc, ToRParenLoc) = *Imp;
67266726

6727-
return new (Importer.getToContext()) StmtExpr(
6728-
ToSubStmt, ToType, ToLParenLoc, ToRParenLoc);
6727+
return new (Importer.getToContext())
6728+
StmtExpr(ToSubStmt, ToType, ToLParenLoc, ToRParenLoc,
6729+
E->isInstantiationDependent());
67296730
}
67306731

67316732
ExpectedStmt ASTNodeImporter::VisitUnaryOperator(UnaryOperator *E) {

clang/lib/Parse/ParseExpr.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2655,7 +2655,8 @@ Parser::ParseParenExpression(ParenParseOption &ExprType, bool stopIfCastExpr,
26552655

26562656
// If the substmt parsed correctly, build the AST node.
26572657
if (!Stmt.isInvalid()) {
2658-
Result = Actions.ActOnStmtExpr(OpenLoc, Stmt.get(), Tok.getLocation());
2658+
Result = Actions.ActOnStmtExpr(getCurScope(), OpenLoc, Stmt.get(),
2659+
Tok.getLocation());
26592660
} else {
26602661
Actions.ActOnStmtExprError();
26612662
}

clang/lib/Sema/SemaExpr.cpp

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13908,9 +13908,8 @@ void Sema::ActOnStmtExprError() {
1390813908
PopExpressionEvaluationContext();
1390913909
}
1391013910

13911-
ExprResult
13912-
Sema::ActOnStmtExpr(SourceLocation LPLoc, Stmt *SubStmt,
13913-
SourceLocation RPLoc) { // "({..})"
13911+
ExprResult Sema::ActOnStmtExpr(Scope *S, SourceLocation LPLoc, Stmt *SubStmt,
13912+
SourceLocation RPLoc) { // "({..})"
1391413913
assert(SubStmt && isa<CompoundStmt>(SubStmt) && "Invalid action invocation!");
1391513914
CompoundStmt *Compound = cast<CompoundStmt>(SubStmt);
1391613915

@@ -13939,9 +13938,18 @@ Sema::ActOnStmtExpr(SourceLocation LPLoc, Stmt *SubStmt,
1393913938
}
1394013939
}
1394113940

13941+
bool IsDependentContext = false;
13942+
if (S)
13943+
IsDependentContext = S->getTemplateParamParent() != nullptr;
13944+
else
13945+
// FIXME: This is not correct when substituting inside a templated
13946+
// context that isn't a DeclContext (such as a variable template).
13947+
IsDependentContext = CurContext->isDependentContext();
13948+
1394213949
// FIXME: Check that expression type is complete/non-abstract; statement
1394313950
// expressions are not lvalues.
13944-
Expr *ResStmtExpr = new (Context) StmtExpr(Compound, Ty, LPLoc, RPLoc);
13951+
Expr *ResStmtExpr =
13952+
new (Context) StmtExpr(Compound, Ty, LPLoc, RPLoc, IsDependentContext);
1394513953
if (StmtExprMayBindToTemp)
1394613954
return MaybeBindToTemporary(ResStmtExpr);
1394713955
return ResStmtExpr;

clang/lib/Sema/SemaExprCXX.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6800,8 +6800,9 @@ Stmt *Sema::MaybeCreateStmtWithCleanups(Stmt *SubStmt) {
68006800
// a new AsmStmtWithTemporaries.
68016801
CompoundStmt *CompStmt = CompoundStmt::Create(
68026802
Context, SubStmt, SourceLocation(), SourceLocation());
6803-
Expr *E = new (Context) StmtExpr(CompStmt, Context.VoidTy, SourceLocation(),
6804-
SourceLocation());
6803+
Expr *E = new (Context)
6804+
StmtExpr(CompStmt, Context.VoidTy, SourceLocation(), SourceLocation(),
6805+
CurContext->isDependentContext());
68056806
return MaybeCreateExprWithCleanups(E);
68066807
}
68076808

clang/lib/Sema/TreeTransform.h

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2526,10 +2526,9 @@ class TreeTransform {
25262526
///
25272527
/// By default, performs semantic analysis to build the new expression.
25282528
/// Subclasses may override this routine to provide different behavior.
2529-
ExprResult RebuildStmtExpr(SourceLocation LParenLoc,
2530-
Stmt *SubStmt,
2531-
SourceLocation RParenLoc) {
2532-
return getSema().ActOnStmtExpr(LParenLoc, SubStmt, RParenLoc);
2529+
ExprResult RebuildStmtExpr(SourceLocation LParenLoc, Stmt *SubStmt,
2530+
SourceLocation RParenLoc) {
2531+
return getSema().ActOnStmtExpr(nullptr, LParenLoc, SubStmt, RParenLoc);
25332532
}
25342533

25352534
/// Build a new __builtin_choose_expr expression.
@@ -11801,6 +11800,8 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
1180111800
NewTrailingRequiresClause = getDerived().TransformExpr(TRC);
1180211801

1180311802
// Create the local class that will describe the lambda.
11803+
// FIXME: KnownDependent below is wrong when substituting inside a templated
11804+
// context that isn't a DeclContext (such as a variable template).
1180411805
CXXRecordDecl *OldClass = E->getLambdaClass();
1180511806
CXXRecordDecl *Class
1180611807
= getSema().createLambdaClosureType(E->getIntroducerRange(),

clang/test/SemaTemplate/dependent-expr.cpp

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
// RUN: %clang_cc1 -fsyntax-only -verify %s
2-
// expected-no-diagnostics
32

43
// PR5908
54
template <typename Iterator>
@@ -108,3 +107,22 @@ namespace PR18152 {
108107
};
109108
template struct A<0>;
110109
}
110+
111+
template<typename T> void stmt_expr_1() {
112+
static_assert( ({ false; }), "" );
113+
}
114+
void stmt_expr_2() {
115+
static_assert( ({ false; }), "" ); // expected-error {{failed}}
116+
}
117+
118+
namespace PR45083 {
119+
struct A { bool x; };
120+
121+
template<typename> struct B : A {
122+
void f() {
123+
const int n = ({ if (x) {} 0; });
124+
}
125+
};
126+
127+
template void B<int>::f();
128+
}

0 commit comments

Comments
 (0)