[clang] e0cdafe - [AST] Better recovery on an expression refers to an invalid decl.
Haojian Wu via cfe-commits
cfe-commits at lists.llvm.org
Thu Sep 22 05:31:49 PDT 2022
Author: Haojian Wu
Date: 2022-09-22T14:23:47+02:00
New Revision: e0cdafe8d4b2f1585f4756447b677fec37954ec4
URL: https://github.com/llvm/llvm-project/commit/e0cdafe8d4b2f1585f4756447b677fec37954ec4
DIFF: https://github.com/llvm/llvm-project/commit/e0cdafe8d4b2f1585f4756447b677fec37954ec4.diff
LOG: [AST] Better recovery on an expression refers to an invalid decl.
Prior to the patch, we didn't build a DeclRefExpr if the Decl being
referred to is invalid, because many clang downstream AST consumers
assume it, violating it will cause many diagnostic regressions.
With this patch, we build a DeclRefExpr enven for an invalid decl (when the
AcceptInvalidDecl is true), and wrap it with a dependent-type
RecoveryExpr (to prevent follow-up semantic analysis, and diagnostic
regressions).
This is a revised version of https://reviews.llvm.org/D76831
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D121599
Added:
Modified:
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaExpr.cpp
clang/test/AST/ast-dump-recovery.cpp
clang/test/SemaTemplate/constraints.cpp
clang/test/SemaTemplate/cxx2a-constraint-exprs.cpp
clang/test/SemaTemplate/instantiate-var-template.cpp
Removed:
################################################################################
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index a916db238389f..85b2bca535809 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -1275,7 +1275,7 @@ ExprResult Sema::ActOnNameClassifiedAsNonType(Scope *S, const CXXScopeSpec &SS,
Result.resolveKind();
bool ADL = UseArgumentDependentLookup(SS, Result, NextToken.is(tok::l_paren));
- return BuildDeclarationNameExpr(SS, Result, ADL);
+ return BuildDeclarationNameExpr(SS, Result, ADL, /*AcceptInvalidDecl=*/true);
}
ExprResult Sema::ActOnNameClassifiedAsOverloadSet(Scope *S, Expr *E) {
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index b49b7ce45cf4a..7f70cf331c7f9 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -3183,8 +3183,9 @@ bool Sema::UseArgumentDependentLookup(const CXXScopeSpec &SS,
/// as an expression. This is only actually called for lookups that
/// were not overloaded, and it doesn't promise that the declaration
/// will in fact be used.
-static bool CheckDeclInExpr(Sema &S, SourceLocation Loc, NamedDecl *D) {
- if (D->isInvalidDecl())
+static bool CheckDeclInExpr(Sema &S, SourceLocation Loc, NamedDecl *D,
+ bool AcceptInvalid) {
+ if (D->isInvalidDecl() && !AcceptInvalid)
return true;
if (isa<TypedefNameDecl>(D)) {
@@ -3230,7 +3231,8 @@ ExprResult Sema::BuildDeclarationNameExpr(const CXXScopeSpec &SS,
// result, because in the overloaded case the results can only be
// functions and function templates.
if (R.isSingleResult() && !ShouldLookupResultBeMultiVersionOverload(R) &&
- CheckDeclInExpr(*this, R.getNameLoc(), R.getFoundDecl()))
+ CheckDeclInExpr(*this, R.getNameLoc(), R.getFoundDecl(),
+ AcceptInvalidDecl))
return ExprError();
// Otherwise, just build an unresolved lookup expression. Suppress
@@ -3262,7 +3264,7 @@ ExprResult Sema::BuildDeclarationNameExpr(
"Cannot refer unambiguously to a function template");
SourceLocation Loc = NameInfo.getLoc();
- if (CheckDeclInExpr(*this, Loc, D)) {
+ if (CheckDeclInExpr(*this, Loc, D, AcceptInvalidDecl)) {
// Recovery from invalid cases (e.g. D is an invalid Decl).
// We use the dependent type for the RecoveryExpr to prevent bogus follow-up
// diagnostics, as invalid decls use int as a fallback type.
@@ -3494,9 +3496,16 @@ ExprResult Sema::BuildDeclarationNameExpr(
break;
}
- return BuildDeclRefExpr(VD, type, valueKind, NameInfo, &SS, FoundD,
- /*FIXME: TemplateKWLoc*/ SourceLocation(),
- TemplateArgs);
+ auto *E =
+ BuildDeclRefExpr(VD, type, valueKind, NameInfo, &SS, FoundD,
+ /*FIXME: TemplateKWLoc*/ SourceLocation(), TemplateArgs);
+ // Clang AST consumers assume a DeclRefExpr refers to a valid decl. We
+ // wrap a DeclRefExpr referring to an invalid decl with a dependent-type
+ // RecoveryExpr to avoid follow-up semantic analysis (thus prevent bogus
+ // diagnostics).
+ if (VD->isInvalidDecl() && E)
+ return CreateRecoveryExpr(E->getBeginLoc(), E->getEndLoc(), {E});
+ return E;
}
static void ConvertUTF8ToWideString(unsigned CharByteWidth, StringRef Source,
diff --git a/clang/test/AST/ast-dump-recovery.cpp b/clang/test/AST/ast-dump-recovery.cpp
index 53043027ddb8c..cc4f8afbfc444 100644
--- a/clang/test/AST/ast-dump-recovery.cpp
+++ b/clang/test/AST/ast-dump-recovery.cpp
@@ -406,8 +406,17 @@ void RecoveryExprForInvalidDecls(Unknown InvalidDecl) {
InvalidDecl + 1;
// CHECK: BinaryOperator {{.*}}
// CHECK-NEXT: |-RecoveryExpr {{.*}} '<dependent type>'
+ // CHECK-NEXT: | | `-DeclRefExpr {{.*}} 'InvalidDecl' 'int'
// CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 1
InvalidDecl();
// CHECK: CallExpr {{.*}}
// CHECK-NEXT: `-RecoveryExpr {{.*}} '<dependent type>'
}
+
+void RecoverToAnInvalidDecl() {
+ Unknown* foo; // invalid decl
+ goo; // the typo was correct to the invalid foo.
+ // Verify that RecoveryExpr has an inner DeclRefExpr.
+ // CHECK: RecoveryExpr {{.*}} '<dependent type>' contains-errors lvalue
+ // CHECK-NEXT: `-DeclRefExpr {{.*}} 'foo' 'int *'
+}
diff --git a/clang/test/SemaTemplate/constraints.cpp b/clang/test/SemaTemplate/constraints.cpp
index 0bc4727245f6a..7576ee345b819 100644
--- a/clang/test/SemaTemplate/constraints.cpp
+++ b/clang/test/SemaTemplate/constraints.cpp
@@ -15,12 +15,11 @@ namespace PR45589 {
template<bool B, typename T> constexpr int test = 0;
template<bool B, typename T> requires C<T> constexpr int test<B, T> = 1;
- template<bool B, typename T> requires (B && C<T>) || (X<T>::value && C<T>) constexpr int test<B, T> = 2; // expected-error {{non-constant expression}} expected-note {{subexpression}} expected-note {{instantiation of}} expected-note {{while substituting}}
+ template<bool B, typename T> requires (B && C<T>) || (X<T>::value && C<T>) constexpr int test<B, T> = 2; // expected-note {{instantiation of}} expected-note {{while substituting}}
static_assert(test<true, False> == 2);
static_assert(test<true, True> == 2);
static_assert(test<true, char> == 2); // satisfaction of second term of || not considered
static_assert(test<false, False> == 1);
static_assert(test<false, True> == 2); // constraints are partially ordered
- // FIXME: These diagnostics are excessive.
- static_assert(test<false, char> == 1); // expected-note 2{{while}} expected-note 2{{during}}
+ static_assert(test<false, char> == 1); // expected-note {{while}} expected-note {{during}}
}
diff --git a/clang/test/SemaTemplate/cxx2a-constraint-exprs.cpp b/clang/test/SemaTemplate/cxx2a-constraint-exprs.cpp
index 5a1c9196e657e..f4403587a6259 100644
--- a/clang/test/SemaTemplate/cxx2a-constraint-exprs.cpp
+++ b/clang/test/SemaTemplate/cxx2a-constraint-exprs.cpp
@@ -26,28 +26,22 @@ namespace unevaluated {
namespace constant_evaluated {
template<typename T> requires f<int[0]> struct S {};
- // expected-note at -1{{in instantiation of}} expected-note at -1{{while substituting}} \
- expected-error at -1{{substitution into constraint expression resulted in a non-constant expression}} \
- expected-note at -1{{subexpression not valid}}
+ // expected-note at -1{{in instantiation of}} expected-note at -1{{while substituting}}
using s = S<int>;
- // expected-note at -1 2{{while checking}}
+ // expected-note at -1 {{while checking}}
template<typename T> void foo() requires f<int[1]> { };
// expected-note at -1{{in instantiation}} expected-note at -1{{while substituting}} \
- expected-note at -1{{candidate template ignored}} expected-note at -1{{subexpression not valid}} \
- expected-error at -1{{substitution into constraint expression resulted in a non-constant expression}}
+ expected-note at -1{{candidate template ignored}}
int a = (foo<int>(), 0);
- // expected-note at -1 2{{while checking}} expected-error at -1{{no matching function}} \
- expected-note at -1 2{{in instantiation}}
+ // expected-note at -1 {{while checking}} expected-error at -1{{no matching function}} \
+ expected-note at -1 {{in instantiation}}
template<typename T> void bar() requires requires { requires f<int[2]>; } { };
- // expected-note at -1{{in instantiation}} expected-note at -1{{subexpression not valid}} \
+ // expected-note at -1{{in instantiation}} \
expected-note at -1{{while substituting}} \
- expected-error at -1{{substitution into constraint expression resulted in a non-constant expression}} \
- expected-note at -1 2{{while checking the satisfaction of nested requirement}}
+ expected-note at -1 {{while checking the satisfaction of nested requirement}}
int b = (bar<int>(), 0);
template<typename T> struct M { static void foo() requires f<int[3]> { }; };
- // expected-note at -1{{in instantiation}} expected-note at -1{{subexpression not valid}} \
- expected-note at -1{{while substituting}} \
- expected-error at -1{{substitution into constraint expression resulted in a non-constant expression}}
+ // expected-note at -1{{in instantiation}} expected-note at -1{{while substituting}}
int c = (M<int>::foo(), 0);
- // expected-note at -1 2{{while checking}}
+ // expected-note at -1 {{while checking}}
}
diff --git a/clang/test/SemaTemplate/instantiate-var-template.cpp b/clang/test/SemaTemplate/instantiate-var-template.cpp
index 1096795ca967f..349a80bdd0dc7 100644
--- a/clang/test/SemaTemplate/instantiate-var-template.cpp
+++ b/clang/test/SemaTemplate/instantiate-var-template.cpp
@@ -28,7 +28,7 @@ namespace InstantiationDependent {
template<typename T> constexpr T b = a<sizeof(sizeof(f(T())))>; // expected-error {{invalid application of 'sizeof' to an incomplete type 'void'}}
static_assert(b<int> == 1, "");
- static_assert(b<char> == 1, ""); // expected-note {{in instantiation of}} expected-error {{not an integral constant}}
+ static_assert(b<char> == 1, ""); // expected-note {{in instantiation of}}
template<typename T> void f() {
static_assert(a<sizeof(sizeof(f(T())))> == 0, ""); // expected-error {{static assertion failed due to requirement 'a<sizeof (sizeof (f(type-parameter-0-0())))> == 0'}} \
More information about the cfe-commits
mailing list