Author: Haojian Wu Date: 2020-10-26T12:40:00+01:00 New Revision: efa9aaad703e6b150980ed1a74b4e7c9da7d85a2
URL: https://github.com/llvm/llvm-project/commit/efa9aaad703e6b150980ed1a74b4e7c9da7d85a2 DIFF: https://github.com/llvm/llvm-project/commit/efa9aaad703e6b150980ed1a74b4e7c9da7d85a2.diff LOG: [clang] Suppress "follow-up" diagnostics on recovery call expressions. Because of typo-correction, the AST can be transformed, and the transformed AST is marginally useful for diagnostics purpose, the following diagnostics usually do harm than good (easily cause confusions). Given the following code: ``` void abcc(); void test() { if (abc()); // diagnostic 1 (for the typo-correction): the typo is correct to `abcc()`, so the code is treate as `if (abcc())` in AST perspective; // diagnostic 2 (for mismatch type): we perform an type-analysis on `if`, discover the type is not match } ``` The secondary diagnostic "convertable to bool" is likely bogus to users. The idea is to use RecoveryExpr (clang's dependent mechanism) to preserve the recovery behavior but suppress all follow-up diagnostics. Differential Revision: https://reviews.llvm.org/D89946 Added: Modified: clang/lib/Sema/SemaOverload.cpp clang/test/AST/ast-dump-recovery.cpp clang/test/SemaCXX/typo-correction-delayed.cpp Removed: ################################################################################ diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp index d7b985a7b329..ae106ff4557a 100644 --- a/clang/lib/Sema/SemaOverload.cpp +++ b/clang/lib/Sema/SemaOverload.cpp @@ -12735,8 +12735,6 @@ class BuildRecoveryCallExprRAII { } /// Attempts to recover from a call where no functions were found. -/// -/// Returns true if new candidates were found. static ExprResult BuildRecoveryCallExpr(Sema &SemaRef, Scope *S, Expr *Fn, UnresolvedLookupExpr *ULE, @@ -12793,7 +12791,7 @@ BuildRecoveryCallExpr(Sema &SemaRef, Scope *S, Expr *Fn, return ExprError(); } - // Build an implicit member call if appropriate. Just drop the + // Build an implicit member access expression if appropriate. Just drop the // casts and such from the call, we don't really care. ExprResult NewFn = ExprError(); if ((*R.begin())->isCXXClassMember()) @@ -12808,12 +12806,19 @@ BuildRecoveryCallExpr(Sema &SemaRef, Scope *S, Expr *Fn, if (NewFn.isInvalid()) return ExprError(); - // This shouldn't cause an infinite loop because we're giving it - // an expression with viable lookup results, which should never - // end up here. - return SemaRef.BuildCallExpr(/*Scope*/ nullptr, NewFn.get(), LParenLoc, - MultiExprArg(Args.data(), Args.size()), - RParenLoc); + auto CallE = + SemaRef.BuildCallExpr(/*Scope*/ nullptr, NewFn.get(), LParenLoc, + MultiExprArg(Args.data(), Args.size()), RParenLoc); + if (CallE.isInvalid()) + return ExprError(); + // We now have recovered a callee. However, building a real call may lead to + // incorrect secondary diagnostics if our recovery wasn't correct. + // We keep the recovery behavior but suppress all following diagnostics by + // using RecoveryExpr. We deliberately drop the return type of the recovery + // function, and rely on clang's dependent mechanism to suppress following + // diagnostics. + return SemaRef.CreateRecoveryExpr(CallE.get()->getBeginLoc(), + CallE.get()->getEndLoc(), {CallE.get()}); } /// Constructs and populates an OverloadedCandidateSet from diff --git a/clang/test/AST/ast-dump-recovery.cpp b/clang/test/AST/ast-dump-recovery.cpp index 69d5f80427cb..366b3bfd9e07 100644 --- a/clang/test/AST/ast-dump-recovery.cpp +++ b/clang/test/AST/ast-dump-recovery.cpp @@ -271,3 +271,14 @@ void InvalidCondition() { // CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 2 invalid() ? 1 : 2; } + +void abcc(); +void TypoCorrection() { + // RecoveryExpr is always dependent-type in this case in order to suppress + // following diagnostics. + // CHECK: RecoveryExpr {{.*}} '<dependent type>' + // CHECK-NEXT: `-CallExpr {{.*}} 'void' + // CHECK-NEXT: `-ImplicitCastExpr + // CHECK-NEXT: `-DeclRefExpr {{.*}} 'abcc' + abc(); +} diff --git a/clang/test/SemaCXX/typo-correction-delayed.cpp b/clang/test/SemaCXX/typo-correction-delayed.cpp index 66d19daf66fd..aa136a08be4f 100644 --- a/clang/test/SemaCXX/typo-correction-delayed.cpp +++ b/clang/test/SemaCXX/typo-correction-delayed.cpp @@ -209,6 +209,15 @@ int z = 1 ? N : ; // expected-error {{expected expression}} // expected-error-re@-1 {{use of undeclared identifier 'N'{{$}}}} } +namespace noSecondaryDiags { +void abcc(); // expected-note {{'abcc' declared here}} + +void test() { + // Verify the secondary diagnostic ".. convertible to 'bool'" is suppressed. + if (abc()) {} // expected-error {{use of undeclared identifier 'abc'; did you mean 'abcc'?}} +} +} + // PR 23285. This test must be at the end of the file to avoid additional, // unwanted diagnostics. // expected-error-re@+2 {{use of undeclared identifier 'uintmax_t'{{$}}}} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
