[clang] 71059f2 - [AST] Produce ReturnStmt containing RecoveryExpr when type is wrong
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 4 08:08:20 PST 2022
Author: Sam McCall
Date: 2022-01-04T17:07:55+01:00
New Revision: 71059f26d31398d109be057e35bb8c5960d8aaf6
URL: https://github.com/llvm/llvm-project/commit/71059f26d31398d109be057e35bb8c5960d8aaf6
DIFF: https://github.com/llvm/llvm-project/commit/71059f26d31398d109be057e35bb8c5960d8aaf6.diff
LOG: [AST] Produce ReturnStmt containing RecoveryExpr when type is wrong
Previously we just drop the ReturnStmt and its argument from the AST,
which blocks analysis of broken code.
Fixes https://github.com/llvm/llvm-project/issues/39944
Differential Revision: https://reviews.llvm.org/D116414
Added:
Modified:
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaStmt.cpp
clang/test/AST/ast-dump-recovery.cpp
clang/test/SemaCXX/constant-expression-cxx11.cpp
clang/test/SemaCXX/constant-expression-cxx14.cpp
clang/test/SemaCXX/constexpr-function-recovery-crash.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 1b3944b35cb41..9521b24e44a79 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -4861,7 +4861,8 @@ class Sema final {
StmtResult ActOnReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp,
Scope *CurScope);
- StmtResult BuildReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp);
+ StmtResult BuildReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp,
+ bool AllowRecovery = false);
StmtResult ActOnCapScopeReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp,
NamedReturnInfo &NRInfo,
bool SupressSimplerImplicitMoves);
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index 1d90759f24069..d18f89d60d787 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -3878,7 +3878,8 @@ Sema::ActOnReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp,
RetValExp, nullptr, /*RecoverUncorrectedTypos=*/true);
if (RetVal.isInvalid())
return StmtError();
- StmtResult R = BuildReturnStmt(ReturnLoc, RetVal.get());
+ StmtResult R =
+ BuildReturnStmt(ReturnLoc, RetVal.get(), /*AllowRecovery=*/true);
if (R.isInvalid() || ExprEvalContexts.back().isDiscardedStatementContext())
return R;
@@ -3908,7 +3909,8 @@ static bool CheckSimplerImplicitMovesMSVCWorkaround(const Sema &S,
return false;
}
-StmtResult Sema::BuildReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp) {
+StmtResult Sema::BuildReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp,
+ bool AllowRecovery) {
// Check for unexpanded parameter packs.
if (RetValExp && DiagnoseUnexpandedParameterPack(RetValExp))
return StmtError();
@@ -3985,11 +3987,25 @@ StmtResult Sema::BuildReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp) {
// If we've already decided this function is invalid, e.g. because
// we saw a `return` whose expression had an error, don't keep
// trying to deduce its return type.
- if (FD->isInvalidDecl())
- return StmtError();
- if (DeduceFunctionTypeFromReturnExpr(FD, ReturnLoc, RetValExp, AT)) {
+ // (Some return values may be needlessly wrapped in RecoveryExpr).
+ if (FD->isInvalidDecl() ||
+ DeduceFunctionTypeFromReturnExpr(FD, ReturnLoc, RetValExp, AT)) {
FD->setInvalidDecl();
- return StmtError();
+ if (!AllowRecovery)
+ return StmtError();
+ // The deduction failure is diagnosed and marked, try to recover.
+ if (RetValExp) {
+ // Wrap return value with a recovery expression of the previous type.
+ // If no deduction yet, use DependentTy.
+ auto Recovery = CreateRecoveryExpr(
+ RetValExp->getBeginLoc(), RetValExp->getEndLoc(), RetValExp,
+ AT->isDeduced() ? FnRetType : QualType());
+ if (Recovery.isInvalid())
+ return StmtError();
+ RetValExp = Recovery.get();
+ } else {
+ // Nothing to do: a ReturnStmt with no value is fine recovery.
+ }
} else {
FnRetType = FD->getReturnType();
}
@@ -4002,7 +4018,7 @@ StmtResult Sema::BuildReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp) {
ReturnStmt *Result = nullptr;
if (FnRetType->isVoidType()) {
if (RetValExp) {
- if (isa<InitListExpr>(RetValExp)) {
+ if (auto *ILE = dyn_cast<InitListExpr>(RetValExp)) {
// We simply never allow init lists as the return value of void
// functions. This is compatible because this was never allowed before,
// so there's no legacy code to deal with.
@@ -4018,8 +4034,12 @@ StmtResult Sema::BuildReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp) {
Diag(ReturnLoc, diag::err_return_init_list)
<< CurDecl << FunctionKind << RetValExp->getSourceRange();
- // Drop the expression.
- RetValExp = nullptr;
+ // Preserve the initializers in the AST.
+ RetValExp = AllowRecovery
+ ? CreateRecoveryExpr(ILE->getLBraceLoc(),
+ ILE->getRBraceLoc(), ILE->inits())
+ .get()
+ : nullptr;
} else if (!RetValExp->isTypeDependent()) {
// C99 6.8.6.4p1 (ext_ since GCC warns)
unsigned D = diag::ext_return_has_expr;
@@ -4116,6 +4136,9 @@ StmtResult Sema::BuildReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp) {
InitializedEntity::InitializeResult(ReturnLoc, RetType);
ExprResult Res = PerformMoveOrCopyInitialization(
Entity, NRInfo, RetValExp, SupressSimplerImplicitMoves);
+ if (Res.isInvalid() && AllowRecovery)
+ Res = CreateRecoveryExpr(RetValExp->getBeginLoc(),
+ RetValExp->getEndLoc(), RetValExp, RetType);
if (Res.isInvalid()) {
// FIXME: Clean up temporaries here anyway?
return StmtError();
diff --git a/clang/test/AST/ast-dump-recovery.cpp b/clang/test/AST/ast-dump-recovery.cpp
index 8c6563961bd60..c196f629bad92 100644
--- a/clang/test/AST/ast-dump-recovery.cpp
+++ b/clang/test/AST/ast-dump-recovery.cpp
@@ -351,3 +351,43 @@ void CtorInitializer() {
// CHECK-NEXT: | `-RecoveryExpr {{.*}} '<dependent type>'
};
}
+
+float *brokenReturn() {
+ // CHECK: FunctionDecl {{.*}} brokenReturn
+ return 42;
+ // CHECK: ReturnStmt
+ // CHECK-NEXT: `-RecoveryExpr {{.*}} 'float *'
+ // CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 42
+}
+
+// Return deduction treats the first, second *and* third
diff erently!
+auto *brokenDeducedReturn(int *x, float *y, double *z) {
+ // CHECK: FunctionDecl {{.*}} invalid brokenDeducedReturn
+ if (x) return x;
+ // CHECK: ReturnStmt
+ // CHECK-NEXT: `-ImplicitCastExpr {{.*}} <LValueToRValue>
+ // CHECK-NEXT: `-DeclRefExpr {{.*}} 'x' 'int *'
+ if (y) return y;
+ // CHECK: ReturnStmt
+ // CHECK-NEXT: `-RecoveryExpr {{.*}} 'int *'
+ // CHECK-NEXT: `-DeclRefExpr {{.*}} 'y' 'float *'
+ if (z) return z;
+ // CHECK: ReturnStmt
+ // CHECK-NEXT: `-RecoveryExpr {{.*}} 'int *'
+ // CHECK-NEXT: `-DeclRefExpr {{.*}} 'z' 'double *'
+ return x;
+ // Unfortunate: we wrap a valid return in RecoveryExpr.
+ // This is to avoid running deduction again after it failed once.
+ // CHECK: ReturnStmt
+ // CHECK-NEXT: `-RecoveryExpr {{.*}} 'int *'
+ // CHECK-NEXT: `-DeclRefExpr {{.*}} 'x' 'int *'
+}
+
+void returnInitListFromVoid() {
+ // CHECK: FunctionDecl {{.*}} returnInitListFromVoid
+ return {7,8};
+ // CHECK: ReturnStmt
+ // CHECK-NEXT: `-RecoveryExpr {{.*}} '<dependent type>'
+ // CHECK-NEXT: |-IntegerLiteral {{.*}} 'int' 7
+ // CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 8
+}
diff --git a/clang/test/SemaCXX/constant-expression-cxx11.cpp b/clang/test/SemaCXX/constant-expression-cxx11.cpp
index c338214b82312..680b2d5307bef 100644
--- a/clang/test/SemaCXX/constant-expression-cxx11.cpp
+++ b/clang/test/SemaCXX/constant-expression-cxx11.cpp
@@ -1939,20 +1939,16 @@ namespace Lifetime {
constexpr int &get(int &&n) { return n; }
// cxx2b-error at -1 {{non-const lvalue reference to type 'int' cannot bind to a temporary of type 'int'}}
- // cxx2b-error at -2 {{no return statement in constexpr function}} See PR40598
constexpr int &&get_rv(int &&n) { return static_cast<int&&>(n); }
struct S {
int &&r;
int &s;
int t;
- constexpr S() : r(get_rv(0)), s(get(0)), t(r) {} // expected-note {{read of object outside its lifetime}}
- constexpr S(int) : r(get_rv(0)), s(get(0)), t(s) {}
- // cxx2b-warning at -1 {{reference 's' is not yet bound to a value when used here}}
- // cxx2b-note at -2 {{read of uninitialized object is not allowed in a constant expression}}
- // cxx11_20-note at -3 {{read of object outside its lifetime}}
+ constexpr S() : r(get_rv(0)), s(get(0)), t(r) {} // cxx11_20-note {{read of object outside its lifetime}}
+ constexpr S(int) : r(get_rv(0)), s(get(0)), t(s) {} // cxx11_20-note {{read of object outside its lifetime}}
};
- constexpr int k1 = S().t; // expected-error {{constant expression}} expected-note {{in call}}
- constexpr int k2 = S(0).t; // expected-error {{constant expression}} expected-note {{in call}}
+ constexpr int k1 = S().t; // expected-error {{constant expression}} cxx11_20-note {{in call}}
+ constexpr int k2 = S(0).t; // expected-error {{constant expression}} cxx11_20-note {{in call}}
struct Q {
int n = 0;
diff --git a/clang/test/SemaCXX/constant-expression-cxx14.cpp b/clang/test/SemaCXX/constant-expression-cxx14.cpp
index ee6d796cdcc42..84ffad3707891 100644
--- a/clang/test/SemaCXX/constant-expression-cxx14.cpp
+++ b/clang/test/SemaCXX/constant-expression-cxx14.cpp
@@ -876,14 +876,12 @@ namespace VirtualFromBase {
namespace Lifetime {
constexpr int &get(int &&r) { return r; }
// cxx2b-error at -1 {{non-const lvalue reference to type 'int' cannot bind to a temporary of type 'int'}}
- // cxx2b-error at -2 {{no return statement in constexpr function}} See PR40598
constexpr int f() {
int &r = get(123);
return r;
- // cxx2b-note at -1 {{use of reference outside its lifetime is not allowed in a constant expression}}
- // cxx14_20-note at -2 {{read of object outside its lifetime}}
+ // cxx14_20-note at -1 {{read of object outside its lifetime}}
}
- static_assert(f() == 123, ""); // expected-error {{constant expression}} expected-note {{in call}}
+ static_assert(f() == 123, ""); // expected-error {{constant expression}} cxx14_20-note {{in call}}
constexpr int g() {
int *p = 0;
diff --git a/clang/test/SemaCXX/constexpr-function-recovery-crash.cpp b/clang/test/SemaCXX/constexpr-function-recovery-crash.cpp
index 4b73cff138386..6f0844f1e0d06 100644
--- a/clang/test/SemaCXX/constexpr-function-recovery-crash.cpp
+++ b/clang/test/SemaCXX/constexpr-function-recovery-crash.cpp
@@ -74,3 +74,6 @@ struct X {} array[] = {undef()}; // expected-error {{use of undeclared identifie
constexpr void test11() {
for (X& e : array) {}
}
+
+constexpr int test12() { return "wrong"; } // expected-error {{cannot initialize return object of type 'int'}}
+constexpr int force12 = test12(); // expected-error {{must be initialized by a constant}}
More information about the cfe-commits
mailing list