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@-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}} } 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'}} \ _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
