consteval if does not form a discarded statement
authorAaron Ballman <aaron@aaronballman.com>
Wed, 20 Oct 2021 11:23:09 +0000 (07:23 -0400)
committerAaron Ballman <aaron@aaronballman.com>
Wed, 20 Oct 2021 11:24:55 +0000 (07:24 -0400)
When we added support for if consteval, we accidentally formed a discarded
statement evaluation context for the branch-not-taken. However, a discarded
statement is a property of an if constexpr statement, not an if consteval
statement (https://eel.is/c++draft/stmt.if#2.sentence-2). This turned out to
cause issues when deducing the return type from a function with a consteval if
statement -- we wouldn't consider the branch-not-taken when deducing the return
type.

This fixes PR52206.

Note, there is additional work left to be done. We need to track discarded
statement and immediate evaluation contexts separately rather than as being
mutually exclusive.

clang/lib/Parse/ParseStmt.cpp
clang/test/SemaCXX/cxx2b-consteval-if.cpp [new file with mode: 0644]

index 4d76dfe..bb87186 100644 (file)
@@ -1441,12 +1441,13 @@ StmtResult Parser::ParseIfStatement(SourceLocation *TrailingElseLoc) {
   SourceLocation InnerStatementTrailingElseLoc;
   StmtResult ThenStmt;
   {
-    bool ShouldEnter =
-        (ConstexprCondition && !*ConstexprCondition) || IsConsteval;
+    bool ShouldEnter = ConstexprCondition && !*ConstexprCondition;
     Sema::ExpressionEvaluationContext Context =
         Sema::ExpressionEvaluationContext::DiscardedStatement;
-    if (NotLocation.isInvalid() && IsConsteval)
+    if (NotLocation.isInvalid() && IsConsteval) {
       Context = Sema::ExpressionEvaluationContext::ImmediateFunctionContext;
+      ShouldEnter = true;
+    }
 
     EnterExpressionEvaluationContext PotentiallyDiscarded(
         Actions, Context, nullptr,
@@ -1485,12 +1486,13 @@ StmtResult Parser::ParseIfStatement(SourceLocation *TrailingElseLoc) {
                           Tok.is(tok::l_brace));
 
     MisleadingIndentationChecker MIChecker(*this, MSK_else, ElseLoc);
-    bool ShouldEnter =
-        (ConstexprCondition && *ConstexprCondition) || IsConsteval;
+    bool ShouldEnter = ConstexprCondition && *ConstexprCondition;
     Sema::ExpressionEvaluationContext Context =
         Sema::ExpressionEvaluationContext::DiscardedStatement;
-    if (NotLocation.isValid() && IsConsteval)
+    if (NotLocation.isValid() && IsConsteval) {
       Context = Sema::ExpressionEvaluationContext::ImmediateFunctionContext;
+      ShouldEnter = true;
+    }
 
     EnterExpressionEvaluationContext PotentiallyDiscarded(
         Actions, Context, nullptr,
diff --git a/clang/test/SemaCXX/cxx2b-consteval-if.cpp b/clang/test/SemaCXX/cxx2b-consteval-if.cpp
new file mode 100644 (file)
index 0000000..e60959e
--- /dev/null
@@ -0,0 +1,57 @@
+// RUN: %clang_cc1 -std=c++2b -verify %s
+
+namespace PR52206 {
+constexpr auto f() {
+  if consteval  { return 0;   }
+  if !consteval { return 0.0; } // expected-error {{'auto' in return type deduced as 'double' here but deduced as 'int' in earlier return statement}}
+}
+
+constexpr auto g() {
+  if !consteval { return 0;   }
+  if consteval  { return 0.0; } // expected-error {{'auto' in return type deduced as 'double' here but deduced as 'int' in earlier return statement}}
+}
+
+constexpr auto h() {
+  if consteval  { return 0; }
+  if !consteval { return 0; } // okay
+}
+
+constexpr auto i() {
+  if consteval {
+    if consteval { // expected-warning {{consteval if is always true in an immediate context}}
+         return 1;
+       }
+       return 2;
+  } else {
+    return 1.0; // expected-error {{'auto' in return type deduced as 'double' here but deduced as 'int' in earlier return statement}}
+  }
+}
+
+void test() {
+  auto x1 = f();
+  constexpr auto y1 = f();
+
+  auto x2 = g();
+  constexpr auto y2 = g();
+
+  auto x3 = h();
+  constexpr auto y3 = h();
+
+  auto x4 = i();
+  constexpr auto y4 = i();
+}
+} // namespace PR52206
+
+consteval int *make() { return new int; }
+auto f() {
+  if constexpr (false) {
+    if consteval {
+      // Immediate function context, so call to `make()` is valid.
+      // Discarded statement context, so `return 0;` is valid too.
+      delete make();
+      return 0;
+    }
+  }
+  // FIXME: this error should not happen.
+  return 0.0; // expected-error {{'auto' in return type deduced as 'double' here but deduced as 'int' in earlier return statement}}
+}