From ab2ca8496d54573de1c8bec204009567ba2b4086 Mon Sep 17 00:00:00 2001 From: Aaron Ballman Date: Wed, 20 Oct 2021 07:23:09 -0400 Subject: [PATCH] consteval if does not form a discarded statement 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 | 14 +++--- clang/test/SemaCXX/cxx2b-consteval-if.cpp | 57 +++++++++++++++++++++++ 2 files changed, 65 insertions(+), 6 deletions(-) create mode 100644 clang/test/SemaCXX/cxx2b-consteval-if.cpp diff --git a/clang/lib/Parse/ParseStmt.cpp b/clang/lib/Parse/ParseStmt.cpp index 4d76dfe91052..bb8718671bb0 100644 --- a/clang/lib/Parse/ParseStmt.cpp +++ b/clang/lib/Parse/ParseStmt.cpp @@ -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 index 000000000000..e60959e5988d --- /dev/null +++ b/clang/test/SemaCXX/cxx2b-consteval-if.cpp @@ -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}} +}