]> granicus.if.org Git - clang/commitdiff
Fix crash when trying to pack-expand a GNU statement expression.
authorRichard Smith <richard-llvm@metafoo.co.uk>
Sat, 3 Feb 2018 00:44:57 +0000 (00:44 +0000)
committerRichard Smith <richard-llvm@metafoo.co.uk>
Sat, 3 Feb 2018 00:44:57 +0000 (00:44 +0000)
We could in principle support such pack expansion, using techniques similar to
what we do for pack expansion of lambdas, but it's not clear it's worthwhile.
For now at least, cleanly reject these cases rather than crashing.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@324160 91177308-0d34-0410-b5e6-96231b3b80d8

include/clang/Sema/ScopeInfo.h
include/clang/Sema/Sema.h
lib/Parse/ParseOpenMP.cpp
lib/Parse/ParseStmt.cpp
lib/Sema/Sema.cpp
lib/Sema/SemaStmt.cpp
lib/Sema/SemaTemplateVariadic.cpp
test/SemaTemplate/stmt-expr.cpp [new file with mode: 0644]

index 3aa34ae0e011fdc1e662a92ae3981610502a3ffd..5f96f595631a08a8d51cab829304f17c8b90f458 100644 (file)
@@ -55,13 +55,17 @@ namespace sema {
 /// parsed.
 class CompoundScopeInfo {
 public:
-  CompoundScopeInfo()
-    : HasEmptyLoopBodies(false) { }
+  CompoundScopeInfo(bool IsStmtExpr)
+    : HasEmptyLoopBodies(false), IsStmtExpr(IsStmtExpr) { }
 
   /// \brief Whether this compound stamement contains `for' or `while' loops
   /// with empty bodies.
   bool HasEmptyLoopBodies;
 
+  /// \brief Whether this compound statement corresponds to a GNU statement
+  /// expression.
+  bool IsStmtExpr;
+
   void setHasEmptyLoopBodies() {
     HasEmptyLoopBodies = true;
   }
index 7349edff6177c3798b47e8ffacbb68f043bda87e..2614a64d4ac8f465ba0e598997e1075efa5e02f5 100644 (file)
@@ -1339,7 +1339,7 @@ public:
       getCurFunction()->recordUseOfWeak(E, IsRead);
   }
 
-  void PushCompoundScope();
+  void PushCompoundScope(bool IsStmtExpr);
   void PopCompoundScope();
 
   sema::CompoundScopeInfo &getCurCompoundScope() const;
@@ -3667,7 +3667,7 @@ public:
   StmtResult ActOnNullStmt(SourceLocation SemiLoc,
                            bool HasLeadingEmptyMacro = false);
 
-  void ActOnStartOfCompoundStmt();
+  void ActOnStartOfCompoundStmt(bool IsStmtExpr);
   void ActOnFinishOfCompoundStmt();
   StmtResult ActOnCompoundStmt(SourceLocation L, SourceLocation R,
                                ArrayRef<Stmt *> Elts, bool isStmtExpr);
@@ -3675,8 +3675,8 @@ public:
   /// \brief A RAII object to enter scope of a compound statement.
   class CompoundScopeRAII {
   public:
-    CompoundScopeRAII(Sema &S): S(S) {
-      S.ActOnStartOfCompoundStmt();
+    CompoundScopeRAII(Sema &S, bool IsStmtExpr = false) : S(S) {
+      S.ActOnStartOfCompoundStmt(IsStmtExpr);
     }
 
     ~CompoundScopeRAII() {
index f3edf7ca51f40b2751326e4cd800b0ea86ce136d..e9f2029fb705a98fc54998299ed32602ee7404f0 100644 (file)
@@ -1080,21 +1080,18 @@ StmtResult Parser::ParseOpenMPDeclarativeOrExecutableDirective(
     StmtResult AssociatedStmt;
     if (HasAssociatedStatement) {
       // The body is a block scope like in Lambdas and Blocks.
-      Sema::CompoundScopeRAII CompoundScope(Actions);
       Actions.ActOnOpenMPRegionStart(DKind, getCurScope());
-      Actions.ActOnStartOfCompoundStmt();
-      // Parse statement
-      AssociatedStmt = ParseStatement();
-      Actions.ActOnFinishOfCompoundStmt();
+      // FIXME: We create a bogus CompoundStmt scope to hold the contents of
+      // the captured region. Code elsewhere assumes that any FunctionScopeInfo
+      // should have at least one compound statement scope within it.
+      AssociatedStmt = (Sema::CompoundScopeRAII(Actions), ParseStatement());
       AssociatedStmt = Actions.ActOnOpenMPRegionEnd(AssociatedStmt, Clauses);
     } else if (DKind == OMPD_target_update || DKind == OMPD_target_enter_data ||
                DKind == OMPD_target_exit_data) {
-      Sema::CompoundScopeRAII CompoundScope(Actions);
       Actions.ActOnOpenMPRegionStart(DKind, getCurScope());
-      Actions.ActOnStartOfCompoundStmt();
-      AssociatedStmt =
-          Actions.ActOnCompoundStmt(Loc, Loc, llvm::None, /*isStmtExpr=*/false);
-      Actions.ActOnFinishOfCompoundStmt();
+      AssociatedStmt = (Sema::CompoundScopeRAII(Actions),
+                        Actions.ActOnCompoundStmt(Loc, Loc, llvm::None,
+                                                  /*isStmtExpr=*/false));
       AssociatedStmt = Actions.ActOnOpenMPRegionEnd(AssociatedStmt, Clauses);
     }
     Directive = Actions.ActOnOpenMPExecutableDirective(
index 09ae9d7f22f6d82f41bf0141972bac9d3df9df49..b63c69db4802de365b2f45c62d56e45082b26202 100644 (file)
@@ -954,7 +954,7 @@ StmtResult Parser::ParseCompoundStatementBody(bool isStmtExpr) {
   if (T.consumeOpen())
     return StmtError();
 
-  Sema::CompoundScopeRAII CompoundScope(Actions);
+  Sema::CompoundScopeRAII CompoundScope(Actions, isStmtExpr);
 
   // Parse any pragmas at the beginning of the compound statement.
   ParseCompoundStatementLeadingPragmas();
index e07f60989d8f5c3df3dd23ebe1d232ae6ad2f04b..45fd2d3249dc597741d4e02ec98c49826eaa0958 100644 (file)
@@ -1393,8 +1393,8 @@ void Sema::PopFunctionScopeInfo(const AnalysisBasedWarnings::Policy *WP,
     delete Scope;
 }
 
-void Sema::PushCompoundScope() {
-  getCurFunction()->CompoundScopes.push_back(CompoundScopeInfo());
+void Sema::PushCompoundScope(bool IsStmtExpr) {
+  getCurFunction()->CompoundScopes.push_back(CompoundScopeInfo(IsStmtExpr));
 }
 
 void Sema::PopCompoundScope() {
index 1ebc36716a881d77c97c9fe1cb36efa985c1a35b..93907038e05f9bd50054b260ec4f9fefb6818d3e 100644 (file)
@@ -337,8 +337,8 @@ void Sema::DiagnoseUnusedExprResult(const Stmt *S) {
   DiagRuntimeBehavior(Loc, nullptr, PDiag(DiagID) << R1 << R2);
 }
 
-void Sema::ActOnStartOfCompoundStmt() {
-  PushCompoundScope();
+void Sema::ActOnStartOfCompoundStmt(bool IsStmtExpr) {
+  PushCompoundScope(IsStmtExpr);
 }
 
 void Sema::ActOnFinishOfCompoundStmt() {
index d81837dad5085fac3a118051826e0be92a8e7bdc..7aa0e317df0f592c7f9f01b8bc48ccc5ee45c235 100644 (file)
@@ -314,8 +314,18 @@ Sema::DiagnoseUnexpandedParameterPacks(SourceLocation Loc,
   // later.
   SmallVector<UnexpandedParameterPack, 4> LambdaParamPackReferences;
   for (unsigned N = FunctionScopes.size(); N; --N) {
-    if (sema::LambdaScopeInfo *LSI =
-          dyn_cast<sema::LambdaScopeInfo>(FunctionScopes[N-1])) {
+    sema::FunctionScopeInfo *Func = FunctionScopes[N-1];
+    // We do not permit pack expansion that would duplicate a statement
+    // expression, not even within a lambda.
+    // FIXME: We could probably support this for statement expressions that do
+    // not contain labels, and for pack expansions that expand both the stmt
+    // expr and the enclosing lambda.
+    if (std::any_of(
+            Func->CompoundScopes.begin(), Func->CompoundScopes.end(),
+            [](sema::CompoundScopeInfo &CSI) { return CSI.IsStmtExpr; }))
+      break;
+
+    if (auto *LSI = dyn_cast<sema::LambdaScopeInfo>(Func)) {
       if (N == FunctionScopes.size()) {
         for (auto &Param : Unexpanded) {
           auto *PD = dyn_cast_or_null<ParmVarDecl>(
diff --git a/test/SemaTemplate/stmt-expr.cpp b/test/SemaTemplate/stmt-expr.cpp
new file mode 100644 (file)
index 0000000..2516a52
--- /dev/null
@@ -0,0 +1,39 @@
+// RUN: %clang_cc1 -verify %s
+
+// FIXME: We could in principle support cases like this (particularly, cases
+// where the statement-expression contains no labels).
+template <typename... T> void f1() {
+  int arr[] = {
+    ({
+      T(); // expected-error {{unexpanded parameter pack}}
+    }) ... // expected-error {{does not contain any unexpanded parameter packs}}
+  };
+}
+
+// FIXME: The error for this isn't ideal; it'd be preferable to say that pack
+// expansion of a statement expression is not permitted.
+template <typename... T> void f2() {
+  [] {
+    int arr[] = {
+      T() + ({
+      foo:
+        T t; // expected-error {{unexpanded parameter pack}}
+        goto foo;
+        0;
+      }) ...
+    };
+  };
+}
+
+template <typename... T> void f3() {
+  ({
+    int arr[] = {
+      [] {
+      foo:
+        T t; // OK, expanded within compound statement
+        goto foo;
+        return 0;
+      } ...
+    };
+  });
+}