hokein created this revision.
hokein added a reviewer: sammccall.
Herald added a project: All.
hokein requested review of this revision.
Herald added a project: clang.
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
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D121599
Files:
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
Index: clang/test/SemaTemplate/instantiate-var-template.cpp
===================================================================
--- clang/test/SemaTemplate/instantiate-var-template.cpp
+++ clang/test/SemaTemplate/instantiate-var-template.cpp
@@ -28,7 +28,7 @@
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_assert failed due to requirement 'a<sizeof (sizeof (f(type-parameter-0-0())))> == 0'}}
Index: clang/test/SemaTemplate/cxx2a-constraint-exprs.cpp
===================================================================
--- clang/test/SemaTemplate/cxx2a-constraint-exprs.cpp
+++ clang/test/SemaTemplate/cxx2a-constraint-exprs.cpp
@@ -26,28 +26,22 @@
namespace constant_evaluated {
template<typename T> requires f<int[0]> struct S {};
- // expected-note@-1{{in instantiation of}} expected-note@-1{{while substituting}} \
- expected-error@-1{{substitution into constraint expression resulted in a non-constant expression}} \
- expected-note@-1{{subexpression not valid}}
+ // expected-note@-1{{in instantiation of}} expected-note@-1{{while substituting}}
using s = S<int>;
- // expected-note@-1 2{{while checking}}
+ // expected-note@-1 {{while checking}}
template<typename T> void foo() requires f<int[1]> { };
// expected-note@-1{{in instantiation}} expected-note@-1{{while substituting}} \
- expected-note@-1{{candidate template ignored}} expected-note@-1{{subexpression not valid}} \
- expected-error@-1{{substitution into constraint expression resulted in a non-constant expression}}
+ expected-note@-1{{candidate template ignored}}
int a = (foo<int>(), 0);
- // expected-note@-1 2{{while checking}} expected-error@-1{{no matching function}} \
- expected-note@-1 2{{in instantiation}}
+ // expected-note@-1 {{while checking}} expected-error@-1{{no matching function}} \
+ expected-note@-1 {{in instantiation}}
template<typename T> void bar() requires requires { requires f<int[2]>; } { };
- // expected-note@-1{{in instantiation}} expected-note@-1{{subexpression not valid}} \
+ // expected-note@-1{{in instantiation}} \
expected-note@-1{{while substituting}} \
- expected-error@-1{{substitution into constraint expression resulted in a non-constant expression}} \
- expected-note@-1 2{{while checking the satisfaction of nested requirement}}
+ expected-note@-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@-1{{in instantiation}} expected-note@-1{{subexpression not valid}} \
- expected-note@-1{{while substituting}} \
- expected-error@-1{{substitution into constraint expression resulted in a non-constant expression}}
+ // expected-note@-1{{in instantiation}} expected-note@-1{{while substituting}}
int c = (M<int>::foo(), 0);
- // expected-note@-1 2{{while checking}}
+ // expected-note@-1 {{while checking}}
}
Index: clang/test/SemaTemplate/constraints.cpp
===================================================================
--- clang/test/SemaTemplate/constraints.cpp
+++ clang/test/SemaTemplate/constraints.cpp
@@ -15,12 +15,11 @@
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}}
}
Index: clang/test/AST/ast-dump-recovery.cpp
===================================================================
--- clang/test/AST/ast-dump-recovery.cpp
+++ clang/test/AST/ast-dump-recovery.cpp
@@ -406,6 +406,7 @@
InvalidDecl + 1;
// CHECK: BinaryOperator {{.*}}
// CHECK-NEXT: |-RecoveryExpr {{.*}} '<dependent type>'
+ // CHECK-NEXT: | | `-DeclRefExpr {{.*}} 'InvalidDecl' 'int'
// CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 1
InvalidDecl();
// CHECK: CallExpr {{.*}}
Index: clang/lib/Sema/SemaExpr.cpp
===================================================================
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -3150,9 +3150,10 @@
/// 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) {
+static bool CheckDeclInExpr(Sema &S, SourceLocation Loc, NamedDecl *D,
+ bool AcceptInvalid) {
if (D->isInvalidDecl())
- return true;
+ return !AcceptInvalid;
if (isa<TypedefNameDecl>(D)) {
S.Diag(Loc, diag::err_unexpected_typedef) << D->getDeclName();
@@ -3197,7 +3198,8 @@
// 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
@@ -3228,7 +3230,7 @@
"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.
@@ -3465,9 +3467,16 @@
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);
+ // Many 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,
Index: clang/lib/Sema/SemaDecl.cpp
===================================================================
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -1255,7 +1255,7 @@
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) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
