Re: r372437 - Fix assertion failure when constant evaluation of a switch jumps over an

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: r372437 - Fix assertion failure when constant evaluation of a switch jumps over an

Kristof Beyls via cfe-dev
It looks like there's a similar crash with while loops, e.g. "while (int m)".  That isn't valid, of course, but we try to perform constexpr evaluation anyway.

-Eli

-----Original Message-----
From: cfe-commits <[hidden email]> On Behalf Of Richard Smith via cfe-commits
Sent: Friday, September 20, 2019 4:09 PM
To: [hidden email]
Subject: [EXT] r372437 - Fix assertion failure when constant evaluation of a switch jumps over an

Author: rsmith
Date: Fri Sep 20 16:08:59 2019
New Revision: 372437

URL: http://llvm.org/viewvc/llvm-project?rev=372437&view=rev
Log:
Fix assertion failure when constant evaluation of a switch jumps over an
uninitialized variable in an init-statement of a 'for' or 'if'.

Modified:
    cfe/trunk/lib/AST/ExprConstant.cpp
    cfe/trunk/test/SemaCXX/constant-expression-cxx2a.cpp

Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=372437&r1=372436&r2=372437&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Fri Sep 20 16:08:59 2019
@@ -4131,6 +4131,21 @@ static EvalStmtResult EvaluateStmt(StmtR
       // preceded by our switch label.
       BlockScopeRAII Scope(Info);

+      // Step into the init statement in case it brings an (uninitialized)
+      // variable into scope.
+      if (const Stmt *Init = IS->getInit()) {
+        EvalStmtResult ESR = EvaluateStmt(Result, Info, Init, Case);
+        if (ESR != ESR_CaseNotFound) {
+          assert(ESR != ESR_Succeeded);
+          return ESR;
+        }
+      }
+
+      // Condition variable must be initialized if it exists.
+      // FIXME: We can skip evaluating the body if there's a condition
+      // variable, as there can't be any case labels within it.
+      // (The same is true for 'for' statements.)
+
       EvalStmtResult ESR = EvaluateStmt(Result, Info, IS->getThen(), Case);
       if (ESR != ESR_CaseNotFound || !IS->getElse())
         return ESR;
@@ -4147,6 +4162,18 @@ static EvalStmtResult EvaluateStmt(StmtR

     case Stmt::ForStmtClass: {
       const ForStmt *FS = cast<ForStmt>(S);
+      BlockScopeRAII Scope(Info);
+
+      // Step into the init statement in case it brings an (uninitialized)
+      // variable into scope.
+      if (const Stmt *Init = FS->getInit()) {
+        EvalStmtResult ESR = EvaluateStmt(Result, Info, Init, Case);
+        if (ESR != ESR_CaseNotFound) {
+          assert(ESR != ESR_Succeeded);
+          return ESR;
+        }
+      }
+
       EvalStmtResult ESR =
           EvaluateLoopBody(Result, Info, FS->getBody(), Case);
       if (ESR != ESR_Continue)

Modified: cfe/trunk/test/SemaCXX/constant-expression-cxx2a.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/constant-expression-cxx2a.cpp?rev=372437&r1=372436&r2=372437&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/constant-expression-cxx2a.cpp (original)
+++ cfe/trunk/test/SemaCXX/constant-expression-cxx2a.cpp Fri Sep 20 16:08:59 2019
@@ -620,4 +620,17 @@ namespace Uninit {
   constexpr int s1 = switch_var(1);
   constexpr int s2 = switch_var(2);
   static_assert(s1 == 1 && s2 == 2);
+
+  constexpr bool switch_into_init_stmt() {
+    switch (1) {
+      if (int n; false) {
+        for (int m; false;) {
+        case 1:
+          n = m = 1;
+          return n == 1 && m == 1;
+        }
+      }
+    }
+  }
+  static_assert(switch_into_init_stmt());
 }


_______________________________________________
cfe-commits mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: r372437 - Fix assertion failure when constant evaluation of a switch jumps over an

Kristof Beyls via cfe-dev
On Mon, 23 Sep 2019 at 14:52, Eli Friedman via cfe-dev <[hidden email]> wrote:
It looks like there's a similar crash with while loops, e.g. "while (int m)".  That isn't valid, of course, but we try to perform constexpr evaluation anyway.

Yes. That's at least not really a regression; we've crashed for a long time on things like

constexpr int f() {
    switch (1) {
        while (int m = 1) {
            case 1:
            return m;
        }
    }
}

constexpr int k = f();

I guess it'd make sense to just treat this case as non-constant rather than asserting. (Though it does raise a question of what AST invariants it's safe to rely on from the constant evaluator.)
 
-Eli

-----Original Message-----
From: cfe-commits <[hidden email]> On Behalf Of Richard Smith via cfe-commits
Sent: Friday, September 20, 2019 4:09 PM
To: [hidden email]
Subject: [EXT] r372437 - Fix assertion failure when constant evaluation of a switch jumps over an

Author: rsmith
Date: Fri Sep 20 16:08:59 2019
New Revision: 372437

URL: http://llvm.org/viewvc/llvm-project?rev=372437&view=rev
Log:
Fix assertion failure when constant evaluation of a switch jumps over an
uninitialized variable in an init-statement of a 'for' or 'if'.

Modified:
    cfe/trunk/lib/AST/ExprConstant.cpp
    cfe/trunk/test/SemaCXX/constant-expression-cxx2a.cpp

Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=372437&r1=372436&r2=372437&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Fri Sep 20 16:08:59 2019
@@ -4131,6 +4131,21 @@ static EvalStmtResult EvaluateStmt(StmtR
       // preceded by our switch label.
       BlockScopeRAII Scope(Info);

+      // Step into the init statement in case it brings an (uninitialized)
+      // variable into scope.
+      if (const Stmt *Init = IS->getInit()) {
+        EvalStmtResult ESR = EvaluateStmt(Result, Info, Init, Case);
+        if (ESR != ESR_CaseNotFound) {
+          assert(ESR != ESR_Succeeded);
+          return ESR;
+        }
+      }
+
+      // Condition variable must be initialized if it exists.
+      // FIXME: We can skip evaluating the body if there's a condition
+      // variable, as there can't be any case labels within it.
+      // (The same is true for 'for' statements.)
+
       EvalStmtResult ESR = EvaluateStmt(Result, Info, IS->getThen(), Case);
       if (ESR != ESR_CaseNotFound || !IS->getElse())
         return ESR;
@@ -4147,6 +4162,18 @@ static EvalStmtResult EvaluateStmt(StmtR

     case Stmt::ForStmtClass: {
       const ForStmt *FS = cast<ForStmt>(S);
+      BlockScopeRAII Scope(Info);
+
+      // Step into the init statement in case it brings an (uninitialized)
+      // variable into scope.
+      if (const Stmt *Init = FS->getInit()) {
+        EvalStmtResult ESR = EvaluateStmt(Result, Info, Init, Case);
+        if (ESR != ESR_CaseNotFound) {
+          assert(ESR != ESR_Succeeded);
+          return ESR;
+        }
+      }
+
       EvalStmtResult ESR =
           EvaluateLoopBody(Result, Info, FS->getBody(), Case);
       if (ESR != ESR_Continue)

Modified: cfe/trunk/test/SemaCXX/constant-expression-cxx2a.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/constant-expression-cxx2a.cpp?rev=372437&r1=372436&r2=372437&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/constant-expression-cxx2a.cpp (original)
+++ cfe/trunk/test/SemaCXX/constant-expression-cxx2a.cpp Fri Sep 20 16:08:59 2019
@@ -620,4 +620,17 @@ namespace Uninit {
   constexpr int s1 = switch_var(1);
   constexpr int s2 = switch_var(2);
   static_assert(s1 == 1 && s2 == 2);
+
+  constexpr bool switch_into_init_stmt() {
+    switch (1) {
+      if (int n; false) {
+        for (int m; false;) {
+        case 1:
+          n = m = 1;
+          return n == 1 && m == 1;
+        }
+      }
+    }
+  }
+  static_assert(switch_into_init_stmt());
 }


_______________________________________________
cfe-commits mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev