Skip to content

Commit 66addf8

Browse files
committed
Revert "Fix regression in bdad0a1: force rebuilding of StmtExpr nodes in", "PR45083: Mark statement expressions as being dependent if they appear in"
This reverts commit f545ede. This reverts commit bdad0a1. This crashes clang. I'll follow up with reproduction instructions.
1 parent 6e9c10f commit 66addf8

File tree

8 files changed

+25
-66
lines changed

8 files changed

+25
-66
lines changed

clang/include/clang/AST/Expr.h

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

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

clang/include/clang/Sema/Sema.h

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4964,10 +4964,8 @@ class Sema final {
49644964
LabelDecl *TheDecl);
49654965

49664966
void ActOnStartStmtExpr();
4967-
ExprResult ActOnStmtExpr(Scope *S, SourceLocation LPLoc, Stmt *SubStmt,
4968-
SourceLocation RPLoc);
4969-
ExprResult BuildStmtExpr(SourceLocation LPLoc, Stmt *SubStmt,
4970-
SourceLocation RPLoc, bool IsDependent);
4967+
ExprResult ActOnStmtExpr(SourceLocation LPLoc, Stmt *SubStmt,
4968+
SourceLocation RPLoc); // "({..})"
49714969
// Handle the final expression in a statement expression.
49724970
ExprResult ActOnStmtExprResult(ExprResult E);
49734971
void ActOnStmtExprError();

clang/lib/AST/ASTImporter.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6631,9 +6631,8 @@ ExpectedStmt ASTNodeImporter::VisitStmtExpr(StmtExpr *E) {
66316631
if (Err)
66326632
return std::move(Err);
66336633

6634-
return new (Importer.getToContext())
6635-
StmtExpr(ToSubStmt, ToType, ToLParenLoc, ToRParenLoc,
6636-
E->isInstantiationDependent());
6634+
return new (Importer.getToContext()) StmtExpr(
6635+
ToSubStmt, ToType, ToLParenLoc, ToRParenLoc);
66376636
}
66386637

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

clang/lib/Parse/ParseExpr.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2655,8 +2655,7 @@ 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(getCurScope(), OpenLoc, Stmt.get(),
2659-
Tok.getLocation());
2658+
Result = Actions.ActOnStmtExpr(OpenLoc, Stmt.get(), Tok.getLocation());
26602659
} else {
26612660
Actions.ActOnStmtExprError();
26622661
}

clang/lib/Sema/SemaExpr.cpp

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13911,14 +13911,9 @@ void Sema::ActOnStmtExprError() {
1391113911
PopExpressionEvaluationContext();
1391213912
}
1391313913

