[clang] 66addf8 - Revert "Fix regression in bdad0a1: force rebuilding of StmtExpr nodes in", "PR45083: Mark statement expressions as being dependent if they appear in"

Benjamin Kramer via cfe-commits cfe-commits at lists.llvm.org
Thu Mar 5 05:14:58 PST 2020


Author: Benjamin Kramer
Date: 2020-03-05T14:14:31+01:00
New Revision: 66addf8e803618758457e4d578c5084e322ca448

URL: https://github.com/llvm/llvm-project/commit/66addf8e803618758457e4d578c5084e322ca448
DIFF: https://github.com/llvm/llvm-project/commit/66addf8e803618758457e4d578c5084e322ca448.diff

LOG: 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 f545ede91c9d9f271e7504282cab7bf509607ead.
This reverts commit bdad0a1b79273733df9acc1be4e992fa5d70ec56.

This crashes clang. I'll follow up with reproduction instructions.

Added: 
    

Modified: 
    clang/include/clang/AST/Expr.h
    clang/include/clang/Sema/Sema.h
    clang/lib/AST/ASTImporter.cpp
    clang/lib/Parse/ParseExpr.cpp
    clang/lib/Sema/SemaExpr.cpp
    clang/lib/Sema/SemaExprCXX.cpp
    clang/lib/Sema/TreeTransform.h
    clang/test/SemaTemplate/dependent-expr.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index ffc1f54fe82d..7271dbb830a2 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -3959,14 +3959,14 @@ class StmtExpr : public Expr {
   Stmt *SubStmt;
   SourceLocation LParenLoc, RParenLoc;
 public:
+  // FIXME: Does type-dependence need to be computed 
diff erently?
+  // FIXME: Do we need to compute instantiation instantiation-dependence for
+  // statements? (ugh!)
   StmtExpr(CompoundStmt *substmt, QualType T,
-           SourceLocation lp, SourceLocation rp, bool InDependentContext) :
-    // Note: we treat a statement-expression in a dependent context as always
-    // being value- and instantiation-dependent. This matches the behavior of
-    // lambda-expressions and GCC.
+           SourceLocation lp, SourceLocation rp) :
     Expr(StmtExprClass, T, VK_RValue, OK_Ordinary,
-         T->isDependentType(), InDependentContext, InDependentContext, false),
-    SubStmt(substmt), LParenLoc(lp), RParenLoc(rp) {}
+         T->isDependentType(), false, false, false),
+    SubStmt(substmt), LParenLoc(lp), RParenLoc(rp) { }
 
   /// Build an empty statement expression.
   explicit StmtExpr(EmptyShell Empty) : Expr(StmtExprClass, Empty) { }

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 8e4d0828e2d0..2304a9718567 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -4964,10 +4964,8 @@ class Sema final {
                             LabelDecl *TheDecl);
 
   void ActOnStartStmtExpr();
-  ExprResult ActOnStmtExpr(Scope *S, SourceLocation LPLoc, Stmt *SubStmt,
-                           SourceLocation RPLoc);
-  ExprResult BuildStmtExpr(SourceLocation LPLoc, Stmt *SubStmt,
-                           SourceLocation RPLoc, bool IsDependent);
+  ExprResult ActOnStmtExpr(SourceLocation LPLoc, Stmt *SubStmt,
+                           SourceLocation RPLoc); // "({..})"
   // Handle the final expression in a statement expression.
   ExprResult ActOnStmtExprResult(ExprResult E);
   void ActOnStmtExprError();