13914-
ExprResult Sema::ActOnStmtExpr(Scope *S, SourceLocation LPLoc, Stmt *SubStmt,
13915-
SourceLocation RPLoc) {
13916-
return BuildStmtExpr(LPLoc, SubStmt, RPLoc,
13917-
S->getTemplateParamParent() != nullptr);
13918-
}
13919-
13920-
ExprResult Sema::BuildStmtExpr(SourceLocation LPLoc, Stmt *SubStmt,
13921-
SourceLocation RPLoc, bool IsDependent) {
13914+
ExprResult
13915+
Sema::ActOnStmtExpr(SourceLocation LPLoc, Stmt *SubStmt,
13916+
SourceLocation RPLoc) { // "({..})"
1392213917
assert(SubStmt && isa<CompoundStmt>(SubStmt) && "Invalid action invocation!");
1392313918
CompoundStmt *Compound = cast<CompoundStmt>(SubStmt);
1392413919

@@ -13949,8 +13944,7 @@ ExprResult Sema::BuildStmtExpr(SourceLocation LPLoc, Stmt *SubStmt,
1394913944

1395013945
// FIXME: Check that expression type is complete/non-abstract; statement
1395113946
// expressions are not lvalues.
13952-
Expr *ResStmtExpr =
13953-
new (Context) StmtExpr(Compound, Ty, LPLoc, RPLoc, IsDependent);
13947+
Expr *ResStmtExpr = new (Context) StmtExpr(Compound, Ty, LPLoc, RPLoc);
1395413948
if (StmtExprMayBindToTemp)
1395513949
return MaybeBindToTemporary(ResStmtExpr);
1395613950
return ResStmtExpr;

clang/lib/Sema/SemaExprCXX.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6946,9 +6946,8 @@ Stmt *Sema::MaybeCreateStmtWithCleanups(Stmt *SubStmt) {
69466946
// a new AsmStmtWithTemporaries.
69476947
CompoundStmt *CompStmt = CompoundStmt::Create(
69486948
Context, SubStmt, SourceLocation(), SourceLocation());
6949-
Expr *E = new (Context)
6950-
StmtExpr(CompStmt, Context.VoidTy, SourceLocation(), SourceLocation(),
6951-
CurContext->isDependentContext());
6949+
Expr *E = new (Context) StmtExpr(CompStmt, Context.VoidTy, SourceLocation(),
6950+
SourceLocation());
69526951
return MaybeCreateExprWithCleanups(E);
69536952
}
69546953

clang/lib/Sema/TreeTransform.h

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2549,9 +2549,10 @@ class TreeTransform {
25492549
///
25502550
/// By default, performs semantic analysis to build the new expression.
25512551
/// Subclasses may override this routine to provide different behavior.
2552-
ExprResult RebuildStmtExpr(SourceLocation LParenLoc, Stmt *SubStmt,
2553-
SourceLocation RParenLoc, bool IsDependent) {
2554-
return getSema().BuildStmtExpr(LParenLoc, SubStmt, RParenLoc, IsDependent);
2552+
ExprResult RebuildStmtExpr(SourceLocation LParenLoc,
2553+
Stmt *SubStmt,
2554+
SourceLocation RParenLoc) {
2555+
return getSema().ActOnStmtExpr(LParenLoc, SubStmt, RParenLoc);
25552556
}
25562557

25572558
/// Build a new __builtin_choose_expr expression.
@@ -10432,20 +10433,16 @@ TreeTransform<Derived>::TransformStmtExpr(StmtExpr *E) {
1043210433
return ExprError();
1043310434
}
1043410435

10435-
// FIXME: This is not correct when substituting inside a templated
10436-
// context that isn't a DeclContext (such as a variable template).
10437-
bool IsDependent = getSema().CurContext->isDependentContext();
10438-
1043910436
if (!getDerived().AlwaysRebuild() &&
10440-
IsDependent == E->isInstantiationDependent() &&
1044110437
SubStmt.get() == E->getSubStmt()) {
1044210438
// Calling this an 'error' is unintuitive, but it does the right thing.
1044310439
SemaRef.ActOnStmtExprError();
1044410440
return SemaRef.MaybeBindToTemporary(E);
1044510441
}
1044610442

10447-
return getDerived().RebuildStmtExpr(E->getLParenLoc(), SubStmt.get(),
10448-
E->getRParenLoc(), IsDependent);
10443+
return getDerived().RebuildStmtExpr(E->getLParenLoc(),
10444+
SubStmt.get(),
10445+
E->getRParenLoc());
1044910446
}
1045010447

1045110448
template<typename Derived>
@@ -11891,8 +11888,6 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
1189111888
NewTrailingRequiresClause = getDerived().TransformExpr(TRC);
1189211889

1189311890
// Create the local class that will describe the lambda.
11894-
// FIXME: KnownDependent below is wrong when substituting inside a templated
11895-
// context that isn't a DeclContext (such as a variable template).
1189611891
CXXRecordDecl *OldClass = E->getLambdaClass();
1189711892
CXXRecordDecl *Class
1189811893
= getSema().createLambdaClosureType(E->getIntroducerRange(),

clang/test/SemaTemplate/dependent-expr.cpp

Lines changed: 1 addition & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
// RUN: %clang_cc1 -fsyntax-only -verify %s
2+
// expected-no-diagnostics
23

34
// PR5908
45
template <typename Iterator>
@@ -107,29 +108,3 @@ namespace PR18152 {
107108
};
108109
template struct A<0>;
109110
}
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-
129-
// Make sure we properly rebuild statement expression AST nodes even if the
130-
// only thing that changes is the "is dependent" flag.
131-
template<typename> void f() {
132-
decltype(({})) x; // expected-error {{incomplete type}}
133-
}
134-
template void f<int>(); // expected-note {{instantiation of}}
135-
}

0 commit comments

Comments
 (0)