diff  --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index ddbd3699bdc2..9f174e9c2440 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -6631,9 +6631,8 @@ ExpectedStmt ASTNodeImporter::VisitStmtExpr(StmtExpr *E) {
   if (Err)
     return std::move(Err);
 
-  return new (Importer.getToContext())
-      StmtExpr(ToSubStmt, ToType, ToLParenLoc, ToRParenLoc,
-               E->isInstantiationDependent());
+  return new (Importer.getToContext()) StmtExpr(
+      ToSubStmt, ToType, ToLParenLoc, ToRParenLoc);
 }
 
 ExpectedStmt ASTNodeImporter::VisitUnaryOperator(UnaryOperator *E) {

diff  --git a/clang/lib/Parse/ParseExpr.cpp b/clang/lib/Parse/ParseExpr.cpp
index b038e6935d87..584de6b87d90 100644
--- a/clang/lib/Parse/ParseExpr.cpp
+++ b/clang/lib/Parse/ParseExpr.cpp
@@ -2655,8 +2655,7 @@ Parser::ParseParenExpression(ParenParseOption &ExprType, bool stopIfCastExpr,
 
       // If the substmt parsed correctly, build the AST node.
       if (!Stmt.isInvalid()) {
-        Result = Actions.ActOnStmtExpr(getCurScope(), OpenLoc, Stmt.get(),
-                                       Tok.getLocation());
+        Result = Actions.ActOnStmtExpr(OpenLoc, Stmt.get(), Tok.getLocation());
       } else {
         Actions.ActOnStmtExprError();
       }

diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 291c38ab20b4..f50a77a40510 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -13911,14 +13911,9 @@ void Sema::ActOnStmtExprError() {
   PopExpressionEvaluationContext();
 }
 
-ExprResult Sema::ActOnStmtExpr(Scope *S, SourceLocation LPLoc, Stmt *SubStmt,
-                               SourceLocation RPLoc) {
-  return BuildStmtExpr(LPLoc, SubStmt, RPLoc,
-                       S->getTemplateParamParent() != nullptr);
-}
-
-ExprResult Sema::BuildStmtExpr(SourceLocation LPLoc, Stmt *SubStmt,
-                               SourceLocation RPLoc, bool IsDependent) {
+ExprResult
+Sema::ActOnStmtExpr(SourceLocation LPLoc, Stmt *SubStmt,
+                    SourceLocation RPLoc) { // "({..})"
   assert(SubStmt && isa<CompoundStmt>(SubStmt) && "Invalid action invocation!");
   CompoundStmt *Compound = cast<CompoundStmt>(SubStmt);
 
@@ -13949,8 +13944,7 @@ ExprResult Sema::BuildStmtExpr(SourceLocation LPLoc, Stmt *SubStmt,
 
   // FIXME: Check that expression type is complete/non-abstract; statement
   // expressions are not lvalues.
-  Expr *ResStmtExpr =
-      new (Context) StmtExpr(Compound, Ty, LPLoc, RPLoc, IsDependent);
+  Expr *ResStmtExpr = new (Context) StmtExpr(Compound, Ty, LPLoc, RPLoc);
   if (StmtExprMayBindToTemp)
     return MaybeBindToTemporary(ResStmtExpr);
   return ResStmtExpr;

diff  --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index b2e21bebd38f..3a4208c44530 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -6946,9 +6946,8 @@ Stmt *Sema::MaybeCreateStmtWithCleanups(Stmt *SubStmt) {
   // a new AsmStmtWithTemporaries.
   CompoundStmt *CompStmt = CompoundStmt::Create(
       Context, SubStmt, SourceLocation(), SourceLocation());
-  Expr *E = new (Context)
-      StmtExpr(CompStmt, Context.VoidTy, SourceLocation(), SourceLocation(),
-               CurContext->isDependentContext());
+  Expr *E = new (Context) StmtExpr(CompStmt, Context.VoidTy, SourceLocation(),
+                                   SourceLocation());
   return MaybeCreateExprWithCleanups(E);
 }
 

diff  --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 382496dad3d5..002b73c3a1dd 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -2549,9 +2549,10 @@ class TreeTransform {
   ///
   /// By default, performs semantic analysis to build the new expression.
   /// Subclasses may override this routine to provide 
diff erent behavior.
-  ExprResult RebuildStmtExpr(SourceLocation LParenLoc, Stmt *SubStmt,
-                             SourceLocation RParenLoc, bool IsDependent) {
-    return getSema().BuildStmtExpr(LParenLoc, SubStmt, RParenLoc, IsDependent);
+  ExprResult RebuildStmtExpr(SourceLocation LParenLoc,
+                                   Stmt *SubStmt,
+                                   SourceLocation RParenLoc) {
+    return getSema().ActOnStmtExpr(LParenLoc, SubStmt, RParenLoc);
   }
 
   /// Build a new __builtin_choose_expr expression.
@@ -10432,20 +10433,16 @@ TreeTransform<Derived>::TransformStmtExpr(StmtExpr *E) {
     return ExprError();
   }
 
-  // FIXME: This is not correct when substituting inside a templated
-  // context that isn't a DeclContext (such as a variable template).
-  bool IsDependent = getSema().CurContext->isDependentContext();
-
   if (!getDerived().AlwaysRebuild() &&
-      IsDependent == E->isInstantiationDependent() &&
       SubStmt.get() == E->getSubStmt()) {
     // Calling this an 'error' is unintuitive, but it does the right thing.
     SemaRef.ActOnStmtExprError();
     return SemaRef.MaybeBindToTemporary(E);
   }
 
-  return getDerived().RebuildStmtExpr(E->getLParenLoc(), SubStmt.get(),
-                                      E->getRParenLoc(), IsDependent);
+  return getDerived().RebuildStmtExpr(E->getLParenLoc(),
+                                      SubStmt.get(),
+                                      E->getRParenLoc());
 }
 
 template<typename Derived>
@@ -11891,8 +11888,6 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
     NewTrailingRequiresClause = getDerived().TransformExpr(TRC);
 
   // Create the local class that will describe the lambda.
-  // FIXME: KnownDependent below is wrong when substituting inside a templated
-  // context that isn't a DeclContext (such as a variable template).
   CXXRecordDecl *OldClass = E->getLambdaClass();
   CXXRecordDecl *Class
     = getSema().createLambdaClosureType(E->getIntroducerRange(),

diff  --git a/clang/test/SemaTemplate/dependent-expr.cpp b/clang/test/SemaTemplate/dependent-expr.cpp
index 97d157f86424..bb1e239c3490 100644
--- a/clang/test/SemaTemplate/dependent-expr.cpp
+++ b/clang/test/SemaTemplate/dependent-expr.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
+// expected-no-diagnostics
 
 // PR5908
 template <typename Iterator>
@@ -107,29 +108,3 @@ namespace PR18152 {
   };
   template struct A<0>;
 }
-
-template<typename T> void stmt_expr_1() {
-  static_assert( ({ false; }), "" );
-}
-void stmt_expr_2() {
-  static_assert( ({ false; }), "" ); // expected-error {{failed}}
-}
-
-namespace PR45083 {
-  struct A { bool x; };
-
-  template<typename> struct B : A {
-    void f() {
-      const int n = ({ if (x) {} 0; });
-    }
-  };
-
-  template void B<int>::f();
-
-  // Make sure we properly rebuild statement expression AST nodes even if the
-  // only thing that changes is the "is dependent" flag.
-  template<typename> void f() {
-    decltype(({})) x; // expected-error {{incomplete type}}
-  }
-  template void f<int>(); // expected-note {{instantiation of}}
-}


        


More information about the cfe-commits mailing list